LowPtElectrons: NanoAOD integration (back port of #33817) #33992

bainbrid · 2021-06-06T23:01:13Z

PR description:

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

Details on the updates are found in the PR description of #33817.

Nearly all developments are aimed at the nano v9 production.

Concerning future re-miniAOD campaigns, the LowPtElectronModifier module is used to calculate and embed the following information in pat::Electrons when the modifier logic (~bParking & run2_miniAOD_devel) is satisfied:

dxy w.r.t. the PV and the BS, and dz w.r.t. the PV
various userInts and userFloats concerning photon conversion information.

PR validation:

Local tests, and workflows 136.8523 (JetHT, 2018C, UL re-nano v9) and 136.88811 (JetHT, 2018D, UL re-mini).

if this PR is a backport please specify the original PR and why you need to backport that PR:

This is a back port of the PR #33817.

This PR is based on top of developments from the "re-mini" back port PR #33589. The resolution of conflicts in several files was required. A rebase is required as soon as #33589 is integrated within a 10_6_X release.

Some minor updates listed here #33992 (comment) have been forwarded ported to master in the PR #34005.

cmsbuild · 2021-06-06T23:01:36Z

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

It involves the following packages:

DataFormats/EgammaCandidates
PhysicsTools/NanoAOD
PhysicsTools/PatAlgos
RecoEgamma/EgammaElectronProducers
RecoEgamma/EgammaTools

@perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @jainshilpi, @hatakeyamak, @emilbols, @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

Backported from LowPtElectrons: NanoAOD integration #33817

bainbrid · 2021-06-06T23:01:46Z

@mariadalfonso @gouskos @crovelli @cavalari @jainshilpi @lfinco

bainbrid · 2021-06-06T23:09:20Z

RecoEgamma/EgammaElectronProducers/plugins/LowPtGsfElectronIDProducer.cc

  auto iter = std::find(names_.begin(), names_.end(), name);
  if (iter != names_.end()) {
    int index = std::distance(names_.begin(), iter);
    std::vector<float> inputs;
    if (version_.empty()) {  // Original XML model
      lowptgsfeleid::Features features;
-      features.set(ele, rho);
+      features.set(*ele, rho);


@slava77 I am required to provide support for an old model (Autumn18) in 10_6_X that no longer exists in master. The method above is defined here, which originally expected a const reco::GsfElectronRef&. Similar methods for newer models (see below this comment) have since been updated to expect an arg const reco::GsfElectron& and I have done the same for the above method (while preserving the original interface). Anther change is a switch from Ref to Ptr (which is then dereferenced). I believe the changes in this file and also here to be the minimal required while being consistent with other methods (e.g. here) and those in master.

bainbrid · 2021-06-06T23:13:20Z

PhysicsTools/NanoAOD/python/lowPtElectrons_cff.py

+# Modifiers
+################################################################################
+
+_modifiers = ( run2_miniAOD_80XLegacy |


@slava77 it's my understanding (after speaking with @mariadalfonso) that these modifiers are the only ones required to ensure the low pT ele are added to nanoAOD for run2_nanoAOD_106Xv2 only.

bainbrid · 2021-06-06T23:14:37Z

PhysicsTools/PatAlgos/python/slimming/slimmedLowPtElectrons_cfi.py

+from Configuration.Eras.Modifier_bParking_cff import bParking
+_slimmedLowPtElectrons = slimmedLowPtElectrons.clone()
+_slimmedLowPtElectrons.modifierConfig.modifications += [lowPtElectronModifier]
+(~bParking & run2_miniAOD_devel).toReplaceWith(slimmedLowPtElectrons,_slimmedLowPtElectrons)


@slava77 the dxy and dz vars, plus the conversions vars, are only intended to be added for future re-miniAOD campaigns.

slava77 · 2021-06-07T00:31:07Z

This branch has conflicts that must be resolved

bainbrid · 2021-06-07T06:41:36Z

This branch has conflicts that must be resolved

Ok, I'm not sure how I can resolve these conflicts... They are w.r.t. what exactly? (Is there more info?)
Perhaps I need to wait for the next IB (with #33589 integrated) and rebase...?

mariadalfonso · 2021-06-07T07:38:44Z

urgent

(mark what is needed for the nanov9)

bainbrid · 2021-06-07T09:49:28Z

@mariadalfonso do we know when there will be a new IB for 10_6_X? this will help, as #33589 will be integrated. then I can rebase and hopefully solve the above conflicts.

qliphy · 2021-06-07T09:59:56Z

@bainbrid You can watch here https://cmssdt.cern.ch/SDT/html/cmssdt-ib/#/ib/CMSSW_10_6_X
The next IB should be in a few hrs.

perrotta · 2021-06-07T10:00:08Z

@mariadalfonso do we know when there will be a new IB for 10_6_X? this will help, as #33589 will be integrated. then I can rebase and hopefully solve the above conflicts.

@bainbrid please keep an eye on http://cms-sw.github.io/showIB.html
Next IB should be CMSSW_10_6_X_2021-06-07-1100, and it will include #33589

bainbrid · 2021-06-07T10:09:26Z

This branch has conflicts that must be resolved

I think I've realised my mistake. Might have screwed up when squashing prior to making the PR. Testing now and then a commit soon that hopefully fixes it ...

cmsbuild · 2021-06-07T10:31:04Z

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

cmsbuild · 2021-06-07T10:35:27Z

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

cmsbuild · 2021-06-07T10:39:15Z

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

perrotta · 2021-06-07T10:40:33Z

backport of #33817

cmsbuild · 2021-06-07T20:07:07Z

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

mariadalfonso · 2021-06-07T20:10:29Z

please test

bainbrid · 2021-06-07T22:20:28Z

@mariadalfonso @gouskos this commit 7d72447 will be "forward ported" in a new PR to master today.

PR to master here: #34005

cmsbuild · 2021-06-07T23:59:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4de9ab/15735/summary.html
COMMIT: ce41e4b
CMSSW: CMSSW_10_6_X_2021-06-07-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33992/15735/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

[backport 106X] Adding UL16 PUID and fixing QGL values in nanoAOD #33964 @alefisico: [backport 106X] Adding UL16 PUID and fixing QGL values in nanoAOD

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4de9ab/15735/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4de9ab/15735/git-merge-result

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 1 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 3215543
DQMHistoTests: Total failures: 4
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3215205
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 28.645 KiB( 34 files compared)
DQMHistoSizes: changed ( 1325.81 ): 17.282 KiB Physics/NanoAODDQM
DQMHistoSizes: changed ( 136.8523 ): 11.363 KiB Physics/NanoAODDQM
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: found differences in 3 / 34 workflows

slava77 · 2021-06-08T00:20:34Z

Concerning future re-miniAOD campaigns, the LowPtElectronModifier module is used to calculate and embed the following information in pat::Electrons when the modifier logic (~bParking & run2_miniAOD_devel) is satisfied:

shouldn't this be actually enabled for the bParking & UL as well?
Is this dropped from the needed features for bParking miniAOD, or is there going to be another PR on top of this one?

slava77 · 2021-06-08T00:24:21Z

+reconstruction

for #33992 ce41e4b

code changes are in line with the PR description and the follow up review
- reco code is consistent with the 120X version LowPtElectrons: NanoAOD integration #33817
- the modifications in slimmedLowPtElectrons are enabled only for non-production run2_miniAOD_devel setup; somehow I thought that the preference was to still add it for bParking & run2_miniAOD_UL setup
jenkins tests pass and comparisons with the baseline show no relevant differences

mariadalfonso · 2021-06-08T08:20:43Z

+xpog

nano consistent with 10_2 LowPtElectrons: NanoAOD integration #33817 and LowPtElectrons: NanoAOD integration (forward port) #34005
more test will be done when merged (take too long to compile)
un2_nanoAOD_106Xv1 (nanoV8) are safe because the product is only added for the un2_nanoAOD_106Xv2

cmsbuild · 2021-06-08T08:21:02Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_0_X is complete. 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 · 2021-06-08T08:48:32Z

+1

bainbrid · 2021-06-08T12:52:13Z

Concerning future re-miniAOD campaigns, the LowPtElectronModifier module is used to calculate and embed the following information in pat::Electrons when the modifier logic (~bParking & run2_miniAOD_devel) is satisfied:

shouldn't this be actually enabled for the bParking & UL as well?
Is this dropped from the needed features for bParking miniAOD, or is there going to be another PR on top of this one?

@slava77 you're absolutely right, thanks for catching this! Added in a separate back port PR: #34023

bainbrid added 2 commits June 7, 2021 00:37

final energy regression and ID for UL re-miniAOD

60e013e

add nanoAOD table for low pT electrons

6c93092

cmsbuild added this to the CMSSW_10_6_X milestone Jun 6, 2021

cmsbuild added orp-pending pending-signatures reconstruction-pending tests-pending xpog-pending labels Jun 6, 2021

bainbrid commented Jun 6, 2021

View reviewed changes

mariadalfonso mentioned this pull request Jun 7, 2021

features for V9 nanoAOD cms-nanoAOD/cmssw#555

Closed

55 tasks

cmsbuild added the urgent label Jun 7, 2021

lowPtElectrons: cfi --> cff, consistent with cms-sw#33589

f6b05b9

Merge branch 'CMSSW_10_6_X' into LowPtElectrons_nanoAOD_integration_106X

0f4abef

lowPtElectrons: cfi-->cff

adc59ec

cmsbuild added the backport-ok label Jun 7, 2021

cmsbuild added the tests-rejected label Jun 7, 2021

bainbrid added 2 commits June 7, 2021 22:05

updatedLowPtElectrons: add missing params

bda1a88

slava review

ce41e4b

cmsbuild added tests-pending and removed tests-rejected labels Jun 7, 2021

cmsbuild added tests-started and removed tests-pending labels Jun 7, 2021

bainbrid mentioned this pull request Jun 7, 2021

LowPtElectrons: NanoAOD integration (forward port) #34005

Merged

cmsbuild added tests-approved and removed tests-started labels Jun 7, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jun 8, 2021

cmsbuild added fully-signed xpog-approved and removed pending-signatures xpog-pending labels Jun 8, 2021

cmsbuild added orp-approved and removed orp-pending labels Jun 8, 2021

cmsbuild merged commit d81c275 into cms-sw:CMSSW_10_6_X Jun 8, 2021

bainbrid mentioned this pull request Jun 8, 2021

LowPtElectrons: embed extra info in pat::Electron for BParking UL #34023

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LowPtElectrons: NanoAOD integration (back port of #33817) #33992

LowPtElectrons: NanoAOD integration (back port of #33817) #33992

bainbrid commented Jun 6, 2021 •

edited

cmsbuild commented Jun 6, 2021 •

edited

bainbrid commented Jun 6, 2021

bainbrid Jun 6, 2021

bainbrid Jun 6, 2021

bainbrid Jun 6, 2021

slava77 commented Jun 7, 2021

bainbrid commented Jun 7, 2021

mariadalfonso commented Jun 7, 2021

bainbrid commented Jun 7, 2021

qliphy commented Jun 7, 2021

perrotta commented Jun 7, 2021

bainbrid commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

perrotta commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

mariadalfonso commented Jun 7, 2021

bainbrid commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

slava77 commented Jun 8, 2021

slava77 commented Jun 8, 2021

mariadalfonso commented Jun 8, 2021 •

edited

cmsbuild commented Jun 8, 2021

qliphy commented Jun 8, 2021

bainbrid commented Jun 8, 2021

LowPtElectrons: NanoAOD integration (back port of #33817) #33992

LowPtElectrons: NanoAOD integration (back port of #33817) #33992

Conversation

bainbrid commented Jun 6, 2021 • edited

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Jun 6, 2021 • edited

bainbrid commented Jun 6, 2021

bainbrid Jun 6, 2021

Choose a reason for hiding this comment

bainbrid Jun 6, 2021

Choose a reason for hiding this comment

bainbrid Jun 6, 2021

Choose a reason for hiding this comment

slava77 commented Jun 7, 2021

bainbrid commented Jun 7, 2021

mariadalfonso commented Jun 7, 2021

bainbrid commented Jun 7, 2021

qliphy commented Jun 7, 2021

perrotta commented Jun 7, 2021

bainbrid commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

perrotta commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

mariadalfonso commented Jun 7, 2021

bainbrid commented Jun 7, 2021

cmsbuild commented Jun 7, 2021

Comparison Summary

slava77 commented Jun 8, 2021

slava77 commented Jun 8, 2021

mariadalfonso commented Jun 8, 2021 • edited

cmsbuild commented Jun 8, 2021

qliphy commented Jun 8, 2021

bainbrid commented Jun 8, 2021

bainbrid commented Jun 6, 2021 •

edited

cmsbuild commented Jun 6, 2021 •

edited

mariadalfonso commented Jun 8, 2021 •

edited