Skip to content

Fix 3D Tests #2980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 17, 2025
Merged

Fix 3D Tests #2980

merged 9 commits into from
Apr 17, 2025

Conversation

amin1377
Copy link
Contributor

@amin1377 amin1377 commented Apr 15, 2025

This issue occurred in 3D architectures when the per-layer bounding box was used. Previously, the net wirelength calculation always relied on the cube bounding box data structures, regardless of the bounding box type. This fix ensures the correct data structure is used based on the selected bounding box type.
In addition, a strong test has been added for the 3D architecture to cover both bounding box types and to test with both 3D connection blocks (CB) and 3D switch blocks (SW).

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Apr 15, 2025
@amin1377 amin1377 requested a review from soheilshahrouz April 15, 2025 16:39
@AmirhosseinPoolad
Copy link
Contributor

Hi Amin, thanks for this PR! Sorry for the random unsolicited review, but the added tests make the strong tests take more than 7 minutes longer on wintermute. Also, Test / R: Strong took 34 minutes for this PR compared to 20 minutes for the two commits in master I looked at. Not that it's the end of the world but it would make testing and also waiting for your PR to finish the tests noticeably harder.

@amin1377
Copy link
Contributor Author

@AmirhosseinPoolad What you’re saying makes sense. For now, I think it’s better to merge this PR as soon as the tests pass, to ensure the test failure is resolved. I’ll look into using smaller circuits for testing in a separate PR.

@amin1377
Copy link
Contributor Author

amin1377 commented Apr 17, 2025

@AmirhosseinPoolad @soheilshahrouz Nightly tests passed (Link). Feel free to merge this PR if there are no further suggestions.

@amin1377 amin1377 changed the title [WIP] Fix 3D Tests Fix 3D Tests Apr 17, 2025
@AlexandreSinger
Copy link
Contributor

AlexandreSinger commented Apr 17, 2025

@amin1377 I agree with Amir. I was looking through the strong tests and the slowest ones were from Titan, which ran only one circuit that took less than 60 seconds; all other tests take less than 30 seconds. Your strong tests take more than 60 seconds each and you add 6 of them:
image
This will make a noticeable change to the run time of the strong tests.

I do not agree with merging this PR and then fixing it later. I would rather you separate out your strong tests changes into a separate PR so we can discuss further without blocking your code fixes. Is there a smaller test circuit you can use with 3D architectures?

@amin1377
Copy link
Contributor Author

@AlexandreSinger As I already mentioned to Amir, I agree with the general principle of keeping strong tests short. The circuit I used is actually the smallest Titan benchmark, but I can reduce the number of tests and apply a few tricks to shorten them further. Given my current workload and the pace of development on VTR, I'd prefer to keep the existing tests as they are and handle the shortening in a follow-up PR.

@soheilshahrouz soheilshahrouz merged commit 2f5e240 into master Apr 17, 2025
36 checks passed
@soheilshahrouz soheilshahrouz deleted the fix_3d_test branch April 17, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants