SegmentLinking / TrackLooper

Apache License 2.0
5 stars 14 forks source link

Bug fix for applying pT5 r-z chi2 #405

Closed YonsiG closed 5 months ago

YonsiG commented 5 months ago

We would like to apply r-z chi2 only for pT5 < 5GeV in the algorithm. However, the original check in the code is problematic: it uses the pixelRadius < 5*kR1GeVf, but the pixelRadius is not defined yet. Bug fix and the efficiency is improved by not applying r-z cut on high pt pT5 tracks.

YonsiG commented 5 months ago

/run standalone /run CMSSW

github-actions[bot] commented 5 months ago

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     44.2    322.6    124.1     48.8     97.3    545.5    134.6    157.8    105.6      1.8    1582.3     992.6+/- 264.1     430.9   explicit_cache[s=4] (master)
   avg     48.0    326.0    123.3     49.5     97.8    498.0    135.7    159.0    100.6      2.7    1540.5     994.5+/- 266.2     841.9   explicit_cache[s=4] (this PR)
YonsiG commented 5 months ago

this is the slides comparing the effect before and after the PR. https://indico.cern.ch/event/1410982/contributions/5930990/attachments/2860991/5005504/first%20study%20of%20cut%20difference%20at%20low%20and%20high%20pt.pdf Although the overall efficiency doesn't change much, the pT5 efficiency is increased, which means our LST objects are having "higher qualities". before bug fix: http://uaf-10.t2.ucsd.edu/~yagu/SDL_GPU_plots/T3_remove_redundant/pT3_pt_redef/new_def_PU200_NEVT-1_a9bf9a-PU200/compare/TC_base_0_0_eff_pt.html after bug fix: http://uaf-10.t2.ucsd.edu/~yagu/SDL_GPU_plots/high_pt_cuts/pT5_rz_low_pt_new_pt_def_PU200_NEVT-1_ac7ce5D-PU200/compare/TC_base_0_0_eff_pt.html

github-actions[bot] commented 5 months ago

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

YonsiG commented 5 months ago

Checking with a large sample, PU200 1000 events. Color codes are reverted with the CI check: red is after this PR, black is the master before. This is very hard to see the efficiency change for the highest pt bin.

Screenshot 2024-05-28 at 7 05 37 PM

The full comparison is linked here. http://uaf-10.t2.ucsd.edu/~yagu/SDL_GPU_plots/high_pt_cuts/large_samples_comparison_PR405_4d67cf-PU200_bf5196-PU200/compare/

slava77 commented 5 months ago

The full comparison is linked here. http://uaf-10.t2.ucsd.edu/~yagu/SDL_GPU_plots/high_pt_cuts/large_samples_comparison_PR405_1e1ebc-PU200_f2dfe2-PU200/compare/TC_eff_base_0.html

the same folder following up from this link apparently shows large changes in the fake rate

while the bot tests show no difference https://github.com/SegmentLinking/TrackLooper/pull/405#issuecomment-2125007216

Does the comparison include also the pt definition change? Is the idea that it doesn't affect the sim-pt-based efficiency plot?

YonsiG commented 5 months ago

Sorry, I forgot to rebase the master after the pt change PR get merged.New link here and also updated in previous comments http://uaf-10.t2.ucsd.edu/~yagu/SDL_GPU_plots/high_pt_cuts/large_samples_comparison_PR405_4d67cf-PU200_bf5196-PU200/compare/