-
Notifications
You must be signed in to change notification settings - Fork 396
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
Inverse of the average number of tracks per channel #2741
Conversation
QoR is compared with #2676 This is measured before inlining
|
/vtr_reg_nightly_test3/vtr_reg_qor_chain_large
|
QoR is compared with #2676 This is measured after inlining
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code all looks good -- I have no comments to address :).
Whoops, I lied. I think the [-1, ...] indexed Matrix you want can just use the VtrNdOffsetMatrix, and doesn't need a new class. |
Ah, I see. Maybe then the new class you've created should be pulled out into a new utility class that allows -ve offsets on top of a VtrNdMatrix? |
Thanks! The changes to generalize and use nd_offset_matrix look good. |
Looks like there are some memory faults / assertion failures. From the valgrind run:
2024-09-26T17:27:08.5689907Z User time (seconds): 0.48 |
Update: seg fault fixed, @soheilshahrouz gathering updated QoR to be safe. |
@soheilshahrouz : if you post the QoR data (and it looks good) we can merge this. |
|
LGTM! |
@vaughnbetz Sorry for all the messages; but I think this PR was the one that is causing the CI to fail now. The CI run for this PR was a week old and since then there have been golden result updates and other minor changes which seems to have been revealed by this PR. @fkosar-ql Sorry for wrongly accusing your PR! I am currently regenerating the golden results for the Strong tests since that is easy to fix. The issue with the basic test is probably something I cannot easily fix. Issue #2754 is tracking the issue. Edit: |
Before this PR, calls to
std::min()
andstd::max()
clipped bounding box coordinates between 1 and grid width/height - 2. This PR removes all these calls and initalizeschanx_place_cost_fac_
andchany_place_cost_fac_
in a way that is compatible with the new index range: [-1, grid width/height - 1].