Remove OOT photons from 2018 heavy-ion (AA) era #23371

mandrenguyen · 2018-05-29T14:47:30Z

OOT photons are not needed for reconstruction of PbPb events.
This PR modifies the era "pp_on_AA_2018", which is intended for PbPb processing this year.
It was tested with wf 158.

cmsbuild · 2018-05-29T14:47:53Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-05-29T14:49:39Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23371/4880

cmsbuild · 2018-05-29T14:49:58Z

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

It involves the following packages:

RecoEcal/EgammaClusterProducers
RecoEgamma/EgammaIsolationAlgos
RecoEgamma/EgammaPhotonProducers
RecoLocalCalo/HcalRecProducers
RecoParticleFlow/PFProducer

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @Sam-Harper, @jainshilpi, @rovere, @argiro, @bachtis, @cbernet, @mariadalfonso, @lgray, @varuns23, @seemasharmafnal 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 · 2018-05-29T15:53:42Z

RecoEcal/EgammaClusterProducers/python/reducedRecHitsSequence_cff.py

-                                      )
+                                        cms.InputTag("interestingGedEgammaIsoESDetId"),
+                                        cms.InputTag("interestingOotEgammaIsoESDetId"),
+                                        )


I think that the previous indentation of closing braces was more consistent before the changes made here

slava77 · 2018-05-29T16:23:03Z

RecoEcal/EgammaClusterProducers/python/reducedRecHitsSequence_cff.py

+pp_on_AA_2018.toReplaceWith(reducedEcalRecHitsTask, _pp_on_AA_reducedEcalRecHitsTask)
+
+_pp_on_AA_interestingDetIdsEB  = cms.VInputTag(
+    cms.InputTag("interestingEcalDetIdEB"),


this and other better be rewritten in a way that there is no copy-paste of the existing vectors.
something along the lines

pp_on_AA_2018.toModify(reducedEcalRecHitsEB.interestingDetIdCollections, func = lambda list: list.remove(cms.InputTag("interestingEcalDetIdOOTPFEE")) ))

slava77 · 2018-05-29T16:28:16Z

RecoEgamma/EgammaPhotonProducers/src/ReducedEGProducer.cc

@@ -46,7 +46,8 @@

 ReducedEGProducer::ReducedEGProducer(const edm::ParameterSet& config) :
  photonT_(consumes<reco::PhotonCollection>(config.getParameter<edm::InputTag>("photons"))),
-  ootPhotonT_(consumes<reco::PhotonCollection>(config.getParameter<edm::InputTag>("ootPhotons"))),
+  doOotPhotons_(!config.getParameter<edm::InputTag>("ootPhotons").label().empty()),
+  ootPhotonT_(doOotPhotons_ ? consumes<reco::PhotonCollection>(config.getParameter<edm::InputTag>("ootPhotons")) : edm::EDGetTokenT<reco::PhotonCollection>()),


I think that this change is not necessary.
Simply test for a value of ootPhotonT_.isUnintialized() in places where this doOotPhotons_ was introduced

modifications that hide calls to produces<some OOT related type>(some oot related name) are missing. Please add.

slava77 · 2018-05-29T16:32:45Z

RecoLocalCalo/HcalRecProducers/python/HcalHitSelection_cfi.py

+
+pp_on_AA_2018.toModify(
+    reducedHcalRecHits,
+    interestingDetIds = _pp_on_AA_interestingDetIds


it would be more practical to use toModify as in the other file without assuming what's in the original interestingDetIds.

slava77 · 2018-05-29T16:33:47Z

@cmsbuild please test

cmsbuild · 2018-05-29T16:34:04Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28264/console Started: 2018/05/29 18:34

cmsbuild · 2018-05-29T19:17:44Z

+1
Tested at: 414283d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23371/28264/summary.html

cmsbuild · 2018-05-29T19:17:47Z

Comparison job queued.

cmsbuild · 2018-05-29T21:15:54Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 31
DQMHistoTests: Total histograms compared: 2902471
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2902279
DQMHistoTests: Total skipped: 190
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
Checked 128 log files, 14 edm output root files, 31 DQM output files

cmsbuild · 2018-06-05T13:06:32Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-06-05T13:08:57Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23371/5038

cmsbuild · 2018-06-05T13:09:15Z

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

slava77 · 2018-06-05T13:15:29Z

@cmsbuild please test workflow 158.0

cmsbuild · 2018-06-05T13:15:47Z

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

cmsbuild · 2018-06-05T16:10:29Z

+1
Tested at: 56a39ea
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23371/28484/summary.html

cmsbuild · 2018-06-05T16:10:31Z

Comparison job queued.

cmsbuild · 2018-06-05T18:57:57Z

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

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

/build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-23371/158.0_HydjetQ_B12_5020GeV_2018+HydjetQ_B12_5020GeV_2018+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 31
DQMHistoTests: Total histograms compared: 2902471
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2902280
DQMHistoTests: Total skipped: 190
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
Checked 128 log files, 14 edm output root files, 31 DQM output files

slava77 · 2018-06-05T22:20:43Z

+1

for #23371 56a39ea

code changes are in line with the PR description; the changes affect only Run2_2018_pp_on_AA era
jenkins tests pass
local tests on 200 events of wf 158.0 (using Run2_2018_pp_on_AA) show expected behavior: OOT-related modules are gone from the execution paths. Savings on this sample are rather minimal, adding up to about 30 ms/ev out of about 9 s/ev in reco modules total.

@mandrenguyen was the OOT a significant contributor in some of your tests? It seems to me that the strategy to file away small contributions from pp like this is not particularly practical.

fabiocos · 2018-06-06T07:29:05Z

+1

fabiocos · 2018-06-06T07:29:15Z

merge

mandrenguyen · 2018-06-07T12:41:43Z

@slava77 I tested the timing together with other customizations that are included in #23404, as well as in a forthcoming PR.
The full set I tested is listed here:
https://twiki.cern.ch/twiki/pub/CMS/HiEgamma2018/egamma_customisations.txt
I would have to dig up the total savings, which are not negligible.

We decided to split this into several PRs. Unfortunately, this one doesn't seem to be save much.

mandrenguyen added 4 commits May 29, 2018 16:25

Remove OOT photons from 2018 heavy-ion (AA) era

4527fac

had forgotten some couts

c186f54

more comment cleanups

5064720

remove unnecessary for loop

414283d

cmsbuild added this to the CMSSW_10_2_X milestone May 29, 2018

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels May 29, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels May 29, 2018

slava77 reviewed May 29, 2018

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels May 29, 2018

cmsbuild added tests-approved and removed tests-started labels May 29, 2018

cmsbuild added comparison-available and removed comparison-pending labels May 29, 2018

mandrenguyen added 2 commits May 29, 2018 23:33

addressed Slava's comments

86838b9

Cleaning of oot photons from mini-AOD

1241d04

cmsbuild added code-checks-pending comparison-pending tests-pending and removed comparison-available tests-approved labels Jun 5, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 5, 2018

cmsbuild added tests-started and removed tests-pending labels Jun 5, 2018

cmsbuild added tests-approved and removed tests-started labels Jun 5, 2018

cmsbuild added comparison-available and removed comparison-pending labels Jun 5, 2018

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jun 5, 2018

cmsbuild added orp-approved and removed orp-pending labels Jun 6, 2018

cmsbuild merged commit 96f61ee into cms-sw:master Jun 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove OOT photons from 2018 heavy-ion (AA) era #23371

Remove OOT photons from 2018 heavy-ion (AA) era #23371

mandrenguyen commented May 29, 2018

cmsbuild commented May 29, 2018

cmsbuild commented May 29, 2018

cmsbuild commented May 29, 2018

slava77 May 29, 2018

slava77 May 29, 2018

slava77 May 29, 2018

slava77 May 29, 2018

slava77 May 29, 2018

slava77 commented May 29, 2018

cmsbuild commented May 29, 2018 •

edited

cmsbuild commented May 29, 2018

cmsbuild commented May 29, 2018

cmsbuild commented May 29, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

slava77 commented Jun 5, 2018

cmsbuild commented Jun 5, 2018 •

edited

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

slava77 commented Jun 5, 2018 •

edited

fabiocos commented Jun 6, 2018

fabiocos commented Jun 6, 2018

mandrenguyen commented Jun 7, 2018

Remove OOT photons from 2018 heavy-ion (AA) era #23371

Remove OOT photons from 2018 heavy-ion (AA) era #23371

Conversation

mandrenguyen commented May 29, 2018

cmsbuild commented May 29, 2018

cmsbuild commented May 29, 2018

cmsbuild commented May 29, 2018

slava77 May 29, 2018

Choose a reason for hiding this comment

slava77 May 29, 2018

Choose a reason for hiding this comment

slava77 May 29, 2018

Choose a reason for hiding this comment

slava77 May 29, 2018

Choose a reason for hiding this comment

slava77 May 29, 2018

Choose a reason for hiding this comment

slava77 commented May 29, 2018

cmsbuild commented May 29, 2018 • edited

cmsbuild commented May 29, 2018

cmsbuild commented May 29, 2018

cmsbuild commented May 29, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

slava77 commented Jun 5, 2018

cmsbuild commented Jun 5, 2018 • edited

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

slava77 commented Jun 5, 2018 • edited

fabiocos commented Jun 6, 2018

fabiocos commented Jun 6, 2018

mandrenguyen commented Jun 7, 2018

cmsbuild commented May 29, 2018 •

edited

cmsbuild commented Jun 5, 2018 •

edited

slava77 commented Jun 5, 2018 •

edited