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

LowPtElectrons: NanoAOD integration #33817

Merged

Conversation

bainbrid
Copy link
Contributor

@bainbrid bainbrid commented May 21, 2021

PR description:

This PR adds low-pT electrons to the NanoAOD content.

  • updates the LowPtGsfElectronIDProducer to handle both reco and pat electrons.
  • adds a low-pT electron (and related MC) table with a subset of the usual EGamma variables
  • schedules modules to:
    • perform energy regression
    • run the latest ID (2020Nov28) on-the-fly
    • determine PF relative isolation
  • adds the scores for the unbiased and pT-biased ElectronSeed BDTs
  • adds the scores for the embedded ID (2020Aug15) and on-the-fly ID (2020Nov28)
  • adds plots to DQM file
  • adds algorithm (and modifiers) to identify electrons from conversions
  • embeds conversion info as userFloats/Ints and add flags to table and DQM
  • adds protection for PF electrons when running energy regression on top of pat::Electrons
  • modifies to give empty sequences for various eras to prevent relvals failures
  • add conversion vertex radius as a float (precision 7)
  • recomputes dxy and dxyErr in , and embeds in pat::Electron (using setDB)

These changes increase the nanoAOD event size from 2.32 kB/event to 2.37 kB/event, an increase of 2.2%. Tested with Run2018C/JetHT/MINIAOD/UL2018_MiniAODv2-v1 as input.

PR validation:

Local tests, and workflow 136.8523 (JetHT, 2018C, UL re-nano v9).

PR status:

  • The plan is to back port this PR to 10_6_X in time for the re-nanoAOD v9 campaign

@bainbrid
Copy link
Contributor Author

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33817/22813

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @bainbrid for master.

It involves the following packages:

PhysicsTools/NanoAOD
PhysicsTools/PatAlgos
RecoEgamma/EgammaElectronProducers

@perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso can you please review it and eventually sign? Thanks.
@emilbols, @gouskos, @jainshilpi, @hatakeyamak, @rappoccio, @mbluj, @varuns23, @seemasharmafnal, @mmarionncern, @ahinzmann, @lgray, @jdolen, @ferencek, @Sam-Harper, @rovere, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @andrzejnovak, @clelange, @swertz, @swozniewski, @JyothsnaKomaragiri, @sobhatta, @lecriste, @afiqaize, @gpetruc, @mariadalfonso, @ram1123 this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@bainbrid
Copy link
Contributor Author

@mariadalfonso @gouskos

  • I still need to add some small changes to protect the E/p calculation against missing corrections structures for very low-pT PF electrons. (@jainshilpi @cavalari)
  • I wish to add information (i.e. at least a veto) relating to conversions.
  • Ideally, I would provide some "recommended" ID working points in the form of a bit mask, but I might delay this to a later iteration.
  • The Dxy variable appears to be broken, this needs investigating.

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8696d/15253/summary.html
COMMIT: 5acde33
CMSSW: CMSSW_12_0_X_2021-05-21-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33817/15253/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8696d/15253/llvm-analysis/cmsclangtidy.txt for details.

Unit Tests

I found errors in the following unit tests:

---> test runtestPhysicsToolsNanoAOD had ERRORS

RelVals

----- Begin Fatal Exception 22-May-2021 11:16:58 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CandMCMatchTableProducer label='lowPtElectronMCTable'
Exception Message:
Unsupported objType 'LowPtElectron'
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 22-May-2021 11:25:32 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CandMCMatchTableProducer label='lowPtElectronMCTable'
Exception Message:
Unsupported objType 'LowPtElectron'
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 22-May-2021 11:30:15 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CandMCMatchTableProducer label='lowPtElectronMCTable'
Exception Message:
Unsupported objType 'LowPtElectron'
----- End Fatal Exception -------------------------------------------------
Expand to see more relval errors ...

RelVals-INPUT

  • 136.7722136.7722_RunJetHT2016H_nano+RunJetHT2016H_nano+NANOEDM2016_80X+HARVESTNANOAOD2016_80X/step2_RunJetHT2016H_nano+RunJetHT2016H_nano+NANOEDM2016_80X+HARVESTNANOAOD2016_80X.log
  • 136.7952136.7952_RunJetHT2017C_94Xv2NanoAODINPUT+RunJetHT2017C_94Xv2NanoAODINPUT+NANOEDM2017_94XMiniAODv2+HARVESTNANOAOD2017_94XMiniAODv2/step2_RunJetHT2017C_94Xv2NanoAODINPUT+RunJetHT2017C_94Xv2NanoAODINPUT+NANOEDM2017_94XMiniAODv2+HARVESTNANOAOD2017_94XMiniAODv2.log
  • 136.8521136.8521_RunJetHT2018A_nano+RunJetHT2018A_nano+NANOEDM2018_102Xv1+HARVESTNANOAOD2018_102Xv1/step2_RunJetHT2018A_nano+RunJetHT2018A_nano+NANOEDM2018_102Xv1+HARVESTNANOAOD2018_102Xv1.log
Expand to see more relval errors ...

@bainbrid
Copy link
Contributor Author

@mariadalfonso @Goukas latest commits:

  • added algorithm (and modifiers) to identify electrons from conversions and
  • embed conversion info as userFloats/Ints and add flags to table and DQM
  • set computeMiniIso = false, and related defaults (as it is redundant given EleIsoValueMapProducer)
  • added lowPtElectronsMCMatchForTableAlt and associated modules

Please don't test quite yet - more commits to come.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33817/22924

@cmsbuild
Copy link
Contributor

Pull request #33817 was updated. @perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso can you please check and sign again.

@bainbrid bainbrid force-pushed the LowPtElectrons_nanoAOD_integration branch from 52a0c7b to ed93503 Compare May 28, 2021 16:03
@bainbrid
Copy link
Contributor Author

bainbrid commented Jun 5, 2021

@slava77 acted on your most recent suggestions, squashed, and force pushed.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33817/23096

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2021

Pull request #33817 was updated. @perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Jun 5, 2021

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8696d/15679/summary.html
COMMIT: 2436a07
CMSSW: CMSSW_12_0_X_2021-06-04-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33817/15679/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8696d/15679/llvm-analysis/cmsclangtidy.txt for details.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 100 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2650486
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2650451
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 21.428 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 1325.81 ): 12.905 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 136.8523 ): 8.519 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: found differences in 7 / 36 workflows

@slava77
Copy link
Contributor

slava77 commented Jun 5, 2021

+reconstruction

for #33817 2436a07

  • code changes are in line with the PR description and the follow up review
    • apart for changes in nanoAOD, there are changes in miniAOD slimmedLowPtElectrons
  • jenkins tests pass and comparisons with the baseline show differences in slimmedLowPtElectrons (BS2D, PV2D PVDZ are now filled; and there are more userInts and Floats corresponding to the added conversion-related data); the root product checks show lowPtElectronTable and lowPtElectronMCTable appearing in nanoAOD as expected

@mariadalfonso
Copy link
Contributor

+xpog

new collection lowPtElectron added to the nanoV9 only inline with PR description.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 6, 2021

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Jun 6, 2021

+1

@mariadalfonso
Copy link
Contributor

on ZtauTau MC samples the size increase is ~ 3%

Branch documentation here:
Screen Shot 2021-06-07 at 14 48 31

DQM plots are here:
Screen Shot 2021-06-07 at 14 59 03
Screen Shot 2021-06-07 at 14 58 42

cmsbuild added a commit that referenced this pull request Jun 8, 2021
…ation_106X

LowPtElectrons: NanoAOD integration (back port of #33817)
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

7 participants