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

Enable pt-dependent JER-SF and phi-dependent JEC application in PAT [106X backport] #28097

Merged
merged 2 commits into from
Oct 29, 2019

Conversation

ahinzmann
Copy link
Contributor

PR description:

Enable pt-dependent jet resolution scale factors in PAT. This is needed to apply Autumn18_V7 JER-SF, released last week.
Enable phi-dependent jet energy scale in PAT. This will be needed for 2018 JEC to deal with the HEM issue.
This PR has no impact with the current global tags in any release. However, it will enable using a new global tag (currently in the makings) with pt-dependent JER-SFs and later with phi-dependent JECs.

A test config to apply resolution smearing from a db file has been added under /test.
Here is an example db-file containing pt-dependent JER-SFs:
https://github.com/cms-jet/JRDatabase/blob/master/SQLiteFiles/Autumn18_V7_MC.db

PR validation:

Test config runs after inclusion of this PR. It was checked that this does not change the memory consumption, compared to using the old code with Autumn18_V1_MC.db (no pt-dependence).

if this PR is a backport please specify the original PR:

Backport of #28096

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 1, 2019

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

It involves the following packages:

JetMETCorrections/Modules
PhysicsTools/PatAlgos
PhysicsTools/PatUtils

@perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@emilbols, @gouskos, @rappoccio, @HeinerTholen, @peruzzim, @seemasharmafnal, @mmarionncern, @smoortga, @acaudron, @jdolen, @ferencek, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @andrzejnovak, @clelange, @riga, @JyothsnaKomaragiri, @mverzett, @gpetruc, @mariadalfonso, @pvmulder 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

@ahinzmann ahinzmann changed the title Enable pt-dependent JER and phi-dependent JEC application in PAT [106X backport] Enable pt-dependent JER-SF and phi-dependent JEC application in PAT [106X backport] Oct 1, 2019
@slava77
Copy link
Contributor

slava77 commented Oct 3, 2019

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2770/console Started: 2019/10/03 11:54

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2019

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3212305
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3211970
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 4, 2019

Pull request #28097 was updated. @perrotta, @cmsbuild, @santocch, @slava77 can you please check and sign again.

@fabiocos
Copy link
Contributor

fabiocos commented Oct 6, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 6, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2805/console Started: 2019/10/06 16:14

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 6, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 6, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 6, 2019

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3212305
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3211970
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

Copy link
Contributor

@slava77 slava77 left a comment

Choose a reason for hiding this comment

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

no need to follow up on these; I'm leaving the notes here for the record of a tiny inconsistency in the backport

{{JME::Binning::JetPt, jet.pt()}, {JME::Binning::JetEta, jet.eta()}, {JME::Binning::Rho, *rho}});
double jer_sf = resolution_sf.getScaleFactor({{JME::Binning::JetPt, jet.pt()}, {JME::Binning::JetEta, jet.eta()}},
m_systematic_variation);
if (m_debug) {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: the master version has an empty line above if (m_debug) {

res_sf.getScaleFactor({{JME::Binning::JetPt, jet.pt()}, {JME::Binning::JetEta, jet.eta()}}, Variation::UP);
float sf_down =
res_sf.getScaleFactor({{JME::Binning::JetPt, jet.pt()}, {JME::Binning::JetEta, jet.eta()}}, Variation::DOWN);
if (m_debug) {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: the master version has an empty line above if (m_debug) {

@slava77
Copy link
Contributor

slava77 commented Oct 8, 2019

backport of #28096
this is more of a copy-paste from the master version though; it includes clang-format changes on top of #28096

@slava77
Copy link
Contributor

slava77 commented Oct 8, 2019

+1

for #28097 ce23314

@santocch
Copy link

santocch commented Oct 9, 2019

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 9, 2019

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_11_0_X is complete. 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)

@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 20bcf6a into cms-sw:CMSSW_10_6_X Oct 29, 2019
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