Migrate EgammaSCCorrectionMaker to esConsumes() #35019

makortel · 2021-08-25T18:20:56Z

PR description:

Part of #31061.

Resolves cms-sw/framework-team#248.

PR validation:

Code compiles.

cmsbuild · 2021-08-25T18:27:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35019/24873

This PR adds an extra 36KB to repository

cmsbuild · 2021-08-25T18:28:05Z

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

RecoEcal/EgammaClusterProducers (reconstruction)
RecoEcal/EgammaCoreTools (reconstruction)
RecoEgamma/EgammaElectronProducers (reconstruction)
RecoEgamma/EgammaPhotonAlgos (reconstruction)

@jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @lecriste, @rchatter, @jainshilpi, @rovere, @lgray, @sobhatta, @thomreis, @afiqaize, @simonepigazzini, @argiro, @varuns23, @ram1123 this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

makortel · 2021-08-25T18:41:27Z

@cmsbuild, please test

cmsbuild · 2021-08-25T19:38:29Z

-1

Failed Tests: RelVals AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-563905/18035/summary.html
COMMIT: e756352
CMSSW: CMSSW_12_1_X_2021-08-25-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35019/18035/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 1 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-563905/18035/llvm-analysis/esrget-sa.txt for details.

RelVals

4.22

----- Begin Fatal Exception 25-Aug-2021 21:03:22 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 160960 stream: 0
   [1] Calling method for module GEDPhotonProducer/'photons'
Exception Message:
The transition ID stored in the ESGetToken does not match the
transition where the token is being used. The associated record
type is: EcalClusterEnergyCorrectionParametersRcd
For producers, filters and analyzers this transition ID is
set as a template parameter to the call to the esConsumes
function that creates the token. Event is the default transition.
Other possibilities are BeginRun, EndRun, BeginLuminosityBlock,
or EndLuminosityBlock. You may need multiple tokens if you want to
get the same data in multiple transitions. The transition ID has a
different meaning in ESProducers. For ESProducers, the transition
ID identifies the function that produces the EventSetup data (often
there is one function named produce but there can be multiple
functions with different names). For ESProducers, the ESGetToken
must be used in the function associated with the ESConsumesCollector
returned by the setWhatProduced function.
----- End Fatal Exception -------------------------------------------------

5.1

----- Begin Fatal Exception 25-Aug-2021 21:03:43 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module GEDPhotonProducer/'photons'
Exception Message:
The transition ID stored in the ESGetToken does not match the
transition where the token is being used. The associated record
type is: EcalClusterEnergyCorrectionParametersRcd
For producers, filters and analyzers this transition ID is
set as a template parameter to the call to the esConsumes
function that creates the token. Event is the default transition.
Other possibilities are BeginRun, EndRun, BeginLuminosityBlock,
or EndLuminosityBlock. You may need multiple tokens if you want to
get the same data in multiple transitions. The transition ID has a
different meaning in ESProducers. For ESProducers, the transition
ID identifies the function that produces the EventSetup data (often
there is one function named produce but there can be multiple
functions with different names). For ESProducers, the ESGetToken
must be used in the function associated with the ESConsumesCollector
returned by the setWhatProduced function.
----- End Fatal Exception -------------------------------------------------

135.4

----- Begin Fatal Exception 25-Aug-2021 21:03:44 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module GEDPhotonProducer/'photons'
Exception Message:
The transition ID stored in the ESGetToken does not match the
transition where the token is being used. The associated record
type is: EcalClusterEnergyCorrectionParametersRcd
For producers, filters and analyzers this transition ID is
set as a template parameter to the call to the esConsumes
function that creates the token. Event is the default transition.
Other possibilities are BeginRun, EndRun, BeginLuminosityBlock,
or EndLuminosityBlock. You may need multiple tokens if you want to
get the same data in multiple transitions. The transition ID has a
different meaning in ESProducers. For ESProducers, the transition
ID identifies the function that produces the EventSetup data (often
there is one function named produce but there can be multiple
functions with different names). For ESProducers, the ESGetToken
must be used in the function associated with the ESConsumesCollector
returned by the setWhatProduced function.
----- End Fatal Exception -------------------------------------------------

Expand to see more relval errors ...

AddOn Tests

fastsim

----- Begin Fatal Exception 25-Aug-2021 21:04:13 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 1
   [1] Calling method for module GEDPhotonProducer/'photons'
Exception Message:
The transition ID stored in the ESGetToken does not match the
transition where the token is being used. The associated record
type is: EcalClusterEnergyCorrectionParametersRcd
For producers, filters and analyzers this transition ID is
set as a template parameter to the call to the esConsumes
function that creates the token. Event is the default transition.
Other possibilities are BeginRun, EndRun, BeginLuminosityBlock,
or EndLuminosityBlock. You may need multiple tokens if you want to
get the same data in multiple transitions. The transition ID has a
different meaning in ESProducers. For ESProducers, the transition
ID identifies the function that produces the EventSetup data (often
there is one function named produce but there can be multiple
functions with different names). For ESProducers, the ESGetToken
must be used in the function associated with the ESConsumesCollector
returned by the setWhatProduced function.
----- End Fatal Exception -------------------------------------------------

fastsim1

----- Begin Fatal Exception 25-Aug-2021 21:04:12 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 2
   [1] Calling method for module GEDPhotonProducer/'photons'
Exception Message:
The transition ID stored in the ESGetToken does not match the
transition where the token is being used. The associated record
type is: EcalClusterEnergyCorrectionParametersRcd
For producers, filters and analyzers this transition ID is
set as a template parameter to the call to the esConsumes
function that creates the token. Event is the default transition.
Other possibilities are BeginRun, EndRun, BeginLuminosityBlock,
or EndLuminosityBlock. You may need multiple tokens if you want to
get the same data in multiple transitions. The transition ID has a
different meaning in ESProducers. For ESProducers, the transition
ID identifies the function that produces the EventSetup data (often
there is one function named produce but there can be multiple
functions with different names). For ESProducers, the ESGetToken
must be used in the function associated with the ESConsumesCollector
returned by the setWhatProduced function.
----- End Fatal Exception -------------------------------------------------

fastsim2

----- Begin Fatal Exception 25-Aug-2021 21:04:13 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module GEDPhotonProducer/'photons'
Exception Message:
The transition ID stored in the ESGetToken does not match the
transition where the token is being used. The associated record
type is: EcalClusterEnergyCorrectionParametersRcd
For producers, filters and analyzers this transition ID is
set as a template parameter to the call to the esConsumes
function that creates the token. Event is the default transition.
Other possibilities are BeginRun, EndRun, BeginLuminosityBlock,
or EndLuminosityBlock. You may need multiple tokens if you want to
get the same data in multiple transitions. The transition ID has a
different meaning in ESProducers. For ESProducers, the transition
ID identifies the function that produces the EventSetup data (often
there is one function named produce but there can be multiple
functions with different names). For ESProducers, the ESGetToken
must be used in the function associated with the ESConsumesCollector
returned by the setWhatProduced function.
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

…ducer::beginRun() It is called unconditionally from GEDPhotonProducer::produce(), I can not see how the call from beginRun() could do anything additional except it would require different set ESGetTokens wihtin PhotonEnergyCorrector etc compared to other users.

cmsbuild · 2021-08-25T20:07:52Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35019/24877

This PR adds an extra 44KB to repository

cmsbuild · 2021-08-25T20:08:18Z

Pull request #35019 was updated. @jpata, @cmsbuild, @slava77 can you please check and sign again.

makortel · 2021-08-25T20:08:52Z

@cmsbuild, please test

makortel · 2021-08-25T20:10:00Z

RecoEgamma/EgammaPhotonProducers/src/GEDPhotonProducer.cc

@@ -360,12 +358,6 @@ GEDPhotonProducer::GEDPhotonProducer(const edm::ParameterSet& config)
  }
 }

-void GEDPhotonProducer::beginRun(edm::Run const& r, edm::EventSetup const& eventSetup) {
-  if (!recoStep_.isFinal()) {
-    photonEnergyCorrector_->init(eventSetup);


The photonEnergyCorrector_->init() is already called unconditionally in GEDPhotonProducer::produce() so this call looks redundant. Removing it allows to keep single set of ESGetTokens in PhotonEnergyCorrector and its helper classes.

makortel · 2021-08-25T20:47:01Z

RecoEcal/EgammaCoreTools/test/testEcalClusterFunctions.cc

@@ -47,7 +45,7 @@ class testEcalClusterFunctions : public edm::EDAnalyzer {

 testEcalClusterFunctions::testEcalClusterFunctions(const edm::ParameterSet& ps) {


A side note that this module is a legacy edm::EDAnalyzer, and needs to be migrated to one of the thread-efficient base classes (assuming it is still useful).

cmsbuild · 2021-08-25T23:19:43Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-563905/18037/summary.html
COMMIT: de70d49
CMSSW: CMSSW_12_1_X_2021-08-25-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35019/18037/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 1 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-563905/18037/llvm-analysis/esrget-sa.txt for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000352
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3000324
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 38 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

thomreis · 2021-08-26T08:45:02Z

This PR seems to have some overlap with #35021

makortel · 2021-08-26T13:31:51Z

Closing in favor of #35021 that does more modernization.

Migrate EgammaSCCorrectionMaker to esConsumes()

e756352

makortel mentioned this pull request Aug 25, 2021

Migrate RecoEcal/EgammaClusterProducers to esConsumes() cms-sw/framework-team#248

Closed

cmsbuild added this to the CMSSW_12_1_X milestone Aug 25, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Aug 25, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 25, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 25, 2021

cmsbuild added tests-rejected and removed tests-started labels Aug 25, 2021

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Aug 25, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 25, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 25, 2021

makortel commented Aug 25, 2021

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Aug 25, 2021

cmsbuild mentioned this pull request Aug 26, 2021

Modernize some EGamma Reco modules #35021

Merged

makortel closed this Aug 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate EgammaSCCorrectionMaker to esConsumes() #35019

Migrate EgammaSCCorrectionMaker to esConsumes() #35019

makortel commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

makortel commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

makortel commented Aug 25, 2021

makortel Aug 25, 2021

makortel Aug 25, 2021

cmsbuild commented Aug 25, 2021

thomreis commented Aug 26, 2021

makortel commented Aug 26, 2021

		@@ -47,7 +45,7 @@ class testEcalClusterFunctions : public edm::EDAnalyzer {

		testEcalClusterFunctions::testEcalClusterFunctions(const edm::ParameterSet& ps) {

Migrate EgammaSCCorrectionMaker to esConsumes() #35019

Migrate EgammaSCCorrectionMaker to esConsumes() #35019

Conversation

makortel commented Aug 25, 2021

PR description:

PR validation:

cmsbuild commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

makortel commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

RelVals

AddOn Tests

cmsbuild commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

makortel commented Aug 25, 2021

makortel Aug 25, 2021

Choose a reason for hiding this comment

makortel Aug 25, 2021

Choose a reason for hiding this comment

cmsbuild commented Aug 25, 2021

Comparison Summary

thomreis commented Aug 26, 2021

makortel commented Aug 26, 2021