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

New l1 track trigger stub window tune #43260

Merged
merged 4 commits into from
Dec 8, 2023

Conversation

rgoldouz
Copy link
Contributor

I have performed various studies related to the stub rate and stub reconstruction efficiencies. As a results I have made a new stub window tune. All details are documented in the following detector note (CMS DN-2020/005 -- The L1 Track Trigger Upgrade: Properties, Efficiencies, and Rates for Track Stubs)
the only updated file is the following: L1Trigger/TrackTrigger/python/TTStubAlgorithmRegister_cfi.py

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43260/37649

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rgoldouz (Reza Goldouzian) for master.

It involves the following packages:

  • L1Trigger/TrackTrigger (l1, upgrade)

@epalencia, @srimanob, @AdrianoDee, @aloeliger, @cmsbuild can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @skinnari, @sviret, @missirol, @erikbutz this is something you requested to watch as well.
@sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43260/37651

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

Pull request #43260 was updated. @cmsbuild, @srimanob, @epalencia, @AdrianoDee, @aloeliger can you please check and sign again.

@aloeliger
Copy link
Contributor

@artlbv & @mcepeda Do you need to be informed on this one for physics changes?

@aloeliger
Copy link
Contributor

please test

@artlbv
Copy link
Contributor

artlbv commented Nov 15, 2023

@artlbv & @mcepeda Do you need to be informed on this one for physics changes?

thanks for referencing us! If I understand correctly, this change affects all the objects relying on the TrackTrigger?
Thus it is actually more general than menu only, and e.g. @EmyrClement @cerminar @folguera as object contacts need to be aware? @mcepeda what is the procedure for validating these changes from the DPG side?

I looked at the DN and I see it contains some measurements for "Electrons" and "Muons", though I am not sure which exact objects were used. @rgoldouz could you clarify?

@rgoldouz
Copy link
Contributor Author

@artlbv & @mcepeda Do you need to be informed on this one for physics changes?

thanks for referencing us! If I understand correctly, this change affects all the objects relying on the TrackTrigger? Thus it is actually more general than menu only, and e.g. @EmyrClement @cerminar @folguera as object contacts need to be aware? @mcepeda what is the procedure for validating these changes from the DPG side?

I looked at the DN and I see it contains some measurements for "Electrons" and "Muons", though I am not sure which exact objects were used. @rgoldouz could you clarify?

Hi @artlbv (let's add @mdhildreth to the loop),

we have developed this new tune based on the stub properties in ttbar, single electron and single muon samples with PU200 (rate, efficiencies,...) and then tested its effect on the electron, muon, displaced muon, and general track efficiency. The previous and new tunes have similar efficiencies/features for track reconstruction while the new tune leads to lower stub rate.

please let me know if you have any further question. I have attached our updated version of note that will be appear on ICMS soon.

Thanks,
Reza

DN-20-005_temp.pdf

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8a2166/35841/summary.html
COMMIT: 8ae84cc
CMSSW: CMSSW_14_0_X_2023-11-15-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43260/35841/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 271 lines from the logs
  • Reco comparison results: 136 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3363070
  • DQMHistoTests: Total failures: 2306
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3360742
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 214 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@aloeliger
Copy link
Contributor

@mcepeda @EmyrClement @cerminar @folguera as far as I am concerned, this is fine from a software perspective, I would like to hear one of you comment on this from a physics one before it is signed on.

@rgoldouz
Copy link
Contributor Author

Hi @mcepeda @EmyrClement @cerminar @folguera @aloeliger @skinnari @tomalin ,
I have attached my slides for comparison of the track efficiencies between the old and new tunes using cmssw13.
Reza_7NoV2023.pdf

More comparison plots can be found in the following link
https://rgoldouz.web.cern.ch/rgoldouz/MyPlots/L1tracker/2023-11-12/

Thanks,
Reza

@cerminar
Copy link
Contributor

@rgoldouz thanks for the plots! In general the electron eff. seems to be ok.
Any reason why you only compared the PU0 electrons? Also, do you have the eff. vs pt separately for barrel and endcap?
Thanks!

@EmyrClement
Copy link
Contributor

@rgoldouz thanks for the comparison - can I just check that the tracker geometry in the D76 geometry this stub window tune was derived on is the same as what's in D88/D95, which is the geometry we use in most of the L1T samples?

@rgoldouz
Copy link
Contributor Author

Hi cerminar,
It is just for simplicity since its faster to run on the PU0 samples. But in our studies we showed that the PU200 does not change the electron/muon stub efficiencies. For the plots, I use the official code for generating trk efficiency plot (L1TrackNtuplePlot.C) and all plots can be found in https://rgoldouz.web.cern.ch/rgoldouz/MyPlots/L1tracker/2023-11-12/ where only eta distribution can show what you want. But in our AN you can find stub efficiencies for barrel and endcap in detail.

Hi @EmyrClement , As you mentioned we have developed/tested/validated the new tun using D76 geometry. In order to make sure that it works in D88, I reran the L1trk efficiency code and I found consistent results https://rgoldouz.web.cern.ch/rgoldouz/MyPlots/L1tracker/2023-11-12/. I have not tested it for D95.

Thanks,
-Reza

@aloeliger
Copy link
Contributor

@EmyrClement & @cerminar I am leaving this PR until you are satisfied by it, otherwise the changes seem trivial enough to me. Let me know if when you think it is ready.

@cerminar
Copy link
Contributor

@aloeliger, as far as I can tell, I see no showstoppers for electrons.

@EmyrClement
Copy link
Contributor

This if fine from my side. Just to confirm what @rgoldouz reported on the geometries, digging through the versions here and for an older CMSSW release here, I see the D76/88/95 geometries used the T21/24/31 tracker geometry, which correspond to identical layouts (from the descriptions of those geometries).

@aloeliger
Copy link
Contributor

+l1

@srimanob
Copy link
Contributor

srimanob commented Dec 5, 2023

@rgoldouz
The current baseline is D98, have you check it? The change is from T31 to T32. Thx.

@rgoldouz
Copy link
Contributor Author

rgoldouz commented Dec 6, 2023

Hi @srimanob ,

I have checked it for D76 and D88. The tune is similar to the previous tune with some small modifications. So it should work also for D98 and later geometries.

Thanks,
-Reza

@srimanob
Copy link
Contributor

srimanob commented Dec 6, 2023

+Upgrade

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2023

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

@rappoccio
Copy link
Contributor

+1

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

Successfully merging this pull request may close these issues.

8 participants