Skip to content
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

Merged

Conversation

AlexandreSinger
Copy link
Contributor

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).

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Jun 8, 2024
@AlexandreSinger AlexandreSinger force-pushed the feature-astar-offset branch 2 times, most recently from a7672d5 to 1df6df1 Compare June 9, 2024 18:31
vpr/src/base/read_options.cpp Outdated Show resolved Hide resolved
vpr/src/route/connection_router.cpp Show resolved Hide resolved
vpr/src/route/connection_router.cpp Outdated Show resolved Hide resolved
vpr/src/base/read_options.cpp Show resolved Hide resolved
vpr/src/base/vpr_types.h Outdated Show resolved Hide resolved
@vaughnbetz
Copy link
Contributor

Should also do a QoR check to compare router runtimes and make sure there isn't a noticeable increase.

@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz FYI, this was the section I was concerned about regarding the RCV path manager:
image

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.

@vaughnbetz
Copy link
Contributor

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.
RCV also won't work unless A* fac is essentially 1 on the delay part; to hit a delay window you need to estimate pretty precisely, rather than overestimating to get a directed search. To make the search more directed I think we'd need to add an additional cost outside the delay window computation that was something like (A*fac-1)remaining delay to make the search more directed. Or perhaps the resource cost being multiplied by Afac would be enough to drive toward the target.

I think we should avoid manipulating the delay part of the cost with RCV at all (no multiply by A*fac, no offset, etc.).

@AlexandreSinger
Copy link
Contributor Author

QoR check for these changes:

  baseline_results.txt parse_results.txt
vtr_flow_elapsed_time 1 0.999587653
num_LAB 1 1
num_DSP 1 1
num_M9K 1 1
num_M144K 1 1
max_vpr_mem 1 1.000038815
num_pre_packed_blocks 1 1
num_post_packed_blocks 1 1
device_grid_tiles 1 1
pack_time 1 1.004174461
placed_wirelength_est 1 1
place_time 1 1.00860537
placed_CPD_est 1 1
routed_wirelength 1 0.999503305
critical_path_delay 1 0.994528741
geomean_nonvirtual_intradomain_critical_path_delay 1 0.994283369
crit_path_route_time 1 0.959547268

Baseline results are from the common commit from Master and this branch. Seems to be no affect on runtime. Rerunning CI.

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).
@github-actions github-actions bot added the docs Documentation label Jul 31, 2024
@AlexandreSinger
Copy link
Contributor Author

@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.

@vaughnbetz
Copy link
Contributor

Looks good, thanks.

@vaughnbetz vaughnbetz merged commit 6405610 into verilog-to-routing:master Jul 31, 2024
53 checks passed
@AlexandreSinger AlexandreSinger deleted the feature-astar-offset branch November 27, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants