Added esConsumes calls to more Muon related code #35697

Dr15Jones · 2021-10-15T18:31:31Z

PR description:

added esConsumes calls. This required propagating ConsumesCollector to many helper classes
converted related modules away from legacy base classes

PR validation:

Code compiles.

- Also updated some modules to no longer be legacy

cmsbuild · 2021-10-15T18:42:42Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35697/25999

This PR adds an extra 108KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Alignment/MuonAlignmentAlgorithms/test/StandAloneTest.cc modified in PR(s): [Draft] esConsumes migration #35659

cmsbuild · 2021-10-15T18:43:09Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

Alignment/CommonAlignmentMonitor (alca)
Alignment/MuonAlignmentAlgorithms (alca)
DQM/TrackingMonitor (dqm)
RecoMTD/TrackExtender (upgrade, reconstruction)
RecoMuon/GlobalMuonProducer (reconstruction)
RecoMuon/GlobalTrackingTools (reconstruction)
RecoMuon/StandAloneMuonProducer (reconstruction)
RecoMuon/StandAloneTrackFinder (reconstruction)
RecoMuon/TrackingTools (reconstruction)
TrackingTools/TrackRefitter (reconstruction)

@malbouis, @yuanchao, @pmandrik, @emanueleusai, @ahmad3213, @tvami, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @slava77, @jpata, @francescobrivio, @pbo0, @rvenditti can you please review it and eventually sign? Thanks.
@felicepantaleo, @abbiendi, @fioriNTU, @tlampen, @threus, @mmusich, @dgulhan, @pakhotin, @hdelanno, @JanFSchulte, @jhgoh, @amagitte, @sscruz, @cericeci, @trocino, @rociovilar, @sroychow, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @tocheng, @jandrea, @idebruyn, @ebrondol, @mtosi, @fabiocos, @arossi83, @HuguesBrun, @adewit, @Fedespring, @calderona, @lecriste, @gpetruc 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

Dr15Jones · 2021-10-15T18:47:38Z

please test

tvami · 2021-10-15T19:20:06Z

RecoMuon/StandAloneTrackFinder/src/StandAloneTrajectoryBuilder.cc

@@ -101,7 +101,7 @@ StandAloneMuonTrajectoryBuilder::StandAloneMuonTrajectoryBuilder(const Parameter
  // The seed transformer (used to refit the seed and get the seed transient state)
  //  ParameterSet seedTransformerPSet = par.getParameter<ParameterSet>("SeedTransformerParameters");
  ParameterSet seedTransformerParameters = par.getParameter<ParameterSet>("SeedTransformerParameters");
-  theSeedTransformer = new SeedTransformer(seedTransformerParameters);
+  theSeedTransformer = new SeedTransformer(seedTransformerParameters, iC);


Can we avoid the new here and move to a smart pointer? I also don't see where this is deleted in case it's kept.

The objects are delete in the destructor

cmssw/RecoMuon/StandAloneTrackFinder/src/StandAloneTrajectoryBuilder.cc

Lines 113 to 125 in aefd5fd

StandAloneMuonTrajectoryBuilder::~StandAloneMuonTrajectoryBuilder() {

LogTrace("Muon|RecoMuon|StandAloneMuonTrajectoryBuilder")

<< "StandAloneMuonTrajectoryBuilder destructor called" << endl;

if (theFilter)

delete theFilter;

if (doBackwardFilter && theBWFilter)

delete theBWFilter;

if (doRefit && theRefitter)

delete theRefitter;

if (theSeedTransformer)

delete theSeedTransformer;

}

I did make the change. In general, since I am not the maintainer of this code, I'd prefer to only make changes which pertain directly to the work of the pull request.

cmsbuild · 2021-10-15T21:25:29Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35697/26001

This PR adds an extra 108KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Alignment/MuonAlignmentAlgorithms/test/StandAloneTest.cc modified in PR(s): [Draft] esConsumes migration #35659

cmsbuild · 2021-10-15T21:25:53Z

Pull request #35697 was updated. @malbouis, @yuanchao, @pmandrik, @emanueleusai, @ahmad3213, @tvami, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @slava77, @jpata, @francescobrivio, @pbo0, @rvenditti can you please check and sign again.

Dr15Jones · 2021-10-15T21:41:17Z

Please test

cmsbuild · 2021-10-16T01:20:40Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-59d355/19670/summary.html
COMMIT: 764854b
CMSSW: CMSSW_12_1_X_2021-10-15-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35697/19670/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: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 2751113
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2751085
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 39 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 170 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

emanueleusai · 2021-10-16T03:26:50Z

+1

tvami · 2021-10-16T12:41:37Z

+alca

tests pass, comparison diff in MsgLogger only
SA shows several warnings, including other es get() functions, but this is to be handled beyond this PR

srimanob · 2021-10-16T12:50:02Z

+Upgrade

slava77 · 2021-10-18T18:07:05Z

+reconstruction

for #35697 764854b

code changes are technical, in line with the PR description
jenkins tests pass

cmsbuild · 2021-10-18T18:07:29Z

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)

qliphy · 2021-10-19T00:39:16Z

+1

removed unnecessary includes

7e11783

cmsbuild added this to the CMSSW_12_1_X milestone Oct 15, 2021

cmsbuild added alca-pending code-checks-pending dqm-pending orp-pending pending-signatures reconstruction-pending tests-pending upgrade-pending labels Oct 15, 2021

Added esConsumes calls to Muon related code

4e51b62

- Also updated some modules to no longer be legacy

Dr15Jones force-pushed the esConsumesMuonLeftovers branch from 70322f6 to 4e51b62 Compare October 15, 2021 18:35

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 15, 2021

cmsbuild added tests-started and removed tests-pending labels Oct 15, 2021

tvami reviewed Oct 15, 2021

View reviewed changes

switched to using std::unique_ptr for memory management

764854b

cmsbuild added code-checks-pending tests-pending and removed tests-started code-checks-approved labels Oct 15, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 15, 2021

cmsbuild added tests-started and removed tests-pending labels Oct 15, 2021

cmsbuild added tests-approved and removed tests-started labels Oct 16, 2021

cmsbuild added dqm-approved and removed dqm-pending labels Oct 16, 2021

cmsbuild added alca-approved and removed alca-pending labels Oct 16, 2021

cmsbuild added upgrade-approved and removed upgrade-pending labels Oct 16, 2021

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Oct 18, 2021

cmsbuild mentioned this pull request Oct 18, 2021

Migrate to esConsumes muon alignment #35719

Merged

cmsbuild added orp-approved and removed orp-pending labels Oct 19, 2021

cmsbuild merged commit 87a4087 into cms-sw:master Oct 19, 2021

Dr15Jones deleted the esConsumesMuonLeftovers branch October 20, 2021 20:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added esConsumes calls to more Muon related code #35697

Added esConsumes calls to more Muon related code #35697

Dr15Jones commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

Dr15Jones commented Oct 15, 2021

tvami Oct 15, 2021

Dr15Jones Oct 15, 2021

Dr15Jones Oct 15, 2021

cmsbuild commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

Dr15Jones commented Oct 15, 2021

cmsbuild commented Oct 16, 2021

emanueleusai commented Oct 16, 2021

tvami commented Oct 16, 2021

srimanob commented Oct 16, 2021

slava77 commented Oct 18, 2021

cmsbuild commented Oct 18, 2021

qliphy commented Oct 19, 2021

	StandAloneMuonTrajectoryBuilder::~StandAloneMuonTrajectoryBuilder() {
	LogTrace("Muon\|RecoMuon\|StandAloneMuonTrajectoryBuilder")
	<< "StandAloneMuonTrajectoryBuilder destructor called" << endl;

	if (theFilter)
	delete theFilter;
	if (doBackwardFilter && theBWFilter)
	delete theBWFilter;
	if (doRefit && theRefitter)
	delete theRefitter;
	if (theSeedTransformer)
	delete theSeedTransformer;
	}

Added esConsumes calls to more Muon related code #35697

Added esConsumes calls to more Muon related code #35697

Conversation

Dr15Jones commented Oct 15, 2021

PR description:

PR validation:

cmsbuild commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

Dr15Jones commented Oct 15, 2021

tvami Oct 15, 2021

Choose a reason for hiding this comment

Dr15Jones Oct 15, 2021

Choose a reason for hiding this comment

Dr15Jones Oct 15, 2021

Choose a reason for hiding this comment

cmsbuild commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

Dr15Jones commented Oct 15, 2021

cmsbuild commented Oct 16, 2021

Comparison Summary

emanueleusai commented Oct 16, 2021

tvami commented Oct 16, 2021

srimanob commented Oct 16, 2021

slava77 commented Oct 18, 2021

cmsbuild commented Oct 18, 2021

qliphy commented Oct 19, 2021