Partial esConsumes migration for TrajectorySeedProducer #35137

wddgit · 2021-09-02T22:17:50Z

PR description:

Partial esConsumes migration for TrajectorySeedProducer. Excludes migration related to MultipleScatteringParametrisation and PixelRecoUtilities because Matti is working on that separately.

PR validation:

Relies on existing tests.

Excludes migration related to MultipleScatteringParametrisation and PixelRecoUtilities because Matti is working on that separately.

cmsbuild · 2021-09-02T22:23:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35137/25047

This PR adds an extra 64KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File FastSimulation/Tracking/src/SeedFinderSelector.cc modified in PR(s): Move "inverse of Bz at origin in GeV" from PixelRecoUtilities to MagneticField #35081
- File RecoPixelVertexing/PixelTriplets/plugins/PixelTripletLargeTipGenerator.cc modified in PR(s): Move "inverse of Bz at origin in GeV" from PixelRecoUtilities to MagneticField #35081
- File RecoTracker/TkSeedGenerator/plugins/MultiHitGeneratorFromChi2.cc modified in PR(s): Move "inverse of Bz at origin in GeV" from PixelRecoUtilities to MagneticField #35081

cmsbuild · 2021-09-02T22:24:01Z

A new Pull Request was created by @wddgit (W. David Dagenhart) for master.

It involves the following packages:

FastSimulation/Tracking (fastsim)
RecoMuon/TrackerSeedGenerator (reconstruction)
RecoPixelVertexing/PixelLowPtUtilities (reconstruction)
RecoPixelVertexing/PixelTriplets (reconstruction)
RecoTracker/SpecialSeedGenerators (reconstruction)
RecoTracker/TkSeedGenerator (reconstruction)

@civanch, @lveldere, @sbein, @ssekmen, @mdhildreth, @cmsbuild, @slava77, @jpata can you please review it and eventually sign? Thanks.
@sscruz, @jhgoh, @trocino, @calderona, @felicepantaleo, @abbiendi, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @bellan, @HuguesBrun, @ebrondol, @Fedespring, @gpetruc, @matt-komm, @mmusich, @mtosi, @dgulhan, @cericeci, @rociovilar 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

wddgit · 2021-09-02T22:24:06Z

please test

wddgit · 2021-09-02T22:43:18Z

I just looked. There are some near misses with #35081 , but no actual conflicting lines. Possibly the lines are so close in a couple places git will complain and one PR or the other will need a trivial rebase.

cmsbuild · 2021-09-03T01:54:51Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c05cec/18273/summary.html
COMMIT: 48460c4
CMSSW: CMSSW_12_1_X_2021-09-02-1600/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35137/18273/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-c05cec/18273/llvm-analysis/esrget-sa.txt for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000404
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3000382
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

slava77 · 2021-09-08T15:59:46Z

RecoTracker/TkSeedGenerator/plugins/SeedGeneratorFromProtoTracksEDProducer.cc

@@ -143,10 +144,8 @@ void SeedGeneratorFromProtoTracksEDProducer::produce(edm::Event& ev, const edm::
            sqrt(proto.momentum().x() * proto.momentum().x() + proto.momentum().y() * proto.momentum().y());


SA points to es.get<TransientRecHitRecord>().get(builderName, ttrhbESH); a few lines above.
Can this be migrated as well?

Sure. I can migrate that module also. I was originally just focusing on the module TrajectorySeedProducer and touched this module as a knock-on because the change to SeedCreator affected it. I'll push a commit soon.

After looking at this more, I realized ttrhbESH is not used, so I just deleted it (I assumed there was not some strange side effect to running the EventSetup get function). Note that I just fixed the one thing referenced in the PR comment. I didn't migrate SeedFromProtoTrack which also uses EventSetup in this module. I could if requested, but that is used elsewhere and possibly would be better handled in a separate PR, if in fact that block of code is ever run. I'll migrate more if requested.

cmsbuild · 2021-09-08T17:27:02Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35137/25139

This PR adds an extra 64KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File FastSimulation/Tracking/src/SeedFinderSelector.cc modified in PR(s): Move "inverse of Bz at origin in GeV" from PixelRecoUtilities to MagneticField #35081
- File RecoPixelVertexing/PixelTriplets/plugins/PixelTripletLargeTipGenerator.cc modified in PR(s): Move "inverse of Bz at origin in GeV" from PixelRecoUtilities to MagneticField #35081
- File RecoTracker/TkSeedGenerator/plugins/MultiHitGeneratorFromChi2.cc modified in PR(s): Move "inverse of Bz at origin in GeV" from PixelRecoUtilities to MagneticField #35081

cmsbuild · 2021-09-08T17:27:26Z

Pull request #35137 was updated. @civanch, @lveldere, @sbein, @ssekmen, @mdhildreth, @cmsbuild, @slava77, @jpata can you please check and sign again.

wddgit · 2021-09-08T17:28:51Z

please test

slava77 · 2021-09-08T22:35:23Z

is the build system particularly busy now or did the build system get stuck for this PR?
it looks like this PR was in Pending — Waiting for tests to start for a few hours now.

slava77 · 2021-09-09T00:23:36Z

is the build system particularly busy now or did the build system get stuck for this PR?
it looks like this PR was in Pending — Waiting for tests to start for a few hours now.

the last "comparison summary" was posted more than 7 hrs ago in the cms-sw repos.
It looks like something is broken.
@cms-sw/core-l2

cmsbuild · 2021-09-09T09:02:48Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c05cec/18419/summary.html
COMMIT: 107d3e7
CMSSW: CMSSW_12_1_X_2021-09-08-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35137/18419/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: 39
DQMHistoTests: Total histograms compared: 3001001
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3000979
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

slava77 · 2021-09-09T15:00:48Z

+reconstruction

for #35137 107d3e7

code changes are technical, in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no differences

makortel · 2021-09-10T13:35:44Z

@cms-sw/fastsim-l2 Could you review and sign, please?

civanch · 2021-09-10T17:51:54Z

+1

cmsbuild · 2021-09-10T17:52:13Z

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 · 2021-09-10T19:26:18Z

+1

Partial esConsumes migration for TrajectorySeedProducer

48460c4

Excludes migration related to MultipleScatteringParametrisation and PixelRecoUtilities because Matti is working on that separately.

cmsbuild added this to the CMSSW_12_1_X milestone Sep 2, 2021

cmsbuild added code-checks-pending fastsim-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Sep 2, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 2, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 2, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 3, 2021

cmsbuild mentioned this pull request Sep 3, 2021

Move "inverse of Bz at origin in GeV" from PixelRecoUtilities to MagneticField #35081

Merged

makortel mentioned this pull request Sep 4, 2021

Migrate FastSim EDModules to esConsumes cms-sw/framework-team#243

Closed

2 tasks

slava77 reviewed Sep 8, 2021

View reviewed changes

Delete unused EventSetup get call

107d3e7

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Sep 8, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 8, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 8, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 9, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Sep 9, 2021

cmsbuild added fastsim-approved fully-signed and removed fastsim-pending pending-signatures labels Sep 10, 2021

cmsbuild added orp-approved and removed orp-pending labels Sep 10, 2021

cmsbuild merged commit e2c2c5a into cms-sw:master Sep 10, 2021

This was referenced Sep 11, 2021

add 11634.7 wf for 2021 ttbar mkFit to the short matrix #35235

Merged

modernize RecoVertex/BeamSpotProducer #35236

Merged

wddgit deleted the esConsumesMigrationTrajectorySeed1 branch May 2, 2022 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partial esConsumes migration for TrajectorySeedProducer #35137

Partial esConsumes migration for TrajectorySeedProducer #35137

wddgit commented Sep 2, 2021

cmsbuild commented Sep 2, 2021

cmsbuild commented Sep 2, 2021

wddgit commented Sep 2, 2021

wddgit commented Sep 2, 2021

cmsbuild commented Sep 3, 2021

slava77 Sep 8, 2021

wddgit Sep 8, 2021

wddgit Sep 8, 2021

cmsbuild commented Sep 8, 2021

cmsbuild commented Sep 8, 2021

wddgit commented Sep 8, 2021

slava77 commented Sep 8, 2021

slava77 commented Sep 9, 2021

cmsbuild commented Sep 9, 2021

slava77 commented Sep 9, 2021

makortel commented Sep 10, 2021

civanch commented Sep 10, 2021

cmsbuild commented Sep 10, 2021

perrotta commented Sep 10, 2021

		@@ -143,10 +144,8 @@ void SeedGeneratorFromProtoTracksEDProducer::produce(edm::Event& ev, const edm::
		sqrt(proto.momentum().x() * proto.momentum().x() + proto.momentum().y() * proto.momentum().y());

Partial esConsumes migration for TrajectorySeedProducer #35137

Partial esConsumes migration for TrajectorySeedProducer #35137

Conversation

wddgit commented Sep 2, 2021

PR description:

PR validation:

cmsbuild commented Sep 2, 2021

cmsbuild commented Sep 2, 2021

wddgit commented Sep 2, 2021

wddgit commented Sep 2, 2021

cmsbuild commented Sep 3, 2021

Comparison Summary

slava77 Sep 8, 2021

Choose a reason for hiding this comment

wddgit Sep 8, 2021

Choose a reason for hiding this comment

wddgit Sep 8, 2021

Choose a reason for hiding this comment

cmsbuild commented Sep 8, 2021

cmsbuild commented Sep 8, 2021

wddgit commented Sep 8, 2021

slava77 commented Sep 8, 2021

slava77 commented Sep 9, 2021

cmsbuild commented Sep 9, 2021

Comparison Summary

slava77 commented Sep 9, 2021

makortel commented Sep 10, 2021

civanch commented Sep 10, 2021

cmsbuild commented Sep 10, 2021

perrotta commented Sep 10, 2021