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 puppi multiplicities to MiniAOD #22396

Merged
merged 3 commits into from Mar 8, 2018

Conversation

ahinzmann
Copy link
Contributor

@ahinzmann ahinzmann commented Feb 28, 2018

PUPPI-weighted multiplicities were found useful to define pileup-safe jet ID criteria and will shortly be recommended for analyses as discussed during this presentation:
https://indico.cern.ch/event/707753/contributions/2905460/attachments/1607520/2551266/JetID_run2017_AK4_PUPPI_recommendations_27022018_v1.pdf
We would therefore like to include them into MiniAOD (and backport to 94 asap).

It was checked that the new userfloats are added correctly to the slimmedJetsPuppi collection running the standard MiniAOD sequence.

Timing and size was checked using test/patMiniAOD_standard_cfg.py with 1000 events of RelValZEE_13/GEN-SIM-RECO/PU25ns_92X_upgrade2017_realistic_v7-v1:

Without this PR:
TimeReport 0.000082 0.000082 0.000082 slimmedJetsPuppi
patJets_slimmedJetsPuppi__PAT. 5769.94 668.333

With this PR:
TimeReport 0.000235 0.000235 0.000235 slimmedJetsPuppi
TimeReport 0.000071 0.000071 0.000071 slimmedJetsPuppiNoMultiplicties
TimeReport 0.000054 0.000054 0.000054 patPuppiJetSpecificProducer
patJets_slimmedJetsPuppi__PAT. 6953.5 705.626

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

PhysicsTools/PatAlgos

@perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @imarches, @acaudron, @mmarionncern, @rappoccio, @jdamgov, @jdolen, @nhanvtran, @gpetruc, @gkasieczka, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder, @seemasharmafnal, @JyothsnaKomaragiri this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Feb 28, 2018

@ahinzmann
please provide estimates of the size on disk needed for this PR per event.
CPU per event would be nice to know as well.

@slava77
Copy link
Contributor

slava77 commented Feb 28, 2018

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 28, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26388/console Started: 2018/02/28 21:50

@cmsbuild
Copy link
Contributor

-1

Tested at: 3d0c460

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22396/26388/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
21234.0 step3

runTheMatrix-results/21234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21.log

20434.0 step3
runTheMatrix-results/20434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D19_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D19+RecoFullGlobal_2023D19+HARVESTFullGlobal_2023D19/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D19_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D19+RecoFullGlobal_2023D19+HARVESTFullGlobal_2023D19.log

20034.0 step3
runTheMatrix-results/20034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17.log

@cmsbuild
Copy link
Contributor

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@ahinzmann
Copy link
Contributor Author

@slava77 I've added the timing and size to the description of the PR

@perrotta
Copy link
Contributor

perrotta commented Mar 1, 2018

Crash looks like being correlated:

----- Begin Fatal Exception 28-Feb-2018 22:31:01 CET-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Calling method for module PATJetSelector/'selectedUpdatedPatJetsPuppiJetSpecific'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for a container with elements of type: pat::Jet
Looking for module label: updatedPatJetsPuppiJetSpecific
Looking for productInstanceName: 

   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 -------------------------------------------------

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2018

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2018

Pull request #22396 was updated. @perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please check and sign again.

@perrotta
Copy link
Contributor

perrotta commented Mar 5, 2018

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26512/console Started: 2018/03/05 16:25

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2018

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22396/26512/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 121 differences found in the comparisons
  • DQMHistoTests: Total files compared: 29
  • DQMHistoTests: Total histograms compared: 2479021
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2478844
  • DQMHistoTests: Total skipped: 176
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.13000000017 KiB( 23 files compared)
  • Checked 118 log files, 9 edm output root files, 29 DQM output files

@perrotta
Copy link
Contributor

perrotta commented Mar 7, 2018

+1

  • Six userfloats added per jet to slimmedJetsPuppi
  • Differences reported in the jenkins tests refer to the newly added userfloats to the miniAOD output
  • miniAOD event size on disk increases by ~40 B (on slimmedJetsPuppi) when measured on Zee events with PU (as reported in the PR description)

@fabiocos
Copy link
Contributor

fabiocos commented Mar 7, 2018

+1

@perrotta
Copy link
Contributor

perrotta commented Mar 8, 2018

@fabiocos : this PR still misses the "analysis" signature and you need a explicit "merge" if you want to merge it

@fabiocos
Copy link
Contributor

fabiocos commented Mar 8, 2018

merge

@cmsbuild cmsbuild merged commit 7980e47 into cms-sw:master Mar 8, 2018
@arizzi
Copy link
Contributor

arizzi commented Mar 9, 2018

for the records: me and @gpetruc understood that we have to sign for AnalysisTools related PRs in PhysicsTools/PatAlgos or NanoAODTools only in the MAOD and analysis branches, not on master. Is that right?

@fabiocos
Copy link
Contributor

fabiocos commented Mar 9, 2018

yes, I would like your explicit approval for the AN branch, that is the general request. As far as PhysicsTools/NanoAOD is concerned anyway, I would say that missing an independent analysis signature it makes sense that you sign everywhere, as you are anyway the responsible and the tester of it

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