[WIP] Router Lookahead Profiler #2683

nedsels · 2024-08-12T18:26:28Z

There is a new class LookaheadProfiler which records a variety of data that can be used to evaluate the quality of the router lookahead.

Description

Using the branch reconstructed in RouteTree::add_subtree_from_heap(), LookaheadProfiler records the following data into a .csv file for every node in the branch:

iteration no.
source node id (the first node in the branch; not necessarily a SOURCE)
sink node id
sink block name
sink atom block model
sink cluster block type
sink cluster tile height
sink cluster tile width
current node
node type
node length
num. nodes from sink
delta x, delta y
actual cost, delay, and congestion from current node to sink
predicted cost, delay, and congestion from current node to sink (predicted by the lookahead)
criticality

The profiler is enabled by specifying CMAKE_PARAMS="DVPR_PROFILE_LOOKAHEAD=on" when building VPR, and also using the command-line option --profile_router_lookahead [output_file_name.csv].

This data can be analyzed using the script vtr_flow/scripts/profiling_utils/parse_lookahead_data.py, which produces a data summary file and a number of scatter plots, bar graphs, pie charts, and heatmaps. (TBD: Currently, using -h gives detailed instructions, but a README will be added as well.)

Motivation and Context

This was used to verify issues that led to #2639. It can be used to identify current/future issues with the lookahead.

How Has This Been Tested?

There is a new CI test that ensures that VPR builds properly with PROFILE_LOOKAHEAD defined.

Currently, even with PROFILE_LOOKAHEAD undefined, there is a degradation in QoR. These results are averages over 3 runs each:

	Routing Runtime (s)	CPD (ns)	Wirelength	Memory Footprint
`vtr` (old)	0.94	10.00	27433.36	153.79
`vtr` (new)	0.91	10.00	27520.35	153.59
`titan` (old)	294.51	17.18	3200094.23	5494.64
`titan` (new)	294.30	17.48	3252463.91	5486.61
`titan_large`* (old)	618.20	17.72	6430833.54	8865.55
`titan_large`* (new)	614.95	18.14	6532452.49	8854.25

For some reason, there is a 1.6% wirelength increase on titan (and titan_large) circuits and a 2.4% CPD increase on titan_large circuits.

*10 largest titan circuits.

Types of changes

Bug fix (change which fixes an issue)
New feature (change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My change requires a change to the documentation
I have updated the documentation accordingly
I have added tests to cover my changes
All new and existing tests passed

vaughnbetz · 2024-08-12T22:35:10Z

@soheilshahrouz was also interested in reviewing this code. We'll eventually need a QoR run to show no slowdown (good to run both VTR and Titan suites and attach spreadsheets).

utils/route_diag/src/main.cpp

soheilshahrouz · 2024-08-13T14:21:43Z

vpr/src/base/read_options.cpp

+        .help(
+            "For every routed sink, records the cost, delay, and congestion estimated by the router lookahead and the "
+            "actual cost, delay, and congestion, from every node along each route to the sink. These results, along with many "
+            "other attributes of the node, are recorded into lookahead_verifier_info.csv.")


I think the user should be able to specify the file name.

vpr/src/base/vpr_context.h

vpr/src/base/vpr_types.h

soheilshahrouz · 2024-08-13T15:32:15Z

vpr/src/route/router_lookahead.cpp

@@ -109,7 +115,7 @@ float NoOpLookahead::get_expected_cost(RRNodeId /*current_node*/, RRNodeId /*tar
    return 0.;
 }

-std::pair<float, float> NoOpLookahead::get_expected_delay_and_cong(RRNodeId /*node*/, RRNodeId /*target_node*/, const t_conn_cost_params& /*params*/, float /*R_upstream*/) const {
+std::pair<float, float> NoOpLookahead::get_expected_delay_and_cong(RRNodeId, RRNodeId, const t_conn_cost_params&, float) const {


why did you remove argument name comments?

soheilshahrouz · 2024-08-13T15:34:58Z

vpr/src/route/router_lookahead_compressed_map.h

-                                                        RRNodeId to_node,
-                                                        const t_conn_cost_params& params,
-                                                        float R_upstream) const override;
+    std::pair<float, float> get_expected_delay_and_cong(RRNodeId from_node, RRNodeId to_node, const t_conn_cost_params& params, float R_upstream) const override;


soheilshahrouz · 2024-08-13T15:35:39Z

vpr/src/route/router_lookahead_extended_map.cpp

@@ -181,7 +181,7 @@ float ExtendedMapLookahead::get_chan_ipin_delays(RRNodeId to_node) const {
 //
 //  The from_node can be of one of the following types: CHANX, CHANY, SOURCE, OPIN
 //  The to_node is always a SINK
-std::pair<float, float> ExtendedMapLookahead::get_expected_delay_and_cong(RRNodeId from_node, RRNodeId to_node, const t_conn_cost_params& params, float /*R_upstream*/) const {
+std::pair<float, float> ExtendedMapLookahead::get_expected_delay_and_cong(RRNodeId from_node, RRNodeId to_node, const t_conn_cost_params& params, float) const {


why was the argument name comment removed?

soheilshahrouz · 2024-08-13T15:36:03Z

vpr/src/route/router_lookahead_extended_map.cpp

-    float expected_delay_cost = params.criticality * expected_delay;
-    float expected_cong_cost = (1.0 - params.criticality) * expected_congestion;
+    float expected_delay_cost = expected_delay * params.criticality;
+    float expected_cong_cost = expected_congestion * (1. - params.criticality);


why did you need to change the order?

vpr/test/test_connection_router.cpp

nedsels · 2024-08-15T00:14:40Z

@vaughnbetz @AlexandreSinger Python lint is failing because some of the libraries I used can't be imported; is there any way around this?

soheilshahrouz · 2024-08-15T04:48:21Z

@vaughnbetz @AlexandreSinger Python lint is failing because some of the libraries I used can't be imported; is there any way around this?

It can be probably fixed by updating requirements.txt file

vpr/src/route/lookahead_profiler.cpp

nedsels · 2024-08-16T15:59:14Z

@soheilshahrouz @vaughnbetz Some regtests are having some QoR failures. On vtr_reg_nightly_test2 (and _odin), there are a few, but I'm not sure if the first 3 are small enough to just update the golden results:

However, on vtr_reg_nightly_test7, there are some failures which seem quite bad, and I'm not sure how to approach fixing these. Do you have any advice?

vaughnbetz · 2024-08-16T22:50:02Z

I'm not too worried about dsip; it is small.

The long route times on wb_conmax are also probably OK if there are just a few of these. Adding @amin1377 since those 3D and flat routing ones are probably his tests.

The more I think about it the more I think we should #ifdef adding data members to the heap though; I am really not keen on carrying around extra data in a big and hot data structure for profiling. I think it would be better to have no impact on the default flow.

vaughnbetz

Looks good; just have some commenting suggestions. We'll also need a QoR run on VTR & Titan to check cpu time & quality.

vaughnbetz · 2024-08-23T17:52:43Z

vpr/src/base/read_options.cpp

+        .help(
+            "For every routed sink, record the cost, delay, and congestion estimated by the router lookahead and the "
+            "actual cost, delay, and congestion, from every node along each route to the sink. These results, along with many "
+            "other attributes of the node, are recorded into the file name provided. File extension must be .csv.")


I'd add "to assist in debugging or validating the router lookahead."

vaughnbetz · 2024-08-23T19:08:08Z

vpr/src/base/vpr_types.h

@@ -1400,6 +1400,8 @@ struct t_router_opts {
    std::string write_intra_cluster_router_lookahead;
    std::string read_intra_cluster_router_lookahead;

+    std::string lookahead_profiling_output;


Add doxygen comment for this (can use ///).
If it's an empty string does it not get printed? (If so, say that).

vaughnbetz · 2024-08-23T19:09:07Z

vpr/src/base/vpr_types.h

 */
 struct t_rr_node_route_inf {
    RREdgeId prev_edge;

    float acc_cost;
    float path_cost;
    float backward_path_cost;
+#ifdef PROFILE_LOOKAHEAD


I'd add a brief comment saying why this extra data is needed for PROFILE_LOOKAHEAD, and that we conditionally compile it because this is a hot and large data structure.

vaughnbetz · 2024-08-23T19:10:31Z

vpr/src/place/timing_place_lookup.cpp

@@ -441,7 +441,7 @@ static void add_delay_to_matrix(
 }

 static void generic_compute_matrix_dijkstra_expansion(


It would be good to add a doxygen comment for this, detailing what you know about the function and its arguments (and for sure what the route_profiler does).

vaughnbetz · 2024-08-23T19:13:52Z

vpr/src/route/connection_router.cpp

@@ -239,7 +239,21 @@ t_heap* ConnectionRouter<Heap>::timing_driven_route_connection_from_heap(RRNodeI
            // This is then placed into the traceback so that the correct path is returned
            // TODO: This can be eliminated by modifying the actual traceback function in route_timing
            if (rcv_path_manager.is_enabled()) {
-                rcv_path_manager.insert_backwards_path_into_traceback(cheapest->path_data, cheapest->cost, cheapest->backward_path_cost, route_ctx);
+#ifdef PROFILE_LOOKAHEAD
+                rcv_path_manager.insert_backwards_path_into_traceback(cheapest->path_data,


Is PROFILE_LOOKAHEAD tested with rcv? If not, we could just choose to not support PROFILE_LOOKAHEAD with RCV to simplify the code a little.

vaughnbetz · 2024-08-23T19:28:34Z

vpr/src/route/router_delay_profiling.h

@@ -53,7 +58,8 @@ class RouterDelayProfiler {

 vtr::vector<RRNodeId, float> calculate_all_path_delays_from_rr_node(RRNodeId src_rr_node,


If you know enough about this routine to write a doxygen comment it would be good to write one.

vaughnbetz · 2024-08-23T23:59:32Z

vpr/src/route/lookahead_profiler.cpp

+    // pass in an iteration value, which is the only context in which we want to profile.
+    if (iteration < 1)
+        return;
+


This function should have a comment giving an overview of how it works, unless that's already covered in the (to be added) lookahead_profiler.cpp file-scope comment that gives the high-level context.

vaughnbetz · 2024-08-24T00:01:44Z

vpr/src/route/lookahead_profiler.h

+     *
+     * @param iteration The router iteration.
+     * @param target_net_pin_index Target pin of this sink in the net.
+     * @param cost_params


Suggest briefly commenting each input's purpose.

You need a high-level comment of how the lookahead profiler works somewhere (file level, or maybe on this routine).

vaughnbetz · 2024-08-24T00:06:04Z

vpr/src/route/router_lookahead.h

@@ -123,7 +169,7 @@ const RouterLookahead* get_cached_router_lookahead(const t_det_routing_arch& det
 class ClassicLookahead : public RouterLookahead {
  public:
    float get_expected_cost(RRNodeId node, RRNodeId target_node, const t_conn_cost_params& params, float R_upstream) const override;
-    std::pair<float, float> get_expected_delay_and_cong(RRNodeId node, RRNodeId target_node, const t_conn_cost_params& params, float R_upstream) const override;
+    std::pair<float, float> get_expected_delay_and_cong_ignore_criticality(RRNodeId node, RRNodeId target_node, const t_conn_cost_params& params, float R_upstream) const override;


Long line; maybe break it.

vaughnbetz · 2024-08-24T00:10:08Z

vpr/src/route/router_lookahead.h

+    /**
+     * @brief Get expected (delay, congestion) from node to target_node.
+     *
+     * @attention Either compute or read methods must be invoked before invoking get_expected_delay_and_cong_ignore_criticality.


must be invoked --> must be invoked to set up the lookahead data structures

nedsels · 2024-08-29T20:09:15Z

Sorry for the delay with the QoR results; for some reason, they have degraded with this change. I'm trying to figure out why; if/when I can, I'll post the results in the description.

vaughnbetz · 2024-08-29T21:48:26Z

Thanks. If you can post what the current QoR is that would be helpful too; even if it is not that good it helps show what the status is.

Nathan Shreve added 4 commits June 26, 2024 18:51

Created lookahead profiler (restored changes from previous branch)

739029b

LookaheadProfiler now saves sink node info for efficiency

aa5570b

Merge branch 'master' into feature_router_lookahead_verifier

c88364a

Refactor

d406c57

github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code scripts Utility & Infrastructure scripts lang-python Python code labels Aug 12, 2024

Added command-line option for profiler

c86ff7c

nedsels force-pushed the feature_router_lookahead_verifier branch from d1025c9 to c86ff7c Compare August 12, 2024 19:44

Commented LookaheadProfiler

ddf34eb

nedsels requested a review from AlexandreSinger August 12, 2024 20:49

More comments

0826ad7

nedsels force-pushed the feature_router_lookahead_verifier branch from 25cbc7f to 0826ad7 Compare August 12, 2024 20:52

Moved parse_lookahead_data.py

e8ea974

Nathan Shreve added 2 commits August 12, 2024 18:49

Moved, refactored, and commented parse_lookahead_data.py

0b5a752

Fixed multithreading in parsing script

cbe1334

nedsels force-pushed the feature_router_lookahead_verifier branch from e939ea2 to cbe1334 Compare August 13, 2024 01:46

soheilshahrouz reviewed Aug 13, 2024

View reviewed changes

Nathan Shreve added 5 commits August 13, 2024 15:26

More refactoring, commenting

80086ff

Fixed linting errors in python script

c747096

Refactored LookaheadProfiler

e753094

Eliminated profile_lookahead parameter in update_from_heap

d9f4ee3

Allowed users to enter a file name for profiler output

dfab0a8

soheilshahrouz reviewed Aug 15, 2024

View reviewed changes

vpr/src/route/lookahead_profiler.cpp Outdated Show resolved Hide resolved

Improved multiprocessing efficiency in script

c9801b7

nedsels force-pushed the feature_router_lookahead_verifier branch from d5dc37d to 3b9615d Compare August 15, 2024 17:49

Nathan Shreve added 4 commits August 15, 2024 14:04

Cleaned a few function calls

7fe0b55

LookaheadProfiler::clear() now uses vtr::release_memory()

c8b196e

Requirements for script added to requirements.txt

ed4460d

Updated some regtest seeds and golden results

ab2c731

nedsels force-pushed the feature_router_lookahead_verifier branch from ebdc050 to 1168090 Compare August 15, 2024 18:06

Merge branch 'master' into feature_router_lookahead_verifier

2db5325

nedsels force-pushed the feature_router_lookahead_verifier branch from d7f1068 to 2db5325 Compare August 15, 2024 19:25

#ifdef-ed extra heap data and added CI test

17ab565

github-actions bot added infra Project Infrastructure build Build system lang-make CMake/Make code labels Aug 20, 2024

Nathan Shreve added 2 commits August 20, 2024 14:37

Merge branch 'master' into feature_router_lookahead_verifier

595af59

Fixed compilation warnings

2f10ec5

github-actions bot added the external_libs label Aug 20, 2024

Nathan Shreve added 4 commits August 20, 2024 16:03

Fixed error in record()

9bcad70

Updated golden results

c05e498

Merge branch 'master' into feature_router_lookahead_verifier

643811b

More updated golden results

1c9bb58

vaughnbetz requested changes Aug 24, 2024

View reviewed changes

Nathan Shreve added 2 commits August 30, 2024 01:37

Disabled RCV support for RouterLookahead, comments

deddc8a

Merge branch 'master' into feature_router_lookahead_verifier

971eebc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Router Lookahead Profiler #2683

[WIP] Router Lookahead Profiler #2683

nedsels commented Aug 12, 2024 •

edited

Loading

vaughnbetz commented Aug 12, 2024

soheilshahrouz Aug 13, 2024

soheilshahrouz Aug 13, 2024

soheilshahrouz Aug 13, 2024

soheilshahrouz Aug 13, 2024

soheilshahrouz Aug 13, 2024

nedsels commented Aug 15, 2024

soheilshahrouz commented Aug 15, 2024

nedsels commented Aug 16, 2024 •

edited

Loading

vaughnbetz commented Aug 16, 2024

vaughnbetz left a comment

vaughnbetz Aug 23, 2024

vaughnbetz Aug 23, 2024

vaughnbetz Aug 23, 2024

vaughnbetz Aug 23, 2024

vaughnbetz Aug 23, 2024

vaughnbetz Aug 23, 2024

vaughnbetz Aug 23, 2024

vaughnbetz Aug 24, 2024

vaughnbetz Aug 24, 2024

vaughnbetz Aug 24, 2024

vaughnbetz Aug 24, 2024

nedsels commented Aug 29, 2024

vaughnbetz commented Aug 29, 2024

		@@ -441,7 +441,7 @@ static void add_delay_to_matrix(
		}

		static void generic_compute_matrix_dijkstra_expansion(

		@@ -53,7 +58,8 @@ class RouterDelayProfiler {

		vtr::vector<RRNodeId, float> calculate_all_path_delays_from_rr_node(RRNodeId src_rr_node,

[WIP] Router Lookahead Profiler #2683

Are you sure you want to change the base?

[WIP] Router Lookahead Profiler #2683

Conversation

nedsels commented Aug 12, 2024 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

vaughnbetz commented Aug 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nedsels commented Aug 15, 2024

soheilshahrouz commented Aug 15, 2024

nedsels commented Aug 16, 2024 • edited Loading

vaughnbetz commented Aug 16, 2024

vaughnbetz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nedsels commented Aug 29, 2024

vaughnbetz commented Aug 29, 2024

nedsels commented Aug 12, 2024 •

edited

Loading

nedsels commented Aug 16, 2024 •

edited

Loading