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

NANO: rework for Prompt (part3) #34714

Merged
merged 11 commits into from Sep 1, 2021
Merged

Conversation

mariadalfonso
Copy link
Contributor

@mariadalfonso mariadalfonso commented Aug 1, 2021

Third PR migrate jet and met. Should be considered as the last part of the larger #34329

commit 6ec89e1 disactivate the MET recalibration for miniV2 and later, otherwise one get the following errors when RECO/mini/nano/DQM at the same time i.e. runs [*]

  1. ValueError: Trying to override definition of pfMetPuppi while it is used by the task highlevelrecoTask with cmsDriver command below [*]; this is prompted by an include like this
    from PhysicsTools.PatAlgos.slimming.puppiForMET_cff import makePuppiesFromMiniAOD

  2. pfMetSequence is kept ; the met-tool doesn't support task; this lead to "module order" problem
    http://dalfonso.web.cern.ch/dalfonso/XPOG/METinduced_ModuleOrder_pr34714.txt

It pass the following command with the step2 from the 11634.0.

cmsDriver.py step3 --conditions auto:phase1_2021_realistic -s RAW2DIGI,L1Reco,RECO,RECOSIM,EI,PAT,NANO,VALIDATION:@standardValidation+@miniAODValidation,DQM:@standardDQM+@ExtraHLT+@miniAODDQM --datatier GEN-SIM-RECO,MINIAODSIM,NANOAODSIM,DQMIO -n 10 --geometry DB:Extended --era Run3,run3_nanoAOD_devel --eventcontent RECOSIM,MINIAODSIM,NANOEDMAODSIM,DQM --filein file:step2.root --fileout file:step3.root

Added two workflows that run MINI,NANO,DQMmini,DQMnano:
136.72412 this is successful (MET is bypassed)
136.72413 this fail because of the MET is involved

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34714/24354

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2021

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

It involves the following packages:

  • Configuration/Eras (operations)
  • Configuration/StandardSequences (operations)
  • PhysicsTools/NanoAOD (xpog)

@perrotta, @gouskos, @silviodonato, @cmsbuild, @fgolf, @qliphy, @mariadalfonso, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@mtosi, @fabiocos, @swertz, @makortel, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @Martin-Grunewald, @lecriste, @gpetruc, @ebrondol, @mmusich, @dgulhan, @slomeo this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

@mariadalfonso
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2021

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-898790/17400/summary.html
COMMIT: 63ea0ee
CMSSW: CMSSW_12_1_X_2021-08-01-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34714/17400/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 136.7722136.7722_RunJetHT2016H_nano+RunJetHT2016H_nano+NANOEDM2016_80X+HARVESTNANOAOD2016_80X/step2_RunJetHT2016H_nano+RunJetHT2016H_nano+NANOEDM2016_80X+HARVESTNANOAOD2016_80X.log
  • 1329.11329.1_ZEE_13_80XNanoAODINPUT+ZEE_13_80XNanoAODINPUT+NANOEDMMC2016_80X+HARVESTNANOAODMC2016_80X/step2_ZEE_13_80XNanoAODINPUT+ZEE_13_80XNanoAODINPUT+NANOEDMMC2016_80X+HARVESTNANOAODMC2016_80X.log

Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 171 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 2998564
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2998541
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: found differences in 1 / 37 workflows

@mariadalfonso
Copy link
Contributor Author

  • Reco comparison results: 171 differences found in the comparisons

these are met related things as expected
the jetTable shows difference because of this variable that got commented together with the other METroutines (then the results from the cms-bot are just all offset)

Screen Shot 2021-08-02 at 10 46 45

@mariadalfonso
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34714/24368

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2021

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2021

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-898790/17425/summary.html
COMMIT: 9f0bea5
CMSSW: CMSSW_12_1_X_2021-08-02-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34714/17425/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 1329.11329.1_ZEE_13_80XNanoAODINPUT+ZEE_13_80XNanoAODINPUT+NANOEDMMC2016_80X+HARVESTNANOAODMC2016_80X/step2_ZEE_13_80XNanoAODINPUT+ZEE_13_80XNanoAODINPUT+NANOEDMMC2016_80X+HARVESTNANOAODMC2016_80X.log
  • 11634.50111634.501_TTbar_14TeV+2021_Patatrack_PixelOnlyCPU+TTbar_14TeV_TuneCP5_GenSimINPUT+Digi+Reco+HARVEST/step2_TTbar_14TeV+2021_Patatrack_PixelOnlyCPU+TTbar_14TeV_TuneCP5_GenSimINPUT+Digi+Reco+HARVEST.log
  • 11601.011601.0_SingleElectronPt10+2021+SingleElectronPt10_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA/step2_SingleElectronPt10+2021+SingleElectronPt10_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA.log
Expand to see more relval errors ...

Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 171 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 2998564
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2998541
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: found differences in 1 / 37 workflows

@jordan-martins
Copy link
Contributor

Hi all,
agreeing with @kskovpen from PdmV. And, just to confirm that indeed we should start from RAW, despite the current justification to run from AOD at this point, as discussed previously.

@mariadalfonso
Copy link
Contributor Author

@perrotta ,@qliphy

This PR should be ready to be merged, we can talk soon at the ORP.
This will allow:

  1. Tier0 do their tests
  2. JME can proceed with the fixed in deepMET and metSignificance
  3. PDMV can set the Run3 wf as they wish.

@qliphy
Copy link
Contributor

qliphy commented Sep 1, 2021

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 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 be automatically merged.

@cmsbuild cmsbuild merged commit e864342 into cms-sw:master Sep 1, 2021
@makortel makortel mentioned this pull request Sep 1, 2021
@qliphy
Copy link
Contributor

qliphy commented Sep 2, 2021

@mariadalfonso @srimanob
We now have "test-rejected" in recent tests:
#35114
#35100
which is due to 136.72413. Although it is expected as a feature of this PR
#35116
Should we consider switching off the workflow for the moment?

@srimanob
Copy link
Contributor

srimanob commented Sep 2, 2021

Hi @qliphy
Do we need to just comment on that workflow, or we can block it from the relval-input test?

If we need to comment that line, here is the PR:
#35121

@qliphy
Copy link
Contributor

qliphy commented Sep 2, 2021

thanks @srimanob Commenting out the line should be fine.

@qliphy
Copy link
Contributor

qliphy commented Sep 3, 2021

@mariadalfonso

There are issues with 3 workflows in recent IB:
10224.15
11024.15
25202.15
for example
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_1_X_2021-09-02-1600/pyRelValMatrixLogs/run/11024.15_TTbar_13+2018PU_JMENano+TTbar_13TeV_TuneCUETP8M1_GenSimINPUT+DigiPU+RecoFakeHLTPU+HARVESTFakeHLTPU+Nano/step5_TTbar_13+2018PU_JMENano+TTbar_13TeV_TuneCUETP8M1_GenSimINPUT+DigiPU+RecoFakeHLTPU+HARVESTFakeHLTPU+Nano.log#/
File "/cvmfs/cms-ib.cern.ch/nweek-02696/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_1_X_2021-09-02-1600/python/PhysicsTools/NanoAOD/custom_jme_cff.py", line 986, in AddNewAK8GenJetsForJEC
SaveGenJets(proc, genJetName, genJetAlgo, genJetSizeNr, genJetFinalColl, genJetTablePrefix, genJetTableDoc, runOnMC=False)
File "/cvmfs/cms-ib.cern.ch/nweek-02696/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_1_X_2021-09-02-1600/python/PhysicsTools/NanoAOD/custom_jme_cff.py", line 920, in SaveGenJets
proc.nanoSequenceMC.insert(proc.nanoSequenceMC.index(proc.jetMC)+1, getattr(proc,genJetSequenceName))
AttributeError: 'Process' object has no attribute 'jetMC'

Would you please have a look?

@mariadalfonso
Copy link
Contributor Author

@mariadalfonso

There are issues with 3 workflows in recent IB:
10224.15
11024.15
25202.15

Would you please have a look?

yes, those are JMEnano and it is well known that they got broken after the work we had to do for prepare for prompt.
It's up to JME to fix them, and simplify them and make them ready for Run3 (xPOG doesn't coordinate neither the fwk update nor the content, we only monitor the size given the budget set by O&C).
As far as I know no central production was started in Run2 based on this up to now.
JME got notified early summer at PC and PPD coordination meetings and via email that this was happening.

If they create problem in operations you can comment like was done for 136.72413 (MET tool) and rehabilitate when/if jme fix them.
@srimanob you introduced them, right ?

@qliphy
Copy link
Contributor

qliphy commented Sep 3, 2021

Thanks @mariadalfonso

Adding @kirschen @camclean Please have a look.

If they create problem in operations you can comment like was done for 136.72413 (MET tool) and rehabilitate when/if jme fix them.
Unlike 136.72413, the JMEnano workflows are not in relval_standard, so not causing problem in PR tests so far.

But still it would be nice to fix them, otherwise the issues will stay appearing in IB.

@srimanob
Copy link
Contributor

srimanob commented Sep 3, 2021

May I suggest we open the issue to collect workflows which have issue with new NanoAOD, instead of adding more comment to closed PR?

Should I ?

For workflow .15, it was introduced together with JME to make sure we can always have NanoAOD that JME used for any derivation later. It was introduced in #29279
I can disable them for now.

@srimanob
Copy link
Contributor

srimanob commented Sep 3, 2021

By the way, here is the PR to disable JME Nano workflows, #35140

@qliphy
Copy link
Contributor

qliphy commented Sep 3, 2021

May I suggest we open the issue to collect workflows which have issue with new NanoAOD, instead of adding more comment to closed PR?

Thanks Phat, yes that sounds good to me.

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

6 participants