Run3-sim118B Try to replace getByLabel with token in SimCalorimetry/CastorSim #37757

bsunanda · 2022-05-01T05:27:25Z

PR description:

Try to replace getByLabel with token in SimCalorimetry/CastorSim

PR validation:

Use the runTheMatrix test workflows

if this PR is a backport please specify the original PR and why you need to backport that PR:

Nothing special

cmsbuild · 2022-05-01T05:33:03Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37757/29631

This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37757/29631/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37757/29631/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2022-05-01T05:50:38Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37757/29632

This PR adds an extra 16KB to repository

cmsbuild · 2022-05-01T05:51:01Z

A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master.

It involves the following packages:

SimCalorimetry/CastorSim (simulation)

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@rovere 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

bsunanda · 2022-05-01T17:40:16Z

@cmsbuild Please test

cmsbuild · 2022-05-01T21:17:47Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3344b0/24367/summary.html
COMMIT: ea3ff20
CMSSW: CMSSW_12_4_X_2022-05-01-0000/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37757/24367/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3704146
DQMHistoTests: Total failures: 14
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3704110
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 205 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

civanch · 2022-05-02T11:22:22Z

+1

cmsbuild · 2022-05-02T11:22:43Z

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 · 2022-05-02T12:39:42Z

SimCalorimetry/CastorSim/plugins/CastorDigiAnalyzer.cc

@@ -79,21 +79,24 @@ class CastorDigiAnalyzer : public edm::one::EDAnalyzer<> {
  CastorHitFilter castorFilter_;
  CaloHitAnalyzer castorHitAnalyzer_;
  CastorDigiStatistics castorDigiStatistics_;
-  edm::InputTag castorDigiCollectionTag_;
+  const edm::InputTag castorDigiCollectionTag_;


This does not need to be a class member

perrotta · 2022-05-02T12:40:56Z

SimCalorimetry/CastorSim/plugins/CastorDigiAnalyzer.cc

+      castorDigiCollectionTag_(conf.getParameter<edm::InputTag>("castorDigiCollectionTag")),
+      castordigiToken_(consumes<CastorDigiCollection>(castorDigiCollectionTag_)),


Suggested change

castorDigiCollectionTag_(conf.getParameter<edm::InputTag>("castorDigiCollectionTag")),

castordigiToken_(consumes<CastorDigiCollection>(castorDigiCollectionTag_)),

castordigiToken_(consumes<CastorDigiCollection>(conf.getParameter<edm::InputTag>("castorDigiCollectionTag")),

perrotta · 2022-05-02T12:43:49Z

SimCalorimetry/CastorSim/plugins/CastorDigiProducer.cc

  const edm::ESGetToken<CastorDbService, CastorDbRecord> theConditionsToken;
  const edm::ESGetToken<CaloGeometry, CaloGeometryRecord> theGeometryToken;
  edm::ESWatcher<CaloGeometryRecord> theGeometryWatcher;
+  const edm::InputTag theHitsProducerTag;


This theHitsProducerTag can also get removed.
Before doing that please notice that one getByLabel was left in

cmssw/SimCalorimetry/CastorSim/plugins/CastorDigiProducer.cc

Line 191 in ea3ff20

e.getByLabel(theHitsProducerTag, castorHandle);

The class PileupEvent has only getByLabel. Once it is upgraded to make use of token, the last reference to getByLabel can be moved out.

perrotta · 2022-05-02T12:46:03Z

SimCalorimetry/CastorSim/plugins/CastorHitAnalyzer.cc

@@ -27,21 +27,24 @@ class CastorHitAnalyzer : public edm::one::EDAnalyzer<> {
  CastorSimParameterMap simParameterMap_;
  CastorHitFilter castorFilter_;
  CaloHitAnalyzer castorAnalyzer_;
-  edm::InputTag castorRecHitCollectionTag_;
+  const edm::InputTag castorRecHitCollectionTag_;


Also this class member is not needed

perrotta · 2022-05-02T12:46:37Z

SimCalorimetry/CastorSim/plugins/CastorHitAnalyzer.cc

+      castorRecHitCollectionTag_(conf.getParameter<edm::InputTag>("castorRecHitCollectionTag")),
+      castorRecHitToken_(consumes<CastorRecHitCollection>(castorRecHitCollectionTag_)),


Suggested change

castorRecHitCollectionTag_(conf.getParameter<edm::InputTag>("castorRecHitCollectionTag")),

castorRecHitToken_(consumes<CastorRecHitCollection>(castorRecHitCollectionTag_)),

castorRecHitToken_(consumes<CastorRecHitCollection>(conf.getParameter<edm::InputTag>("castorRecHitCollectionTag"))),

cmsbuild · 2022-05-02T13:50:39Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37757/29650

This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37757/29650/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37757/29650/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2022-05-02T14:04:58Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37757/29654

This PR adds an extra 16KB to repository

cmsbuild · 2022-05-02T14:05:18Z

Pull request #37757 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again.

bsunanda · 2022-05-02T14:15:13Z

@cmsbuild Please test

cmsbuild · 2022-05-02T18:17:08Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3344b0/24389/summary.html
COMMIT: fc5123d
CMSSW: CMSSW_12_4_X_2022-05-02-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37757/24389/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3703092
DQMHistoTests: Total failures: 13
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3703056
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 48 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 205 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

civanch · 2022-05-03T08:12:47Z

+1

cmsbuild · 2022-05-03T08:13:09Z

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 · 2022-05-03T09:39:39Z

+1

Try to replace getByLabel with token in SimCalorimetry/CastorSim

ab98b57

cmsbuild added this to the CMSSW_12_4_X milestone May 1, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures simulation-pending tests-pending labels May 1, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels May 1, 2022

Code check

ea3ff20

cmsbuild added code-checks-pending and removed code-checks-rejected labels May 1, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels May 1, 2022

cmsbuild added tests-started and removed tests-pending labels May 1, 2022

cmsbuild added tests-approved and removed tests-started labels May 1, 2022

cmsbuild added fully-signed simulation-approved and removed simulation-pending pending-signatures labels May 2, 2022

perrotta reviewed May 2, 2022

View reviewed changes

cmsbuild added the tests-pending label May 2, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels May 2, 2022

Code check

fc5123d

cmsbuild added code-checks-pending and removed code-checks-rejected labels May 2, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels May 2, 2022

cmsbuild added tests-started and removed tests-pending labels May 2, 2022

cmsbuild added tests-approved and removed tests-started labels May 2, 2022

cmsbuild added fully-signed simulation-approved and removed simulation-pending pending-signatures labels May 3, 2022

cmsbuild added orp-approved and removed orp-pending labels May 3, 2022

cmsbuild merged commit a206869 into cms-sw:master May 3, 2022

This was referenced May 4, 2022

Update vecgeom to 1.1.20 cms-sw/cmsdist#7735

Merged

fix use after scope in DMRChecker::bookResidualsHistogram #37796

Merged

bsunanda deleted the Run3-sim118B branch August 20, 2022 04:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run3-sim118B Try to replace getByLabel with token in SimCalorimetry/CastorSim #37757

Run3-sim118B Try to replace getByLabel with token in SimCalorimetry/CastorSim #37757

bsunanda commented May 1, 2022

cmsbuild commented May 1, 2022

cmsbuild commented May 1, 2022

cmsbuild commented May 1, 2022

bsunanda commented May 1, 2022

cmsbuild commented May 1, 2022

civanch commented May 2, 2022

cmsbuild commented May 2, 2022

perrotta May 2, 2022

perrotta May 2, 2022

perrotta May 2, 2022

bsunanda May 2, 2022

perrotta May 2, 2022

perrotta May 2, 2022

cmsbuild commented May 2, 2022

cmsbuild commented May 2, 2022

cmsbuild commented May 2, 2022

bsunanda commented May 2, 2022

cmsbuild commented May 2, 2022

civanch commented May 3, 2022

cmsbuild commented May 3, 2022

perrotta commented May 3, 2022

		castorDigiCollectionTag_(conf.getParameter<edm::InputTag>("castorDigiCollectionTag")),
		castordigiToken_(consumes<CastorDigiCollection>(castorDigiCollectionTag_)),

		castorRecHitCollectionTag_(conf.getParameter<edm::InputTag>("castorRecHitCollectionTag")),
		castorRecHitToken_(consumes<CastorRecHitCollection>(castorRecHitCollectionTag_)),

Run3-sim118B Try to replace getByLabel with token in SimCalorimetry/CastorSim #37757

Run3-sim118B Try to replace getByLabel with token in SimCalorimetry/CastorSim #37757

Conversation

bsunanda commented May 1, 2022

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented May 1, 2022

cmsbuild commented May 1, 2022

cmsbuild commented May 1, 2022

bsunanda commented May 1, 2022

cmsbuild commented May 1, 2022

Comparison Summary

civanch commented May 2, 2022

cmsbuild commented May 2, 2022

perrotta May 2, 2022

Choose a reason for hiding this comment

perrotta May 2, 2022

Choose a reason for hiding this comment

perrotta May 2, 2022

Choose a reason for hiding this comment

bsunanda May 2, 2022

Choose a reason for hiding this comment

perrotta May 2, 2022

Choose a reason for hiding this comment

perrotta May 2, 2022

Choose a reason for hiding this comment

cmsbuild commented May 2, 2022

cmsbuild commented May 2, 2022

cmsbuild commented May 2, 2022

bsunanda commented May 2, 2022

cmsbuild commented May 2, 2022

Comparison Summary

civanch commented May 3, 2022

cmsbuild commented May 3, 2022

perrotta commented May 3, 2022