Skip to content

Conversation

@JanFSchulte
Copy link
Contributor

This is a backport of #33758 to the 106X release cycle so the weights can be added to the next nano-AOD production.

Incidentally this PR also serves as a backport of #32728 by @lathomas.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 17, 2021

A new Pull Request was created by @JanFSchulte (Jan-Frederik Schulte) for CMSSW_10_6_X.

It involves the following packages:

PhysicsTools/NanoAOD
PhysicsTools/PatUtils

@perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso can you please review it and eventually sign? Thanks.
@mmarionncern, @gouskos, @swertz, @JyothsnaKomaragiri, @ahinzmann, @schoef, @rappoccio, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @clelange, @emilbols, @hatakeyamak, @ferencek, @gpetruc, @andrzejnovak, @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

@mariadalfonso
Copy link
Contributor

as a note, we need to safeguard the current ongoing v8 production in 10_6_X; you should add things like
(run2_nanoAOD_106Xv1 & ~run2_nanoAOD_devel).toModify(xyzTable , xyxVariable = None)

@silviodonato
Copy link
Contributor

kind reminder @JanFSchulte (see the comment above)

@perrotta
Copy link
Contributor

kind reminder @JanFSchulte (see the comment above)

To be complete, this PR still needs the following:

@perrotta
Copy link
Contributor

backport of #33758

@cmsbuild
Copy link
Contributor

Pull request #33759 was updated. @perrotta, @gouskos, @silviodonato, @cmsbuild, @jpata, @fgolf, @slava77, @qliphy, @mariadalfonso, @fabiocos, @davidlange6 can you please check and sign again.

@JanFSchulte
Copy link
Contributor Author

Some comments:

@cmsbuild
Copy link
Contributor

Pull request #33759 was updated. @perrotta, @gouskos, @silviodonato, @cmsbuild, @jpata, @fgolf, @slava77, @qliphy, @mariadalfonso, @fabiocos, @davidlange6 can you please check and sign again.

@mariadalfonso
Copy link
Contributor

  • I have added the protection, at least insofar as I understood it. This was about not adding the new variables to nanoAOD, right? Because the weight calculation will change regardless with the new code. If that is not desired, I can add a switch to the producer to exclude the muon weights.

this is ok to keep the same set of branches as in production
https://github.com/cms-sw/cmssw/pull/33759/files#diff-9b898944f08810913ea5c1605c47bd9a6c99eb91fc20f491ba2767702f4a0184R259
but also the general weight should not change (so will have to stay the ECAL only), so we need to add a switch

@cmsbuild
Copy link
Contributor

Pull request #33759 was updated. @perrotta, @gouskos, @silviodonato, @cmsbuild, @jpata, @fgolf, @slava77, @qliphy, @mariadalfonso, @fabiocos, @davidlange6 can you please check and sign again.

@JanFSchulte
Copy link
Contributor Author

I have added a flag and added the switch to the config to turn muon weights offs in nano V8. I also realized that by backporting #32728 we switched to the new UL2017 maps for ECAL, so I added a protection for that, too. @lathomas could you check if this makes sense.

@perrotta
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-a7131b/15423/summary.html
COMMIT: 728ae17
CMSSW: CMSSW_10_6_X_2021-05-29-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test runtestPhysicsToolsNanoAOD had ERRORS

RelVals

----- Begin Fatal Exception 29-May-2021 23:02:51 CEST-----------------------
An exception of category 'FileInPathError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=L1PrefiringWeightProducer label='prefiringweight'
Exception Message:
edm::FileInPath unable to find file PhysicsTools/PatUtils/data/L1MuonPrefiringParametriations.root anywhere in the search path.
The search path is defined by: CMSSW_SEARCH_PATH
${CMSSW_SEARCH_PATH} is: /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/poison:/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/src:/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/external/slc7_amd64_gcc700/data:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/poison:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/src:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/external/slc7_amd64_gcc700/data
Current directory is: /data/cmsbld/jenkins/workspace/ib-run-pr-relvals/runTheMatrix-results/136.8523_RunJetHT2018C_nanoULremini+RunJetHT2018C_nanoULremini+NANOEDM2018_106Xv2+HARVESTNANOAOD2018_106Xv2
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 29-May-2021 23:03:29 CEST-----------------------
An exception of category 'FileInPathError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=L1PrefiringWeightProducer label='prefiringweight'
Exception Message:
edm::FileInPath unable to find file PhysicsTools/PatUtils/data/L1MuonPrefiringParametriations.root anywhere in the search path.
The search path is defined by: CMSSW_SEARCH_PATH
${CMSSW_SEARCH_PATH} is: /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/poison:/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/src:/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/external/slc7_amd64_gcc700/data:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/poison:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/src:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/external/slc7_amd64_gcc700/data
Current directory is: /data/cmsbld/jenkins/workspace/ib-run-pr-relvals/runTheMatrix-results/136.8311_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 29-May-2021 23:03:31 CEST-----------------------
An exception of category 'FileInPathError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=L1PrefiringWeightProducer label='prefiringweight'
Exception Message:
edm::FileInPath unable to find file PhysicsTools/PatUtils/data/L1MuonPrefiringParametriations.root anywhere in the search path.
The search path is defined by: CMSSW_SEARCH_PATH
${CMSSW_SEARCH_PATH} is: /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/poison:/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/src:/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15423/CMSSW_10_6_X_2021-05-29-1100/external/slc7_amd64_gcc700/data:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/poison:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/src:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_6_X_2021-05-29-1100/external/slc7_amd64_gcc700/data
Current directory is: /data/cmsbld/jenkins/workspace/ib-run-pr-relvals/runTheMatrix-results/136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM
----- End Fatal Exception -------------------------------------------------
Expand to see more relval errors ...

RelVals-INPUT

  • 136.72411136.72411_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM/step2_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM.log
  • 136.7722136.7722_RunJetHT2016H_nano+RunJetHT2016H_nano+NANOEDM2016_80X+HARVESTNANOAOD2016_80X/step2_RunJetHT2016H_nano+RunJetHT2016H_nano+NANOEDM2016_80X+HARVESTNANOAOD2016_80X.log
  • 136.7611136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log
Expand to see more relval errors ...

@mariadalfonso
Copy link
Contributor

test parameters:

pull_request = cms-data/PhysicsTools-PatUtils#2

@mariadalfonso
Copy link
Contributor

please test

@JanFSchulte
Copy link
Contributor Author

I can do that, but that will change the name of already existing variables in V8, since the naming scheme was already this way before: https://github.com/cms-sw/cmssw/pull/33759/files#diff-9b898944f08810913ea5c1605c47bd9a6c99eb91fc20f491ba2767702f4a0184R239-R249

@mariadalfonso
Copy link
Contributor

@JanFSchulte
Copy link
Contributor Author

Ah, I get it now, it's because we have added a name to the table. I'll fix that.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 9, 2021

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

@mariadalfonso
Copy link
Contributor

please test

@mariadalfonso
Copy link
Contributor

results from this morning nano-test:
tests done on IB: CMSSW_10_6_X_2021-06-08-2300 (it include the final cms-dist)
on Z --> tau tau

Screen Shot 2021-06-09 at 11 44 54

looks ready to be signed, I wait the cms-sw bot finish

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 9, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a7131b/15801/summary.html
COMMIT: 991fa3b
CMSSW: CMSSW_10_6_X_2021-06-08-2300/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33759/15801/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
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 3215678
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215342
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.52 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 1325.81 ): 0.562 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 136.8523 ): 0.958 KiB Physics/NanoAODDQM
  • Checked 143 log files, 29 edm output root files, 35 DQM output files
  • TriggerResults: found differences in 2 / 34 workflows

@mariadalfonso
Copy link
Contributor

comparing master vs backport, I need a clarification on the 2018 set up
see here
https://github.com/cms-sw/cmssw/pull/33759/files#r648949152

stage1L1Trigger.toModify(process.prefiringweight, DataEra = "2016BtoH")
stage2L1Trigger_2017.toModify(process.prefiringweight, DataEra = "2017BtoF")
process.load("PhysicsTools.PatUtils.L1PrefiringWeightProducer_cff")
stage2L1Trigger.toModify(process.prefiringweight, DataEraECAL = "2017BtoF", JetMaxMuonFraction = -1, DoMuons = cms.bool(False) )
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the era correct here ("2017BtoF")?

Copy link
Contributor

Choose a reason for hiding this comment

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

if we want to reproduce the configuration previous to this PR to comply with the rule forbidding changes to RECO, yes. (these 2017 maps were also mistakenly picked previously for 2016 ...)

Copy link
Contributor

Choose a reason for hiding this comment

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

if we want to reproduce the configuration previous to this PR to comply with the rule forbidding changes to RECO, yes. (these 2017 maps were also mistakenly picked previously for 2016 ...)

Ok, thank you!

@perrotta
Copy link
Contributor

@JanFSchulte @mariadalfonso please notify in this github thread as soon as everything got clarified, so that we can conclude with this backport PR (I can take a "+1" from xpog as an implicit notification).

@mariadalfonso
Copy link
Contributor

+xpog

changes inline with master; protected the nanov8 settings
some results here #33759 (comment)

@perrotta
Copy link
Contributor

+1

@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 d7f9097 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.

10 participants