-
Notifications
You must be signed in to change notification settings - Fork 415
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
Fix 3D Tests #2980
Conversation
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. |
@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. |
@AmirhosseinPoolad @soheilshahrouz Nightly tests passed (Link). Feel free to merge this PR if there are no further suggestions. |
@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: 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? |
@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. |
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).