Skip to content

Conversation

@stephenswat
Copy link
Member

This commit switches the Kálmán filters to use the Joseph form covariance update which is known to be usable for any gain matrix $K$ even if the gain matrix is distorted by numerical imprecision. Empirically, this seems to resolve quite a few issues with our filtering algorithms.

@stephenswat stephenswat added improvement Improve an existing feature physics Physics performance monitoring labels Jul 1, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 1, 2025

@stephenswat
Copy link
Member Author

Performance summary

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

Graphical

Tabular

Kernel 673d408 e2c041d Delta
fit_forward 80.40 ms 82.52 ms 2.6%
propagate_to_next_surface 56.52 ms 56.50 ms -0.0%
fit_backward 40.55 ms 43.15 ms 6.4%
count_triplets 8.34 ms 8.34 ms -0.0%
find_tracks 4.28 ms 4.69 ms 9.5%
find_triplets 4.43 ms 4.44 ms 0.2%
count_doublets 1.70 ms 1.73 ms 1.5%
find_doublets 1.28 ms 1.27 ms -1.1%
fit_prelude 1.10 ms 1.10 ms 0.0%
ccl_kernel 870.63 μs 871.46 μs 0.1%
Thrust::sort 424.23 μs 424.08 μs -0.0%
select_seeds 349.30 μs 344.97 μs -1.2%
build_tracks 193.36 μs 193.29 μs -0.0%
update_triplet_weights 99.15 μs 99.91 μs 0.8%
apply_interaction 63.58 μs 63.49 μs -0.1%
fill_sort_keys 44.27 μs 44.31 μs 0.1%
estimate_track_params 34.47 μs 34.47 μs -0.0%
populate_grid 30.45 μs 30.41 μs -0.1%
count_grid_capacities 29.13 μs 29.12 μs -0.0%
unknown 20.51 μs 20.59 μs 0.4%
form_spacepoints 12.53 μs 12.45 μs -0.6%
reduce_triplet_counts 5.39 μs 5.39 μs 0.0%
make_barcode_sequence 1.02 μs 1.01 μs -0.3%
fill_prefix_sum 165.36 ns 165.36 ns -0.0%
Total 200.80 ms 205.94 ms 2.6%

Important

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

Note

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

@beomki-yeo
Copy link
Contributor

The longer computation time is because there are more tracks which survive during their propagation? or is it purely from the increased amount of computation?

@stephenswat
Copy link
Member Author

The longer computation time is because there are more tracks which survive during their propagation? or is it purely from the increased amount of computation?

I'm not entirely sure; the fact that propagate_to_next_surface doesn't change suggests the number of tracks is the same. Only the kernels that actually do the Kálmán update step increase so it seems to just be a computational change.

@krasznaa
Copy link
Member

So, what's the idea with this PR? If it makes our calculations more stable, I would still try to get it in, even with the small performance penalty.

@stephenswat
Copy link
Member Author

Yes I am happy go ahead with this. @niermann999 could you test if this fixes any of the KF problems you found? We should also merge in your KF tests ASAP.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 6, 2025

@stephenswat

This comment was marked as outdated.

This commit switches the Kálmán filters to use the Joseph form
covariance update which is known to be usable for any gain matrix $K$
even if the gain matrix is distorted by numerical imprecision.
Empirically, this seems to resolve quite a few issues with our filtering
algorithms.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 7, 2025

@stephenswat
Copy link
Member Author

Physics performance summary

Here is a summary of the physics performance effects of this PR. Command used:

traccc_seeding_example_cuda --input-directory=/data/Acts/odd-simulations-20240506/geant4_ttbar_mu200 --digitization-file=geometries/odd/odd-digi-geometric-config.json --detector-file=geometries/odd/odd-detray_geometry_detray.json --grid-file=geometries/odd/odd-detray_surface_grids_detray.json --material-file=geometries/odd/odd-detray_material_detray.json --input-events=10 --use-acts-geom-source=on --check-performance --truth-finding-min-track-candidates=5 --truth-finding-min-pt=1.0 --truth-finding-min-z=-150 --truth-finding-max-z=150 --truth-finding-max-r=10 --seed-matching-ratio=0.99 --track-matching-ratio=0.5 --track-candidates-range=5:100 --seedfinder-vertex-range=-150:150

Seeding performance

Total number of seeds went from 298341 to 298345 (+0.0%)

Seeding plots



Track finding performance

Total number of found tracks went from 55970 to 55972 (+0.0%)

Finding plots









Track fitting performance

Fitting plots



















Note

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

const matrix_type<D, D> i_minus_hk = I_m - H * K;
// See
// https://indico.cern.ch/event/1564924/contributions/6629447/attachments/3113201/5519076/asami_250731_acts.pdf
const matrix_type<D, D> R =
Copy link
Contributor

Choose a reason for hiding this comment

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

Should something like this also be done for R_smt? Or does that not make sense, since the smoothed covariance is calculated differently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improve an existing feature physics Physics performance monitoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants