Navigation Menu

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

Add missing modifier: run2_nanoAOD_106Xv2 for MET significance #34096

Merged
merged 1 commit into from Jun 12, 2021

Conversation

gouskos
Copy link
Contributor

@gouskos gouskos commented Jun 11, 2021

Add missing modifier in 106X relevant for nanoAOD-v9 production.
This makes the MET covariance matrix and MET significance to be identical between 106Xv2 and master [120X] - as they should be

PR validation:

runTheMatrix.py -l 136.8523

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

RecoMET/METProducers

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @ahinzmann, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @clelange, @schoef, @mariadalfonso, @seemasharmafnal 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 gouskos changed the title Add missing modifier: run2_nanoAOD_106Xv1 Add missing modifier: run2_nanoAOD_106Xv2 Jun 11, 2021
@gouskos gouskos changed the title Add missing modifier: run2_nanoAOD_106Xv2 Add missing modifier: run2_nanoAOD_106Xv2 for MET significance Jun 11, 2021
@gouskos
Copy link
Contributor Author

gouskos commented Jun 11, 2021

please test

@mariadalfonso
Copy link
Contributor

urgent

(bug fix for nanov9)

(run2_miniAOD_UL|run2_nanoAOD_106Xv1).toModify(METSignificanceParams_Data, useDeltaRforFootprint = True)
from Configuration.Eras.Modifier_run2_nanoAOD_106Xv2_cff import run2_nanoAOD_106Xv2
(run2_miniAOD_UL|run2_nanoAOD_106Xv1|run2_nanoAOD_106Xv2).toModify(METSignificanceParams, useDeltaRforFootprint = True)
(run2_miniAOD_UL|run2_nanoAOD_106Xv1|run2_nanoAOD_106Xv2).toModify(METSignificanceParams_Data, useDeltaRforFootprint = True)
Copy link
Contributor

Choose a reason for hiding this comment

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

is any of this needed in the master branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I do not think they are needed in the master. @lathomas @kirschen

Copy link
Contributor

Choose a reason for hiding this comment

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

This are already the settings in the master branch.
We backported back then but pipelined only for the nanov8 (run2_nanoAOD_106Xv1), but now we had to set explicitly for the nanov9 run2_nanoAOD_106Xv2 as well. Ideally we should set this as default and then reset for the nanos before UL (while keeping not the default in mini)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-48bba3/15906/summary.html
COMMIT: d6be8dc
CMSSW: CMSSW_10_6_X_2021-06-11-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34096/15906/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 3215678
  • DQMHistoTests: Total failures: 5
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215339
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
  • Checked 143 log files, 29 edm output root files, 35 DQM output files
  • TriggerResults: no differences found

@slava77
Copy link
Contributor

slava77 commented Jun 11, 2021

+reconstruction

for #34096 d6be8dc

  • code changes are in line with the PR description and the follow up discussion
    • the changes are specific to nanoAOD. Since this is submitted by XPOG I did not assign to XPOG
  • jenkins tests pass and comparisons with the baseline show a difference in the nanoAOD met significance plots in wf 136.8523 (controlled by run2_nanoAOD_106Xv2) as expected

e.g.
wf136 8523_smuPtunclustered

@cmsbuild
Copy link
Contributor

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
Copy link
Contributor

qliphy commented Jun 12, 2021

+1

@cmsbuild cmsbuild merged commit 345e76f into cms-sw:CMSSW_10_6_X Jun 12, 2021
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