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

Combined backport of PR #19205 and #19404: ootPhotons in miniAOD from AOD from either 92X or 80X inputs #19448

Merged
merged 20 commits into from Jul 6, 2017

Conversation

kmcdermo
Copy link
Contributor

@kmcdermo kmcdermo commented Jun 27, 2017

Backport of both PR #19205 and #19404 for 92X. Both have been merged into 93X.

19205: ootPhotons in miniAOD from AOD (if ootPhotons produced during RECO step)
19404: ootPhotons in miniAOD from 80X AOD (run a era-specific customized ootPhoton RECO step, then run the PAT step to produce ootPhotons in miniAO)

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @kmcdermo (Kevin McDermott) for CMSSW_9_2_X.

It involves the following packages:

PhysicsTools/PatAlgos
RecoEgamma/EgammaPhotonProducers

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

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Jun 27, 2017

@kmcdermo
please edit the title of the PR to be more descriptive
Thank you.

@kmcdermo kmcdermo changed the title Backport of PR #19205 Backport of PR #19205 (ootPhotons for miniAOD from AOD in 92X) Jun 27, 2017
@kmcdermo
Copy link
Contributor Author

@slava77
can this PR be tested now for the backport?

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 30, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21023/console Started: 2017/06/30 16:28

@arizzi
Copy link
Contributor

arizzi commented Jun 30, 2017

@kmcdermo @davidlange6 are the 92X relval already containing the ootPhotons at RECO level so this should work out of the box?

@kmcdermo
Copy link
Contributor Author

@arizzi
yes, indeed, this was included:
175981a

so it will pick up the relvals from 92X with ootPhotons already at AOD.

@arizzi
Copy link
Contributor

arizzi commented Jun 30, 2017

mhh... then why recent 93X IB (containing the other PR) fail on 92X relval with
30-Jun-2017 16:48:45 CEST Successfully opened file root://eoscms.cern.ch//eos/cms/store/relval/CMSSW_9_2_1/RelValTTbar_13/GEN-SIM-RECO/PU25ns_92X_upgrade2017_realistic_v1-v1/10000/0C07A2DB-594A-E711-A1FF-0025905A6134.root
Begin processing the 1st record. Run 1, Event 7607, LumiSection 153 at 30-Jun-2017 16:49:19.603 CEST
----- Begin Fatal Exception 30-Jun-2017 16:49:32 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
[0] Processing Event run: 1 lumi: 153 event: 7607 stream: 0
[1] Running path 'MINIAODSIMoutput_step'
[2] Prefetching for module PoolOutputModule/'MINIAODSIMoutput'
[3] Calling method for module ReducedEGProducer/'reducedEgamma'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vectorreco::Photon
Looking for module label: ootPhotons
Looking for productInstanceName:

@kmcdermo
Copy link
Contributor Author

@arizzi

Where is this file getting picked up? ootPhotons at AOD were added into 9_2_2, and this is a 9_2_1 sample, so that test needs to be updated.

@arizzi
Copy link
Contributor

arizzi commented Jun 30, 2017

that was my test, not something central. so not any 92X is ok, should be >=922 (will try this)

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • You potentially added 2 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1813696
  • DQMHistoTests: Total failures: 29444
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1784086
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@slava77
Copy link
Contributor

slava77 commented Jul 1, 2017

as noted in #19404 (comment)
we will need the legacy reminiAOD part combined with this PR for 92X integration.

…ation to before loops, const auto where possible, use index=-1 in EGM object loops to avoid keeping track of incrementing index
…mma_cfi, make ootPhotons a task in ootPhotonSequence_cff, import task and modify accordingly for legacy task in ootPhotonProducer_cff
@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2017

Pull request #19448 was updated. @perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please check and sign again.

@kmcdermo kmcdermo changed the title Backport of PR #19205 (ootPhotons for miniAOD from AOD in 92X) Combined backport of PR #19205 and #19404: ootPhotons in miniAOD from AOD from either 92X or 80X inputs Jul 3, 2017
@slava77
Copy link
Contributor

slava77 commented Jul 3, 2017

@kmcdermo
is this backport done, or is it still in the middle of commits and tests?

@kmcdermo
Copy link
Contributor Author

kmcdermo commented Jul 3, 2017

@slava77
please let me test locally first to make sure I did not break anything, but otherwise these are all the commits that are needed.

@kmcdermo
Copy link
Contributor Author

kmcdermo commented Jul 3, 2017

@slava77
Okay, tested locally on an 80X AOD file, and it worked (produced ootPhotons) .

Let me know if you want me to squash any of these commits. I just rebased this branch, then cherry-picked the seven commits from 19404 on top.

@slava77
Copy link
Contributor

slava77 commented Jul 3, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21109/console Started: 2017/07/03 23:51
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21112/console Started: 2017/07/04 00:59

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

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

Comparison Summary:

  • You potentially added 2 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1813730
  • DQMHistoTests: Total failures: 93
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 1813470
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@kmcdermo
Copy link
Contributor Author

kmcdermo commented Jul 5, 2017

ping on this PR

@slava77
Copy link
Contributor

slava77 commented Jul 6, 2017

+1

for #19448 321d0bb

  • changes in the code in this PR are identical to the combination of Out-of-time photons for MiniAOD #19205 06ec2b4 and ootPhotons for 80X Legacy reprocessing #19404 73da687; both are present in 930pre1 and apparently have not produced any issues there
  • jenkins tests pass and comparisons with baseline show differences only in the reducedEgamma reducedEBrechits, where some increment is expected; based on a copy of jenkins outputs in 136.731 I also can confirm that the expected new products are present (slimmedOOTPhotons and reducedEgamma_*OOT*Clusters)

@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit d38320d into cms-sw:CMSSW_9_2_X Jul 6, 2017
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