-
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
Round compressed grid locations #2509
Conversation
Titan benchmarks
Link to QoR spreadsheet |
QoR looks promising ... will need to go through and check the various test failures to make sure they are small circuit QoR changes that are neutral or irrelevant. |
(Your idea): could also run a 3D arch regtest (your call). |
vtr_reg_nightly_test1 and vtr_reg_nightly_test1_odin failures are fixed. vtr_reg_nightly_test3_odin hits the 7 hours executin time limit, which is a little strange because vtr_reg_nightly_test3 finishes in ~1 hour. |
If this looks like a random failure we could move some test out of vtr_reg_nightly_test3_odin (and put it in another suite, or tweak the designs used). |
I ran nightly3_odin tasks on wintermute. There is no significant execution time difference between this branch and the master branch on wintermute. |
OK, if you resolve the conflicts we can merge. They look like a bunch of golden result conflicts, so hopefully not too hard to resolve. Maybe the odin II run is at the edge of 6 hours and we need to split it? |
I am running tasks in nightly3_odin as different CI tests to see how long each of them takes. On master, the entire test takes ~3.5 hours. If we are at the edge of the 7 hours time limit, it means that the execution time has doubled. |
The mcml circuit in vtr_reg_qor_chain_predictor_off has an execution time about 5 times longer than the master branch. Finding the minimum channel width takes 38 times longer than on the master branch. I missed this unusual increase in runtime because I was using the geomean to compare the execution time of two branches. Since only a single circuit's runtime increased significantly, the geomean value didn't change much. |
It seems that the router tries to route the design at a channel width much lower than the minimum channel width. Since routing_failure_predictor is off, it does not terminate the router even when each iteration takes ~500 seconds. Is this behaviour expected? |
Yes, that can happen. With the routing predictor off, a minimum channel width search could take a long time. We could just take mcml out of the test -- probably doing a minimum channel width search with larger circuits and the predictor off is not a great idea. |
…est3_odin With routing failure predictor turned off, finding the minimum channel width takes for this circuit takes so long that the CI test failed.
Update: @soheilshahrouz has removed mcml from the problematic (slow) test. Need to remove it from the golden results: 2024-06-03T19:56:18.0542628Z �[32;1m19:56:18�[0m | regression_tests/vtr_reg_nightly_test3_odin/vtr_reg_qor_chain_depop...[Pass] Also some minimum channel width increases (out of the allowed bounds) on a few small circuits, which is probably due to overwriting new golden results with the results from this PR. Checking that all those failures are in that class and updating golden should fix that. |
Whoops, it looks like another merge I did nuked this. The error is that an enum was changed to an enum class. /root/vtr-verilog-to-routing/vtr-verilog-to-routing/vpr/src/pack/cluster_util.cpp:1248:20: error: 'BLK_FAILED_FEASIBLE' was not declared in this scope; did you mean 'e_block_pack_status::BLK_FAILED_FEASIBLE If you change that line to return e_block_pack_status::BLK_FAILED_FEASIBLE this PR should work again Kimia. |
Looks like we're getting disk quota issues in CI. @AlexandreSinger : I think you were about to change the artifact collection settings? That might be getting time critical. 2024-06-06T17:14:03.2100495Z Requested labels: self-hosted, Linux, X64 |
@vaughnbetz Perhaps. However, I think the artifacts are stored on the GitHub servers not the self-hosted runners. My theory is that we have too many CI runs in flight right now. We currently have 12 CI runs in flight, each one running 17 independent tests (running on different self-hosted runners), and we have 100 self-hosted runners. I do not think its a coincidence that we are starting to hit issues when the number of runners required goes above 100. I think once we change the CI triggers it should help reduce the load. |
…rea_per_tile for vtr_reg_nightly_test1_odin/arithmetic_tasks/multless_consts/fixed_k6_frac_2ripple_N8_22nm.xml/mult_007.v
@vaughnbetz |
echo_compressed_grids() only printed the first element in each compressed grid axis 3D support added a new dimension to each compressed grid axis. This new dimension was not indexed properly in echo_compressed_grids()
When translating a grid location to a compressed grid location, we call std::lower_bound() to find the corresponding compressed location. Assume that we want to translate the grid lcoation (66, 67) for a DSP block in a grid with DSP columns at x=50 and x=70. The current grid_loc_to_compressed_loc_approx() would select the column at x=50, while the one at x=70 was closer. This PR changes grid_loc_to_compressed_loc_approx() to choose the closest compressed location instead.
In calculate_centroid_loc(), we cast float variables to int, which is equivalent to taking their floor. This means that the centroid layer is always zero unless all connected pins are on layer 1.
How Has This Been Tested?
I ran experiments on Titan benchmarks. I'll post QoR summary in a comment in this PR. If the results are promising, we can experiment with other benchmarks and architectures.