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

decoupling NANO V10 and V11 [WIP] #39337

Merged
merged 15 commits into from Oct 4, 2022
Merged

Conversation

vlimant
Copy link
Contributor

@vlimant vlimant commented Sep 7, 2022

PR description:

This is work in progress, but I'd like to have initial feedback on some of the changes in ConfigBuilder.

The aim with this changes is to be able to support both V10 and V11(in development) in 12.6, in a sustainable way.
The current process is to add whatever NANO feature to master/12.6 for V11, and "remove it" by customisation "back to" V10 ; which obviously is extremely error prone.

The proposal here is copying most configuration under the V10 directory to avoid touching it while putting new features in V11 (master) ; there are however, it seems some dependencies still to be removed in this initial PR. We might have to dump some of the module configs in V10 directory to fully decouple.

PR validation:

for NANO, these commands run and produce runable configurations
for the V11 configuration cmsDriver.py nanoMaster --step NANO --datatier NANOAOD --eventcontent NANOAOD --conditions 124X_dataRun3_Prompt_v4 --era Run3 --no_exec --data --filein "file:86c32527-92dd-4ec9-9e72-aba8878ab893.root" --fileout nanoMaster.root -n 100

for the V10 configuration cmsDriver.py nanoV10 --step NANO:PhysicsTools/NanoAOD/V10/nano_cff --datatier NANOAOD --eventcontent NANOAOD --conditions 124X_dataRun3_Prompt_v4 --era Run3 --no_exec --data --filein "file:86c32527-92dd-4ec9-9e72-aba8878ab893.root" --fileout nanoV10.root -n 100

ConfigBuilder was tested with runTheMatrix.py -l limited -j 0 that managed to configure all 50 workflows steps (except for dad queries) ; that indicates that I have not majorly broken cmsDriver

EDITS (30/9/22):
The change in ConfigBuilder is done to allow STEP:<a specific path to configuration> without having to specify the actual Sequence to be used, and hence pick up the default ones defined in ConfigBuilder.
The new ERA is not mandatory in this PR, but will be later on #39452 and fits well in here.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39337/32035

  • This PR adds an extra 88KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2022

A new Pull Request was created by @vlimant (vlimant) for master.

It involves the following packages:

  • Configuration/Applications (operations)
  • PhysicsTools/NanoAOD (xpog)

@perrotta, @rappoccio, @swertz, @vlimant, @cmsbuild, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@swertz, @makortel, @Martin-Grunewald, @missirol, @gpetruc, @fabiocos this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@vlimant
Copy link
Contributor Author

vlimant commented Sep 8, 2022

please test

@vlimant
Copy link
Contributor Author

vlimant commented Sep 8, 2022

I might use #39342 to further cleanup the V10 configuration as part of the decoupling

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2022

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3983e2/27422/summary.html
COMMIT: e820750
CMSSW: CMSSW_12_6_X_2022-09-07-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39337/27422/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 TestConfigDP had ERRORS

RelVals

  • 4.534.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log
  • 136.731136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2_skimSinglePh/step2_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2_skimSinglePh.log
  • 136.793136.793_RunDoubleEG2017C+RunDoubleEG2017C+HLTDR2_2017+RECODR2_2017reHLT_skimDoubleEG_Prompt+HARVEST2017_skimDoubleEG/step2_RunDoubleEG2017C+RunDoubleEG2017C+HLTDR2_2017+RECODR2_2017reHLT_skimDoubleEG_Prompt+HARVEST2017_skimDoubleEG.log
Expand to see more relval errors ...

RelVals-INPUT

  • 4.174.17_RunMinBias2011A+RunMinBias2011A+HLTD+RECODR1reHLT+HARVESTDR1reHLT+SKIMDreHLT/step2_RunMinBias2011A+RunMinBias2011A+HLTD+RECODR1reHLT+HARVESTDR1reHLT+SKIMDreHLT.log
  • 4.264.26_ZMuSkim2011A+ZMuSkim2011A+HLTDSKIM+RECODR1reHLT+HARVESTDR1reHLT/step2_ZMuSkim2011A+ZMuSkim2011A+HLTDSKIM+RECODR1reHLT+HARVESTDR1reHLT.log
  • 4.234.23_ValSkim2011A+ValSkim2011A+HLTDSKIM+RECODR1reHLT+HARVESTDR1reHLT/step2_ValSkim2011A+ValSkim2011A+HLTDSKIM+RECODR1reHLT+HARVESTDR1reHLT.log
Expand to see more relval errors ...

AddOn Tests

----- Begin Fatal Exception 08-Sep-2022 10:19:12 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 4 stream: 0
   [1] Running path 'generation_step'
   [2] Calling method for module BetafuncEvtVtxGenerator/'VtxSmeared'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: edm::HepMCProduct
Looking for module label: generator
Looking for productInstanceName: unsmeared

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 08-Sep-2022 10:19:12 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 2 stream: 3
   [1] Running path 'generation_step'
   [2] Calling method for module BetafuncEvtVtxGenerator/'VtxSmeared'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: edm::HepMCProduct
Looking for module label: generator
Looking for productInstanceName: unsmeared

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 08-Sep-2022 10:19:12 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 4 stream: 0
   [1] Running path 'generation_step'
   [2] Calling method for module BetafuncEvtVtxGenerator/'VtxSmeared'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: edm::HepMCProduct
Looking for module label: generator
Looking for productInstanceName: unsmeared

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------
Expand to see more addon errors ...

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39337/32050

  • This PR adds an extra 36KB to repository

@rappoccio
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 4, 2022

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests.

@perrotta
Copy link
Contributor

perrotta commented Oct 4, 2022

@rappoccio you have merged after the latest commit could be tested: it is a simple commented out line, but still we should have followed the usual procedure and let the test run...

By the way @smuzaffar why bot merged this even if it was still in the status "test started" right before #39337 (comment) ?

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 4, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3983e2/27991/summary.html
COMMIT: 95aca85
CMSSW: CMSSW_12_6_X_2022-10-04-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39337/27991/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-3983e2/41834.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 24 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3391103
  • DQMHistoTests: Total failures: 50
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3391031
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -76887.64699999998 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): -640.710 KiB JetMET/PFCandidates
  • DQMHistoSizes: changed ( 10024.0,... ): -448.898 KiB ParticleFlow/slimmedJetValidation
  • DQMHistoSizes: changed ( 10024.0,... ): -263.359 KiB ParticleFlow/slimmedElectronValidation
  • DQMHistoSizes: changed ( 10024.0,... ): -261.354 KiB ParticleFlow/slimmedMuonValidation
  • DQMHistoSizes: changed ( 10024.0,... ): -239.348 KiB ParticleFlow/slimmedMETValidation
  • DQMHistoSizes: changed ( 10024.0,... ): -239.010 KiB Physics/Top
  • DQMHistoSizes: changed ( 10024.0,... ): -235.113 KiB JetMET/Jet
  • DQMHistoSizes: changed ( 10024.0,... ): -198.366 KiB JetMET/MET
  • DQMHistoSizes: changed ( 10024.0,... ): -37.338 KiB ParticleFlow/PFJetResValidation
  • DQMHistoSizes: changed ( 10024.0,... ): -24.309 KiB Tracking/PackedCandidate
  • DQMHistoSizes: changed ( 13234.0 ): ...
  • Checked 204 log files, 49 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor

perrotta commented Oct 5, 2022

@vlimant are the differences in the jet dqm plots understood?

Moreiover: this PR is causing errors in the IB, see for example https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_amd64_gcc10/CMSSW_12_6_X_2022-10-04-2300/pyRelValMatrixLogs/run/11634.15_TTbar_14TeV+2021_JMENano+TTbar_14TeV_TuneCP5_GenSimINPUT+Digi+RecoNano+HARVESTNano+ALCA/step3_TTbar_14TeV+2021_JMENano+TTbar_14TeV_TuneCP5_GenSimINPUT+Digi+RecoNano+HARVESTNano+ALCA.log#/

I think this PR was merged a bit too hastily: I'm preparing a revert PR, to have it ready just in case we are not yet fully convinced by tomorrow, when we'll cut 12_6_0_pre3

@smuzaffar
Copy link
Contributor

By the way @smuzaffar why bot merged this even if it was still in the status "test started" right before #39337 (comment) ?

@rappoccio merged commit aa85daa) into cms-sw:master 13 hours ago

looks like @rappoccio merged it instead of bot :-)

@vlimant
Copy link
Contributor Author

vlimant commented Oct 5, 2022

11634.15 was not part of the CI tests it seems. It using the customize function for V10 that this PR is getting rid of.

vlimant added a commit to vlimant/cmssw that referenced this pull request Oct 5, 2022
vlimant added a commit to vlimant/cmssw that referenced this pull request Oct 6, 2022
nothingface0 pushed a commit to borzari/cmssw that referenced this pull request Oct 17, 2022
@vlimant vlimant mentioned this pull request Oct 20, 2022
3 tasks
cramonal pushed a commit to cramonal/cmssw that referenced this pull request Nov 14, 2022
@vlimant vlimant deleted the v10_v11_decoupling branch November 22, 2022 10:17
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

7 participants