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

Clean up T3T3 flag and add flag to allow triplet pLSs in TCs #358

Merged
merged 2 commits into from
Jan 10, 2024

Conversation

VourMa
Copy link
Contributor

@VourMa VourMa commented Dec 21, 2023

The -3 flag (T3T3EXTENSION in the Makefile) was deprecated in the code. I am removing any references to this option and I am reusing the same flag (-3) to add an option to include pLS triplets in the TC collection. This is not meant to be a flag to be used in the standalone (hence the physics and timing comparison below should not be interpreted as a proposal for standalone usage) but it has proven to be very useful for CMSSW configurations.

Physics performance
I am comparing the master branch with the updated branch with:

  • No -3 flag = no pLS triplet in TCs =>
    Same performance as the master: plots

  • No -3 flag = no pLS triplet in TCs =>
    Higher efficiency, fake and duplicate rate than the master, mostly coming from pLSs: plots

Timing performance

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg      1.7      1.4      0.6      2.5      2.9      0.6      1.0      0.7      1.5      0.1      13.1      10.8+/-  2.7      14.2   explicit_cache[s=1]
   avg      2.6      1.8      0.8      3.9      3.8      0.7      1.8      1.0      2.5      0.2      19.1      15.8+/-  3.9      10.2   explicit_cache[s=2]
   avg      5.4      2.5      1.5      6.3      6.1      0.9      3.8      2.1      4.5      0.5      33.6      27.3+/-  6.3       8.8   explicit_cache[s=4]
   avg      9.1      3.6      2.4      8.5      8.2      1.0      6.0      3.2      6.4      0.6      49.2      39.0+/-  9.4       8.5   explicit_cache[s=6]
   avg     13.4      4.6      3.0     10.4     10.7      1.3      8.0      4.1      7.7      0.8      64.0      49.4+/- 11.4       8.3   explicit_cache[s=8]
  • master @ d61593f nopLSTripletInTC
   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg      1.7      1.4      0.6      2.5      2.9      0.6      1.0      0.7      1.5      0.1      13.0      10.7+/-  2.6      13.5   explicit_cache[s=1]
   avg      2.8      1.8      0.8      3.7      3.8      0.7      1.7      1.0      2.3      0.2      18.7      15.3+/-  4.1      11.1   explicit_cache[s=2]
   avg      7.6      2.6      1.6      6.2      6.0      0.8      4.0      2.3      4.1      0.5      35.8      27.3+/-  6.2       9.3   explicit_cache[s=4]
   avg      9.5      3.7      2.4      8.4      8.0      1.0      5.6      3.2      6.3      0.6      48.7      38.2+/-  9.1       8.5   explicit_cache[s=6]
   avg     13.1      4.5      2.8     10.5     10.2      1.3      7.6      4.3      7.4      0.8      62.6      48.1+/-  8.5       8.1   explicit_cache[s=8]
   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg      1.7      1.5      0.6      2.5      2.9      0.6      1.0      0.7      1.6      0.1      13.1      10.8+/-  2.6      13.8   explicit_cache[s=1]
   avg      2.6      1.8      0.9      3.8      3.8      0.7      1.8      1.1      2.4      0.2      19.1      15.8+/-  3.9      10.3   explicit_cache[s=2]
   avg      5.4      2.5      1.6      6.3      6.0      0.9      3.9      2.1      4.5      0.4      33.4      27.2+/-  6.1       8.8   explicit_cache[s=4]
   avg      8.8      3.5      2.4      8.4      8.3      1.0      5.9      3.1      6.1      0.6      48.1      38.3+/-  7.8       8.3   explicit_cache[s=6]
   avg     13.3      4.3      3.1     10.3     10.9      1.4      7.9      4.1      7.7      0.8      64.0      49.3+/-  8.7       8.2   explicit_cache[s=8]

@ariostas
Copy link
Member

Do you think if checkHitspLS is also modified to include triplet pLSs then we could still get higher efficiency, but with fake rate and duplicate rate not increasing as much?

@VourMa
Copy link
Contributor Author

VourMa commented Dec 22, 2023

Do you think if checkHitspLS is also modified to include triplet pLSs then we could still get higher efficiency, but with fake rate and duplicate rate not increasing as much?

I have checked that and, unfortunately, that's not the case:
master vs. master with cleaned pLS triplets in TCs

@VourMa VourMa mentioned this pull request Dec 28, 2023
@ariostas
Copy link
Member

I fixed a minor formatting issue. Now that the CI is implemented, it will be clear when there are formatting issues in PRs.

All the changes look good to me and it works perfectly. It's a very impressive efficiency improvement, and I'm curious to see if some better cleanup can be done so that we can make it the default option. I'll approve the PR, but I'll wait a bit more to merge it since many people are on vacation and they might want to take a look when they come back. Thanks, @VourMa!

@ariostas
Copy link
Member

/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.

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.

@ariostas
Copy link
Member

I'll merge this now that people had more time to look at it and I was able to test the CI. Thank you @VourMa!

@ariostas ariostas merged commit 50d8a25 into master Jan 10, 2024
3 checks passed
@ariostas ariostas deleted the compileFlags branch January 30, 2024 20:13
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.

2 participants