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

Migrate BMixingModule and EcalShapeBase-derived classes to esConsumes #34902

Merged
merged 4 commits into from Sep 1, 2021

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Aug 17, 2021

PR description:

Part of #31061. This PR should cover the last uses of the deprecated EventSetupRecord::get() in MixingModule and PreMixingModule. In addition I removed all bare new calls from EcalMixingModuleValidation.

In EcalDigiProducer and EcalDigiProducer_Ph2 (used by MixingModule and PreMixingModule) the changes were in APDShape, EBShape, and EBShape classes, that are also used several places outside of the MixingModules.

Within the BMixingModule infrastructure there are still some calls in DataMixingModule that would either need to be migrated or removed (in a timescale of 9 months or so). I'm not planning to touch those.

Resolves cms-sw/framework-team#221

PR validation:

Code compiles.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34902/24709

  • This PR adds an extra 108KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • CalibCalorimetry/EcalSRTools (alca)
  • CalibCalorimetry/EcalTPGTools (l1, alca)
  • Mixing/Base (simulation)
  • RecoLocalCalo/EcalRecProducers (reconstruction)
  • RecoTBCalo/EcalTBRecProducers (reconstruction)
  • SimCalorimetry/EcalSimAlgos (simulation)
  • SimCalorimetry/EcalSimProducers (simulation)
  • SimGeneral/MixingModule (simulation)
  • Validation/EcalDigis (dqm)

@perrotta, @malbouis, @civanch, @yuanchao, @kmaeshima, @tlampen, @andrius-k, @mdhildreth, @rvenditti, @cmsbuild, @rekovic, @jfernan2, @ahmad3213, @slava77, @jpata, @ErnestaP, @pohsun, @francescobrivio, @cecilecaillol, @tvami can you please review it and eventually sign? Thanks.
@rchatter, @rovere, @argiro, @apsallid, @tocheng, @thomreis, @simonepigazzini, @mmusich, @fabiocos, @slomeo 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

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-768ae1/17815/summary.html
COMMIT: 2c94140
CMSSW: CMSSW_12_1_X_2021-08-16-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34902/17815/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 7 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-768ae1/17815/llvm-analysis/esrget-sa.txt for details.
CMS StaticAnalyzer warnings: There are 1 inherits from legacy modules warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-768ae1/17815/llvm-analysis/legacy-mod-sa.txt for details.

RelVals

----- Begin Fatal Exception 17-Aug-2021 08:59:21 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module MixingModule/'mix'
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: EcalSimPulseShapeRcd
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 -------------------------------------------------
----- Begin Fatal Exception 17-Aug-2021 08:59:49 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module MixingModule/'mix'
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: EcalSimPulseShapeRcd
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 -------------------------------------------------
----- Begin Fatal Exception 17-Aug-2021 08:59:52 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module MixingModule/'mix'
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: EcalSimPulseShapeRcd
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 ...

RelVals-INPUT

  • 6.06.0_SingleMuPt1+SingleMuPt1INPUT+DIGI+RECO+HARVEST/step2_SingleMuPt1+SingleMuPt1INPUT+DIGI+RECO+HARVEST.log
  • 8.08.0_BeamHalo+BeamHaloINPUT+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step2_BeamHalo+BeamHaloINPUT+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log
  • 9.09.0_Higgs200ChargedTaus+Higgs200ChargedTausINPUT+DIGI+RECO+HARVEST/step2_Higgs200ChargedTaus+Higgs200ChargedTausINPUT+DIGI+RECO+HARVEST.log
Expand to see more relval errors ...

AddOn Tests

----- Begin Fatal Exception 17-Aug-2021 09:03:27 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 2
   [1] Calling method for module MixingModule/'mix'
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: EcalSimPulseShapeRcd
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 -------------------------------------------------
----- Begin Fatal Exception 17-Aug-2021 09:03:26 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 3
   [1] Calling method for module MixingModule/'mix'
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: EcalSimPulseShapeRcd
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 -------------------------------------------------
----- Begin Fatal Exception 17-Aug-2021 09:03:27 CEST-----------------------
An exception of category 'ESGetTokenWrongTransition' occurred while
   [0] Processing  stream begin Run run: 1 stream: 3
   [1] Calling method for module MixingModule/'mix'
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: EcalSimPulseShapeRcd
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 ...

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34902/24715

  • This PR adds an extra 112KB to repository

@cmsbuild
Copy link
Contributor

@makortel
Copy link
Contributor Author

@cmsbuild, please test

@makortel
Copy link
Contributor Author

  • The PR title and description look a bit as if the commits are not on the subject; some improvement to both may be useful

@makortel since this PR is still not merged, would you please do this?

How about now?

@makortel
Copy link
Contributor Author

ping @cms-sw/l1-l2

1 similar comment
@makortel
Copy link
Contributor Author

ping @cms-sw/l1-l2

@makortel
Copy link
Contributor Author

ping @cms-sw/l1-l2

@tvami
Copy link
Contributor

tvami commented Aug 31, 2021

Hi @perrotta @qliphy since this PR waits on the L1 signature for more than 10 days, would you consider merging the PR without it?

@rekovic
Copy link
Contributor

rekovic commented Aug 31, 2021

+1

@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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

please test
(to refresh them)

@smuzaffar
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-768ae1/18198/summary.html
COMMIT: 9ebd1a9
CMSSW: CMSSW_12_1_X_2021-08-31-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34902/18198/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 7 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-768ae1/18198/llvm-analysis/esrget-sa.txt for details.
CMS StaticAnalyzer warnings: There are 1 inherits from legacy modules warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-768ae1/18198/llvm-analysis/legacy-mod-sa.txt for details.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 3000404
  • DQMHistoTests: Total failures: 11
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3000370
  • 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

@perrotta
Copy link
Contributor

perrotta commented Sep 1, 2021

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment