Skip to content

Conversation

@stephenswat
Copy link
Member

@stephenswat stephenswat commented Jun 24, 2025

This commit adds a new cut to the doublet finding which is able to determine based on only the doublets if it is possible to find a third spacepoint which will form a seed within the required helix radius bounds.

Note that this cut should, mathematically, not remove any triplets, but it should reduce the time taken to find triplets.

This is the cut discussed on the traccc Mattermost recently.

Pinging @krasznaa and @pbutti as interested parties.

@stephenswat stephenswat requested a review from krasznaa June 24, 2025 15:38
@stephenswat stephenswat added the performance Performance-relevant changes label Jun 24, 2025
@stephenswat
Copy link
Member Author

stephenswat commented Jun 24, 2025

This commit was inspired by the G-200 performance breakdown shown today, where the seed finding takes up an outsided amount of time:

Screenshot From 2025-06-24 13-45-38

Thus, optimising the seeding is once again the flavour of the month.

@stephenswat
Copy link
Member Author

Performance summary

Here is a summary of the performance effects of this PR:

Graphical

Tabular

Kernel 32950a5 ef228f5 Delta
fit_forward 109.46 ms 109.15 ms -0.3%
propagate_to_next_surface 69.93 ms 69.96 ms 0.0%
fit_backward 54.04 ms 53.82 ms -0.4%
count_triplets 14.24 ms 8.19 ms -42.5%
find_triplets 6.01 ms 4.37 ms -27.2%
find_tracks 4.28 ms 4.28 ms 0.0%
count_doublets 618.27 μs 1.57 ms 154.0%
find_doublets 890.84 μs 1.33 ms 49.7%
fit_prelude 1.12 ms 1.12 ms 0.2%
ccl_kernel 872.06 μs 872.65 μs 0.1%
select_seeds 360.96 μs 348.10 μs -3.6%
Thrust::sort 271.94 μs 271.38 μs -0.2%
build_tracks 194.24 μs 192.64 μs -0.8%
update_triplet_weights 100.28 μs 99.14 μs -1.1%
DeviceRadixSortHistogramKernel 97.85 μs 97.69 μs -0.2%
apply_interaction 63.69 μs 63.84 μs 0.2%
DeviceRadixSortOnesweepKernel 54.83 μs 54.61 μs -0.4%
fill_sort_keys 44.78 μs 44.58 μs -0.4%
estimate_track_params 34.46 μs 34.37 μs -0.3%
populate_grid 30.32 μs 30.36 μs 0.1%
count_grid_capacities 29.20 μs 29.09 μs -0.4%
unknown 21.50 μs 20.52 μs -4.6%
form_spacepoints 12.59 μs 12.46 μs -1.0%
reduce_triplet_counts 6.62 μs 5.36 μs -19.1%
make_barcode_sequence 1.00 μs 1.02 μs 1.2%
DeviceRadixSortExclusiveSumKernel 510.28 ns 510.45 ns 0.0%
fill_prefix_sum 165.49 ns 165.37 ns -0.1%
Total 262.78 ms 255.97 ms -2.6%

Important

All metrics in this report are given as reciprocal throughput, not as wallclock runtime.

Warning

At least one kernel incurred a significant performance regression.

Note

This is an automated message produced on the explicit request of a human being.

@stephenswat
Copy link
Member Author

Quick note, this looks unspectacular but if I put in the numbers from the ITk execution and keep the percentages the same it looks like this:

Kernel 32950a5 ef228f5 Delta
count_triplets 295.23 ms 169.75 ms -42.5%
find_triplets 67.29 ms 48.98 ms -27.2%
count_doublets 19.64 ms 49.88 ms 154.0%
find_doublets 31.46 ms 47.09 ms 49.7%
Total 413.62 ms 315.70 ms -23.7%

@stephenswat stephenswat force-pushed the perf/extra_doublet_cut branch 4 times, most recently from c04e44c to 8902a61 Compare June 25, 2025 07:58
@stephenswat stephenswat marked this pull request as ready for review June 25, 2025 08:00
@stephenswat
Copy link
Member Author

This is now ready to go!

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it. As long as the comparison test with Acts doesn't break, I'm happy to go forward with this.

This commit adds a new cut to the doublet finding which is able to
determine based on only the doublets if it is possible to find a third
spacepoint which will form a seed within the required helix radius
bounds.

This is the cut discussed on the traccc Mattermost recently.
@stephenswat stephenswat force-pushed the perf/extra_doublet_cut branch from 8902a61 to 1b05db5 Compare June 25, 2025 17:22
@stephenswat stephenswat enabled auto-merge June 25, 2025 17:22
@sonarqubecloud
Copy link

@stephenswat stephenswat merged commit c482614 into acts-project:main Jun 25, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance-relevant changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants