-
Notifications
You must be signed in to change notification settings - Fork 397
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
[Route] Added astar_offset Parameter #2601
[Route] Added astar_offset Parameter #2601
Conversation
a7672d5
to
1df6df1
Compare
1df6df1
to
fbcca5d
Compare
Should also do a QoR check to compare router runtimes and make sure there isn't a noticeable increase. |
@vaughnbetz FYI, this was the section I was concerned about regarding the RCV path manager: The issue I see here is that the astar_fac is used to scale the raw delay and congestion costs independently (not the weighted sum as its done in the default router). I am not sure the affects this may have on the results but it may cause more critical nets to be biased toward / away from congestion / delay way more than intended. For this PR I just followed the example and offset at the same point; but I wonder how this may affect RCV's results. |
For RCV we need to keep the delay separate from congestion for longer, as we're not targeting min delay anymore but a delay window instead. I think we should avoid manipulating the delay part of the cost with RCV at all (no multiply by A*fac, no offset, etc.). |
fbcca5d
to
ada43a5
Compare
776f5f4
to
e3ce4bc
Compare
QoR check for these changes:
Baseline results are from the common commit from Master and this branch. Seems to be no affect on runtime. Rerunning CI. |
e3ce4bc
to
5a91e12
Compare
Using an astar_offset can help better tune the ordering heuristic for the search used to find the shortest path in the routing graph. It is also necessary to ensure that the heuristic is an underestimate (without setting the astar_fac to 0.0).
5a91e12
to
70c8c2c
Compare
@vaughnbetz This PR has passed CI. I have made sure that the documentation is updated with this new CLI parameter. QoR results are shown above on Titan. Seems to be no loss in runtime or quality. |
Looks good, thanks. |
Using an astar_offset can help better tune the ordering heuristic for the search used to find the shortest path in the routing graph. It is also necessary to ensure that the heuristic is an underestimate (without setting the astar_fac to 0.0).