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

modify electron track seeding regions for pp_on_AA era #24413

Merged
merged 2 commits into from Sep 1, 2018

Conversation

bi-ran
Copy link
Contributor

@bi-ran bi-ran commented Aug 30, 2018

this PR switches the tracking regions for the pixel pair/triplet and strip pair electron track seeding modules to regions centred on vertices. the ptmin threshold in the seeding regions for all three modules are also raised to 8 GeV. this improves the timing of the stripPairElectronSeeds module by a factor of 6-7, while also slightly improving the efficiency and fake rate. a short set of slides comparing the performance before/after this change can be found at [1].

[[1]]: https://twiki.cern.ch/twiki/pub/CMS/HiEgamma2018/electron-strippair-seeding.pdf

@mandrenguyen

pixel pairs/triplets, strip pairs
- switch to tracking region with vertices (firststepprimaryvertices) if
not already using it
- increase ptmin to 4.0 across the board
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @bi-ran for master.

It involves the following packages:

RecoTracker/IterativeTracking

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @gpetruc, @ebrondol, @dgulhan this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Aug 30, 2018

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 30, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30137/console Started: 2018/08/30 02:15

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24413/30137/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3143879
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3143675
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 133 log files, 14 edm output root files, 32 DQM output files

@slava77
Copy link
Contributor

slava77 commented Aug 30, 2018

Reco comparison results: 0 differences found in the comparisons

the tests include wf 158.0.
However it has only 1 event.
Is the effect expected to be small at general event level?

@bi-ran
Copy link
Contributor Author

bi-ran commented Aug 30, 2018

@slava77 yes, changes are expected to be small for min bias events where there are few electrons, since supercluster thresholds are now at 15 GeV.

@slava77
Copy link
Contributor

slava77 commented Aug 31, 2018

Some observations

  • in "filtered 3000" HI minBias sample of 230 events
    • there are no significant change in electrons/photons
    • there is a fairly significant reduction in CPU use in electron seeding: the combined decrease is about 15 s/event, reducing the production-like job total time to 142 s/evt on 230 events
  • Z->ee/mm sample with HI minbias mix ("pyquen" based on what's in wf 302.0 with beam energy 5020 as for this year and with reco using pp_on_AA)
    • about a factor of 2 reduction in electron gsf tracks below pt of 10 GeV (expected)
      all_sign1037vsorig_pyquenzeemumujetspprecoin2018wf302p18c_log10recogsftracks_electrongsftracks__reco_obj_pt
    • changes in the reconstructed electrons are fairly small due to the pt threshold
      pf electrons:
      all_sign1037vsorig_pyquenzeemumujetspprecoin2018wf302p18c_log10recopfcandidates_particleflow__reco_obj_p56
      ged electrons
      all_sign1037vsorig_pyquenzeemumujetspprecoin2018wf302p18c_recogsfelectrons_gedgsfelectrons__reco_obj_classification

the MTV fake rate plot shows a decrease of about 30% in fakes
wf302 18_gsf_mtv_fr_eta

@slava77
Copy link
Contributor

slava77 commented Aug 31, 2018

+1

for #24413 66ef5db

  • code changes are in line with the PR description
  • jenkins tests pass and comparisons with the baseline show no differences (only 1 event is tested in pp_on_AA workflow)
  • local tests show roughly expected behavior with a significant reduction of CPU time spent in electron seeding (which was substantially below the threshold of the electron reco in the baseline).

@cmsbuild
Copy link
Contributor

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@kpedro88
Copy link
Contributor

kpedro88 commented Sep 1, 2018

+1

@cmsbuild cmsbuild merged commit a8f27c9 into cms-sw:master Sep 1, 2018
@bi-ran bi-ran deleted the eleseedingregions_pp_on_AA branch April 4, 2019 22:37
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.

None yet

4 participants