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

Maintain access to genParticles from signal event in packed collection for HI miniAOD #32668

Merged
merged 6 commits into from Jan 20, 2021

Conversation

mandrenguyen
Copy link
Contributor

@mandrenguyen mandrenguyen commented Jan 14, 2021

PR description:

For event-overlay MC workflows, we need to know which particles are from the signal event, i.e., the one with collisionID =0. This method is not available in the packedGenParticle collection. This PR adds a PackedGenParticleRefVector containing the PackedGenParticles with collisionID =0. It's run for all HI eras; there is no change to pp workflows.

PR validation:

Tested with 158.01

if this PR is a backport please specify the original PR and why you need to backport that PR:

Will need an 11_2_X backport

@ttrk @stepobr @stahlleiton

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32668/20745

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mandrenguyen (Matthew Nguyen) for master.

It involves the following packages:

DataFormats/PatCandidates
PhysicsTools/PatAlgos

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

@slava77
Copy link
Contributor

slava77 commented Jan 14, 2021

@cmsbuild please test

@perrotta
Copy link
Contributor

assign xpog
(perhaps more appropriate than reco for packedGenParticles)

@cmsbuild
Copy link
Contributor

New categories assigned: xpog

@fgolf,@mariadalfonso,@gouskos you have been requested to review this Pull request/Issue and eventually sign? Thanks

@perrotta
Copy link
Contributor

DataFormats/PatCandidates and PhysicsTools/PatAlgos were just removed from the Analysis category: I wonder whether they should stay under xpog now (in addition to reconstruction): @cms-sw/xpog-l2 what do you think?

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 14, 2021

-1

Failed Tests: INPUTRelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-56c8c2/12284/summary.html
COMMIT: f18dfa4
CMSSW: CMSSW_11_3_X_2021-01-13-2300/slc7_amd64_gcc900

INPUTRelVals

140.5611_RunHI2018AOD+RunHI2018AOD+REMINIAODHID18+HARVESTHI18MINIAOD/step2_RunHI2018AOD+RunHI2018AOD+REMINIAODHID18+HARVESTHI18MINIAOD.log

Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2716967
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2716938
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 156 log files, 37 edm output root files, 37 DQM output files

@mandrenguyen
Copy link
Contributor Author

Do I interpret correctly that the failing test has to do with input files?

@slava77
Copy link
Contributor

slava77 commented Jan 14, 2021

Do I interpret correctly that the failing test has to do with input files?

no; the workflows outputs are less easy to find now are now extended to more tests and "INPUTS" here means that the jobs are not redoing GEN-SIM if one exists.

in the results "Matrix INPUT Tests Outputs" https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-56c8c2/12284/runTheMatrixINPUT-results/140.5611_RunHI2018AOD+RunHI2018AOD+REMINIAODHID18+HARVESTHI18MINIAOD/step2_RunHI2018AOD+RunHI2018AOD+REMINIAODHID18+HARVESTHI18MINIAOD.log

An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 326479 lumi: 15 event: 3808176 stream: 0
   [1] Running path 'MINIAODoutput_step'
   [2] Prefetching for module PoolOutputModule/'MINIAODoutput'
   [3] Calling method for module PackedGenParticleSignalProducer/'packedGenParticlesSignal'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: edm::Association<std::vector<pat::PackedGenParticle> >
Looking for module label: packedGenParticles
Looking for productInstanceName: 

@slava77
Copy link
Contributor

slava77 commented Jan 14, 2021

genParticles should not be running on data

@slava77
Copy link
Contributor

slava77 commented Jan 14, 2021

the workflows outputs are less easy to find now

that was a wrong statement; I corrected it now. We have more short tests done and they are placed in the "INPUTRelVals" category of tests.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32668/20777

@cmsbuild
Copy link
Contributor

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

@mandrenguyen
Copy link
Contributor Author

@perrotta The MC input file in relval is not representative. It's a pythia event embedded into a very, very peripheral (b=12) PbPb event. Running on normal embedded MC, the output miniAOD is only a fraction of a percent smaller.
The file I tested is:
/store/himc/HINPbPbAutumn18DR/Bjet_pThat-15_TuneCP5_HydjetDrumMB_5p02TeV_Pythia8/AODSIM/mva98_103X_upgrade2018_realistic_HI_v11-v1/30000/7D8C962E-9A46-9541-94AA-69C01C817F2E.root

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-56c8c2/12325/summary.html
COMMIT: e21b2a0
CMSSW: CMSSW_11_3_X_2021-01-17-2300/slc7_amd64_gcc900

Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2716961
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2716932
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 156 log files, 37 edm output root files, 37 DQM output files

@perrotta
Copy link
Contributor

+1

  • The new code adds a PackedGenParticleRefVector with refs to selected "signal" generated particles, in HI MC
  • Jenkins tests pass and show no difference but the additional collection in HI MC workflows

@mariadalfonso
Copy link
Contributor

+xpog

@cmsbuild
Copy link
Contributor

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 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 Jan 20, 2021

+1

@cmsbuild cmsbuild merged commit 583cfcf into cms-sw:master Jan 20, 2021
ttrk added a commit to ttrk/cmssw that referenced this pull request Sep 7, 2021
collisionId_ is not kept in pat::PackedGenParticle, use "packedGenParticlesSignal" (added by cms-sw#32668) to tag particles from signal process
ttrk added a commit to CmsHI/cmssw that referenced this pull request Sep 8, 2021
* add HiGenAnalyzer
This analyzer is copied from its AOD counterpart https://github.com/CmsHI/cmssw/blob/2c806f88506f7ef732b725142ae85750a31dc646/HeavyIonsAnalysis/EventAnalysis/src/HiEvtAnalyzer.cc and adapted for gen info in miniAOD
Notes : GenHIEvent is currently missing from miniAOD

* fix sube
collisionId_ is not kept in pat::PackedGenParticle, use "packedGenParticlesSignal" (added by cms-sw#32668) to tag particles from signal process
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