Remove CMS deprecated warnings from RecoRomanPot/RecoFP420 #36586

Dr15Jones · 2021-12-23T21:54:24Z

PR description:

Reworked some helper classes to be const correct
Removed unnecessary includes
Moved away from legacy module types

PR validation:

Code compiles and does not issue warnings anymore.

The classes now only have const member functions.

Dr15Jones · 2021-12-23T21:54:42Z

@makortel FYI

cmsbuild · 2021-12-23T22:00:56Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36586/27550

This PR adds an extra 44KB to repository

cmsbuild · 2021-12-23T22:01:17Z

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

It involves the following packages:

RecoRomanPot/RecoFP420 (reconstruction)

@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2021-12-23T22:01:48Z

please test

cmsbuild · 2021-12-24T01:55:28Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-820924/21462/summary.html
COMMIT: 59b1946
CMSSW: CMSSW_12_3_X_2021-12-23-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36586/21462/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: 43
DQMHistoTests: Total histograms compared: 3461692
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461664
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

slava77 · 2021-12-29T14:40:03Z

RecoRomanPot/RecoFP420/interface/FP420ClusterMain.h


 private:
-  ClusterProducerFP420 *threeThreshold_;
+  std::unique_ptr<const ClusterProducerFP420> threeThreshold_;
  std::string clusterMode_;

  //std::vector<HDigiFP420> collector;


edm::ParameterSet conf_; could be removed (a line below)

slava77 · 2021-12-29T14:50:44Z

RecoRomanPot/RecoFP420/interface/FP420TrackMain.h

@@ -24,7 +17,7 @@ class FP420TrackMain {
  ~FP420TrackMain();

  /// Runs the algorithm
-  void run(edm::Handle<ClusterCollectionFP420> &input, TrackCollectionFP420 *toutput);
+  void run(edm::Handle<ClusterCollectionFP420> &input, TrackCollectionFP420 *toutput) const;

 private:
  edm::ParameterSet conf_;


it seems like the conf_ can be dropped

Suggested change

edm::ParameterSet conf_;

shouldn't TrackProducerFP420 *finderParameters_; (one line below) be made const?

- removed unnecessary member data - const improvement

cmsbuild · 2021-12-29T15:53:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36586/27589

This PR adds an extra 20KB to repository

cmsbuild · 2021-12-29T15:54:04Z

Pull request #36586 was updated. @jpata, @cmsbuild, @clacaputo, @slava77 can you please check and sign again.

Dr15Jones · 2021-12-29T16:22:46Z

please test

cmsbuild · 2021-12-29T19:49:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-820924/21501/summary.html
COMMIT: ef611c4
CMSSW: CMSSW_12_3_X_2021-12-29-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36586/21501/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: 43
DQMHistoTests: Total histograms compared: 3461692
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461670
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

slava77 · 2021-12-30T14:49:58Z

+reconstruction

for #36586 ef611c4

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

cmsbuild · 2021-12-30T14:50:23Z

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-12-30T16:56:10Z

+1

Dr15Jones added 3 commits December 23, 2021 15:48

Remove/add appropriate headers

96cec5c

Make classes const correct

7bd3ed1

The classes now only have const member functions.

Move away from legacy module types

59b1946

cmsbuild added this to the CMSSW_12_3_X milestone Dec 23, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Dec 23, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 23, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 23, 2021

cmsbuild added tests-approved and removed tests-started labels Dec 24, 2021

makortel mentioned this pull request Dec 28, 2021

Various remove legacy module types cms-sw/framework-team#340

Closed

4 tasks

slava77 reviewed Dec 29, 2021

View reviewed changes

Changes based on code review comments

ef611c4

- removed unnecessary member data - const improvement

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Dec 29, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 29, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 29, 2021

cmsbuild added tests-approved and removed tests-started labels Dec 29, 2021

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Dec 30, 2021

cmsbuild added orp-approved and removed orp-pending labels Dec 30, 2021

cmsbuild merged commit ef7ab7a into cms-sw:master Dec 30, 2021

Dr15Jones deleted the deprecated_RecoRomanPotRecoFP420 branch January 6, 2022 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove CMS deprecated warnings from RecoRomanPot/RecoFP420 #36586

Remove CMS deprecated warnings from RecoRomanPot/RecoFP420 #36586

Dr15Jones commented Dec 23, 2021

Dr15Jones commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

Dr15Jones commented Dec 23, 2021

cmsbuild commented Dec 24, 2021

slava77 Dec 29, 2021

slava77 Dec 29, 2021

cmsbuild commented Dec 29, 2021

cmsbuild commented Dec 29, 2021

Dr15Jones commented Dec 29, 2021

cmsbuild commented Dec 29, 2021

slava77 commented Dec 30, 2021

cmsbuild commented Dec 30, 2021

perrotta commented Dec 30, 2021

Remove CMS deprecated warnings from RecoRomanPot/RecoFP420 #36586

Remove CMS deprecated warnings from RecoRomanPot/RecoFP420 #36586

Conversation

Dr15Jones commented Dec 23, 2021

PR description:

PR validation:

Dr15Jones commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

Dr15Jones commented Dec 23, 2021

cmsbuild commented Dec 24, 2021

Comparison Summary

slava77 Dec 29, 2021

Choose a reason for hiding this comment

slava77 Dec 29, 2021

Choose a reason for hiding this comment

cmsbuild commented Dec 29, 2021

cmsbuild commented Dec 29, 2021

Dr15Jones commented Dec 29, 2021

cmsbuild commented Dec 29, 2021

Comparison Summary

slava77 commented Dec 30, 2021

cmsbuild commented Dec 30, 2021

perrotta commented Dec 30, 2021