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 (forward port) #34005

Merged

Conversation

bainbrid
Copy link
Contributor

@bainbrid bainbrid commented Jun 7, 2021

PR description:

This PR adds some minor fixes to the python config that steers the addition of low-pT electrons to the NanoAOD content, done in #33817.

The same minor fixes are already included in the back port PR #33992 to 10_6_X.

This PR ensures consistency between 10_6_X and master.

The changes are as follows:

  • the LeptonUpdater module was mistakenly removed from PhysicsTools/NanoAOD/python/lowPtElectrons_cff.py in an earlier simplification to LowPtElectrons: NanoAOD integration #33817; here it is added back with the param computeMiniIso=True, required to correctly compute the variables miniPFRelIso_chg and miniPFRelIso_all.
  • the low-pT nano DQM plots have been updated: miniPFRelIso_chg and miniPFRelIso_all have a minor axis range tweak; ip3d and sip3d have been removed as they are not computed.

PR validation:

Local tests with wfs 1325.81, 136.8523, and 136.88811.

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

These changes are already included in the back port PR #33992 to 10_6_X.

… DQM: fix miniPFRelIso_chg/all, remove s/ip3d histos
@bainbrid
Copy link
Contributor Author

bainbrid commented Jun 7, 2021

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34005/23127

  • This PR adds an extra 24KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2021

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

It involves the following packages:

PhysicsTools/NanoAOD

@cmsbuild, @mariadalfonso, @gouskos, @fgolf can you please review it and eventually sign? Thanks.
@gpetruc, @swertz 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

@gouskos
Copy link
Contributor

gouskos commented Jun 7, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-39b0b1/15738/summary.html
COMMIT: 5ff103d
CMSSW: CMSSW_12_0_X_2021-06-07-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34005/15738/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2648335
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2648306
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -1.255 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 1325.81 ): -0.759 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 136.8523 ): -0.500 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

@mariadalfonso
Copy link
Contributor

This PR fix the empty variables as reported in
#33817 (comment)

The miniPFRelIso_chg and all are now filled
Screen Shot 2021-06-08 at 09 31 12

@mariadalfonso
Copy link
Contributor

The DQM plots are now saved for all possible nanos, altought empty for when the lowPtElectron doesn't run.
in principle can be customized in PhysicsTools/NanoAOD/python/nanoDQM_cff.py
with the same set of modifiers used in
https://github.com/cms-sw/cmssw/pull/33817/files#diff-31eff23ea52a2b619e0192f0f8d2e89b7e1740522a692066a66284fc43d547f5R195-R203

@mariadalfonso
Copy link
Contributor

urgent

(mark what is needed for the nanov9)

@cmsbuild cmsbuild added the urgent label Jun 8, 2021
doc="mini PF relative isolation, charged component"),
miniPFRelIso_all = Var("userFloat('miniIsoAll')/pt",float,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these relative isolation or not ?
In this last PR the division for the pt is removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they are rel. iso. even if the name suggests not. So the /pt is no longer needed. This is now done here.

@mariadalfonso
Copy link
Contributor

one more things we can do for master is to clean these warnings
https://github.com/cms-sw/cmssw/pull/33817/files#r645094126

@bainbrid
Copy link
Contributor Author

bainbrid commented Jun 8, 2021

one more things we can do for master is to clean these warnings
https://github.com/cms-sw/cmssw/pull/33817/files#r645094126

I should fix this in this PR?

@mariadalfonso
Copy link
Contributor

one more things we can do for master is to clean these warnings
https://github.com/cms-sw/cmssw/pull/33817/files#r645094126

I should fix this in this PR?

yes, fine for me

@bainbrid
Copy link
Contributor Author

bainbrid commented Jun 8, 2021

one more things we can do for master is to clean these warnings
https://github.com/cms-sw/cmssw/pull/33817/files#r645094126

I should fix this in this PR?

yes, fine for me

after discussing privately, we've agreed to factorise this non-essential fix into a separate PR.

And keep this PR is nano-specific.

@mariadalfonso
Copy link
Contributor

The DQM plots are now saved for all possible nanos, altought empty for when the lowPtElectron doesn't run.
in principle can be customized in PhysicsTools/NanoAOD/python/nanoDQM_cff.py
with the same set of modifiers used in
https://github.com/cms-sw/cmssw/pull/33817/files#diff-31eff23ea52a2b619e0192f0f8d2e89b7e1740522a692066a66284fc43d547f5R195-R203

we agreed to defer this update to another PR in master

@mariadalfonso
Copy link
Contributor

+xpog

  • this PR fix two variables spotted to be broken
  • the feature is already in 10_6 for nanov9

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 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 8, 2021

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

None yet

5 participants