Skip to content
This repository has been archived by the owner on Dec 9, 2024. It is now read-only.

Bug fix for applying pT5 r-z chi2 #405

Merged
merged 2 commits into from
May 29, 2024
Merged

Bug fix for applying pT5 r-z chi2 #405

merged 2 commits into from
May 29, 2024

Conversation

YonsiG
Copy link
Contributor

@YonsiG YonsiG commented May 22, 2024

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
Copy link
Contributor Author

YonsiG commented May 22, 2024

/run standalone
/run CMSSW

Copy link

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
Copy link
Contributor Author

YonsiG commented May 22, 2024

Copy link

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.

Copy link
Contributor

@slava77 slava77 left a comment

Choose a reason for hiding this comment

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

The code looks OK.
However, the comparisons are suggestive that there may be a loss in the total efficiency, which is not really expected since the selections are less strict with this PR>

As discussed yesterday the plan is to check on a larger sample.

@YonsiG
Copy link
Contributor Author

YonsiG commented May 28, 2024

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
Copy link
Contributor

slava77 commented May 28, 2024

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
#405 (comment)

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

@YonsiG
Copy link
Contributor Author

YonsiG commented May 29, 2024

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/

@slava77 slava77 merged commit f93339a into master May 29, 2024
3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants