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

Add a distance-based cut to select measurements close to the track #856

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

beomki-yeo
Copy link
Contributor

@beomki-yeo beomki-yeo commented Feb 11, 2025

This PR adds a CKF finding option for a given distance-based cut value: If the distance between track and measurement is smaller than the cut value, the measurement is passed and Kalman updater is applied.

With this PR, the CKF can be more input covariance agnostic, which means that we can finally go with #846.

Command used:

byeo@hermes:/mnt/nvme0n1/byeo/projects/traccc/traccc_build_FP32$     ./bin/traccc_seq_example_cuda \
    --digitization-file=geometries/odd/odd-digi-geometric-config.json \
    --detector-file=geometries/odd/odd-detray_geometry_detray.json \
    --material-file=geometries/odd/odd-detray_material_detray.json \
    --grid-file=geometries/odd/odd-detray_surface_grids_detray.json \
    --use-detray-detector=true \
    --use-acts-geom-source=true \
    --input-directory=odd/odd-simulations-20240509/geant4_ttbar_mu200/ \
    --input-events=1 \
    --compare-with-cpu

Output from this PR with the default cut value (10 mm)


==> Statistics ... 
- read    438151 cells
- created (cpu)  117907 measurements     
- created (cuda)  117907 measurements     
- created (cpu)  92892 spacepoints     
- created (cuda) 92892 spacepoints     
- created  (cpu) 42372 seeds
- created (cuda) 42372 seeds
- found (cpu)    70942 tracks
- found (cuda)   71096 tracks
- fitted (cpu)   70942 tracks
- fitted (cuda)  71096 tracks
==>Elapsed times...
           File reading  (cpu)  917 ms
         Clusterization (cuda)  141 ms
         Clusterization  (cpu)  28 ms
   Spacepoint formation (cuda)  1 ms
   Spacepoint formation  (cpu)  4 ms
                Seeding (cuda)  44 ms
                Seeding  (cpu)  6353 ms
           Track params (cuda)  0 ms
           Track params  (cpu)  13 ms
          Track finding (cuda)  326 ms
           Track finding (cpu)  8150 ms
          Track fitting (cuda)  393 ms
           Track fitting (cpu)  9147 ms
                     Wall time  25525 ms

Output from the main branch

==> Statistics ... 
- read    438151 cells
- created (cpu)  117907 measurements     
- created (cuda)  117907 measurements     
- created (cpu)  92892 spacepoints     
- created (cuda) 92892 spacepoints     
- created  (cpu) 42372 seeds
- created (cuda) 42372 seeds
- found (cpu)    133042 tracks
- found (cuda)   135006 tracks
- fitted (cpu)   133042 tracks
- fitted (cuda)  135006 tracks
==>Elapsed times...
           File reading  (cpu)  923 ms
         Clusterization (cuda)  16 ms
         Clusterization  (cpu)  31 ms
   Spacepoint formation (cuda)  1 ms
   Spacepoint formation  (cpu)  5 ms
                Seeding (cuda)  43 ms
                Seeding  (cpu)  6327 ms
           Track params (cuda)  0 ms
           Track params  (cpu)  15 ms
          Track finding (cuda)  478 ms
           Track finding (cpu)  18282 ms
          Track fitting (cuda)  857 ms
           Track fitting (cpu)  17201 ms
                     Wall time  44189 ms

The cut value decrease the number of tracks a lot. I will also post the tendency of the tracking efficiency as a function of distance cut value once the analysis is done

@beomki-yeo beomki-yeo marked this pull request as draft February 11, 2025 00:49
Copy link

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'm generally on board. Though on first look this seems a bit "simple". Does a cut like this work well for high-pT and low-pT tracks as well? 🤔

Does Acts have something similar? If not, do they have some additional cuts on top of the ones we have? 🤔

@beomki-yeo
Copy link
Contributor Author

beomki-yeo commented Feb 11, 2025

I will also post the tendency of the tracking efficiency as a function of distance cut value once the analysis is done

@krasznaa You will see some plots tomorrow (tomorrow for me..). Running with check_performance takes lots of time

@beomki-yeo
Copy link
Contributor Author

beomki-yeo commented Feb 11, 2025

Does Acts have something similar? If not, do they have some additional cuts on top of the ones we have? 🤔

There was no other cuts when I checked 1~2 years ago. Could be different now
I have not checked but andis (@asalzburger @andiwand) can comment.

@andiwand
Copy link

ACTS only has a chi2 cut which should be sufficient if the covariance is not too big

@krasznaa
Copy link
Member

Interesting... Do I understand correctly that applying such a distance cut can be done with much less floating point calculation? So that it could act as a more crude cut, reducing the number of measurements with which a new chi2 would need to be calculated? 🤔

If so, technically that would indeed sound interesting.

@stephenswat
Copy link
Member

Doesn't the fact that a distance-based cut reduce the number of tracks this much indicate that either our $\chi^2$ cut is way too loose or our covariances are way too large?

@andiwand
Copy link

Chi2 on a 2D measurement should be similar cost to a 2D distance cut. The chi2 cut will use the full 2D cov which forms an ellipse and the 2D distance cut will form a circle. So you basically neglect the off diagonal and safe a few FP ops.

You can add another cut but I would question why. It is less generic and the computation does not have a big advantage

@beomki-yeo
Copy link
Contributor Author

beomki-yeo commented Feb 11, 2025

chi2 requires kalman updater so it is super much less.

I have to note that distance-based cut is meant to be very loose not to lose any seeds. It is enough if it can replace the role of chi2-based cut for the first 1 or 2 modules where covariance is pretty large. Once the covariance is stabilized after few modules, distance-based cut will be worthless and chi2-based cut will take the major role in measurement selection

@andiwand
Copy link

andiwand commented Feb 11, 2025

you can calculate a $\chi^2$ cut before you do the KF update. this is what we do in the ACTS based Athena track finding chain

if the covariance is scaled properly you don't have the issue of collecting too many compatible measurements in the first place

@beomki-yeo
Copy link
Contributor Author

Doesn't the fact that a distance-based cut reduce the number of tracks this much indicate that either our chi2 cut is way too loose or our covariances are way too large?

Probably yes.

@beomki-yeo
Copy link
Contributor Author

you can calculate a χ cut before you do the KF update

How do you calculate this exactly?

@beomki-yeo
Copy link
Contributor Author

beomki-yeo commented Feb 11, 2025

I think the calculation of residualCovariance needs to go thru some parts of Kalman updater 🤔
oh but it does not seem to do that.. interesting

@andiwand
Copy link

not if you calculate the $\chi^2$ before you do the KF

@stephenswat stephenswat changed the title Add a distance-based cut to select measureemnts close to the track Add a distance-based cut to select measurements close to the track Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants