Migrate rest of RecoLocalTracker to esConsumes #33123

mmusich · 2021-03-09T21:13:37Z

PR description:

Title says it all, follows the migrations done for Strips at PR #31826 and for Pixels at PR #32093.

PR validation:

It compiles.

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

Not a backport, no backport is needed.

cmsbuild · 2021-03-09T21:20:06Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33123/21474

This PR adds an extra 28KB to repository

cmsbuild · 2021-03-09T21:20:28Z

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

RecoLocalTracker/Phase2TrackerRecHits
RecoLocalTracker/SubCollectionProducers

@perrotta, @kpedro88, @cmsbuild, @srimanob, @slava77, @jpata can you please review it and eventually sign? Thanks.
@mtosi, @echabert, @pieterdavid, @GiacomoSguazzoni, @makortel, @felicepantaleo, @yduhm, @robervalwalsh, @JanFSchulte, @rovere, @gbenelli, @gpetruc, @alesaggio, @ebrondol, @VinInn, @threus this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

perrotta · 2021-03-10T07:29:36Z

@cmsbuild please test

cmsbuild · 2021-03-10T10:01:57Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-817052/13397/summary.html
COMMIT: 733ea9b
CMSSW: CMSSW_11_3_X_2021-03-09-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33123/13397/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2849195
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2849170
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files

slava77 · 2021-03-10T15:24:08Z

RecoLocalTracker/SubCollectionProducers/src/SeedClusterRemover.cc

@@ -111,7 +111,9 @@ void SeedClusterRemover::readPSet(
 }

 SeedClusterRemover::SeedClusterRemover(const ParameterSet &iConfig)
-    : doStrip_(iConfig.existsAs<bool>("doStrip") ? iConfig.getParameter<bool>("doStrip") : true),
+    : tTrackerGeom_(
+          esConsumes<TrackerGeometry, TrackerDigiGeometryRecord>(edm::ESInputTag("", ""))),  //is it correct to use "" ?


the original comment was made 6 years ago.
65ded94#diff-34ddaf02250754d45c345cbe666b5e9ef429c81f61cc386f5a2761a1fc7b9829R385

is it reasonable to expect that the comment is not necessary anymore? (there is no default argument value to escape passing something)

I suppose it can. If I remove the comment, I think I can remove entirely the ESInputTag as I think that ("","") it's the default.

OK then, even simpler/shorter

slava77 · 2021-03-10T15:26:39Z

RecoLocalTracker/SubCollectionProducers/src/SeedClusterRemoverPhase2.cc

@@ -61,7 +61,9 @@ using namespace std;
 using namespace edm;

 SeedClusterRemoverPhase2::SeedClusterRemoverPhase2(const ParameterSet &iConfig)
-    : doOuterTracker_(iConfig.existsAs<bool>("doOuterTracker") ? iConfig.getParameter<bool>("doOuterTracker") : true),
+    : tTrackerGeom_(
+          esConsumes<TrackerGeometry, TrackerDigiGeometryRecord>(edm::ESInputTag("", ""))),  //is it correct to use "" ?


the comment looks like a copy of that from the non-Phase2 variant.
is it reasonable to expect that the comment is not necessary anymore?

cmsbuild · 2021-03-10T16:10:31Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33123/21492

This PR adds an extra 16KB to repository

cmsbuild · 2021-03-10T16:10:58Z

Pull request #33123 was updated. @perrotta, @kpedro88, @cmsbuild, @srimanob, @slava77, @jpata can you please check and sign again.

slava77 · 2021-03-10T16:35:57Z

@cmsbuild please test

cmsbuild · 2021-03-10T19:05:29Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-817052/13414/summary.html
COMMIT: 0a8736c
CMSSW: CMSSW_11_3_X_2021-03-09-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33123/13414/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: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2849195
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2849166
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 37 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 160 log files, 37 edm output root files, 38 DQM output files

slava77 · 2021-03-11T16:36:26Z

+1

for #33123 0a8736c

code changes are in line with the PR description and the follow up review; no physics changes are expected
jenkins tests pass and comparisons with the baseline show no (relevant) differences

srimanob · 2021-03-13T11:40:11Z

+Upgrade

Migration PR, no change expected in Physics result.

cmsbuild · 2021-03-13T11:40:33Z

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

silviodonato · 2021-03-13T11:44:32Z

+1

migrate Phase2TrackerRecHits to esConsumes

0754a5d

cmsbuild added this to the CMSSW_11_3_X milestone Mar 9, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending upgrade-pending labels Mar 9, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 9, 2021

cmsbuild added tests-started and removed tests-pending labels Mar 10, 2021

cmsbuild added tests-approved and removed tests-started labels Mar 10, 2021

slava77 reviewed Mar 10, 2021

View reviewed changes

migrate RecoLocalTracker/SubCollectionProducers to esConsumes

0a8736c

mmusich force-pushed the RecoLocalTrackerEsConsumes_otherpackages branch from 733ea9b to 0a8736c Compare March 10, 2021 16:02

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Mar 10, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 10, 2021

cmsbuild added tests-started and removed tests-pending labels Mar 10, 2021

cmsbuild added tests-approved and removed tests-started labels Mar 10, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Mar 11, 2021

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Mar 13, 2021

cmsbuild added orp-approved and removed orp-pending labels Mar 13, 2021

cmsbuild merged commit 86ed681 into cms-sw:master Mar 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate rest of RecoLocalTracker to esConsumes #33123

Migrate rest of RecoLocalTracker to esConsumes #33123

mmusich commented Mar 9, 2021

cmsbuild commented Mar 9, 2021

cmsbuild commented Mar 9, 2021

perrotta commented Mar 10, 2021

cmsbuild commented Mar 10, 2021

slava77 Mar 10, 2021

mmusich Mar 10, 2021

slava77 Mar 10, 2021

slava77 Mar 10, 2021

mmusich Mar 10, 2021

cmsbuild commented Mar 10, 2021

cmsbuild commented Mar 10, 2021

slava77 commented Mar 10, 2021

cmsbuild commented Mar 10, 2021

slava77 commented Mar 11, 2021

srimanob commented Mar 13, 2021

cmsbuild commented Mar 13, 2021

silviodonato commented Mar 13, 2021

Migrate rest of RecoLocalTracker to esConsumes #33123

Migrate rest of RecoLocalTracker to esConsumes #33123

Conversation

mmusich commented Mar 9, 2021

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 Mar 9, 2021

cmsbuild commented Mar 9, 2021

perrotta commented Mar 10, 2021

cmsbuild commented Mar 10, 2021

Comparison Summary

slava77 Mar 10, 2021

Choose a reason for hiding this comment

mmusich Mar 10, 2021

Choose a reason for hiding this comment

slava77 Mar 10, 2021

Choose a reason for hiding this comment

slava77 Mar 10, 2021

Choose a reason for hiding this comment

mmusich Mar 10, 2021

Choose a reason for hiding this comment

cmsbuild commented Mar 10, 2021

cmsbuild commented Mar 10, 2021

slava77 commented Mar 10, 2021

cmsbuild commented Mar 10, 2021

Comparison Summary

slava77 commented Mar 11, 2021

srimanob commented Mar 13, 2021

cmsbuild commented Mar 13, 2021

silviodonato commented Mar 13, 2021