Prepare some Geometry ESProducers for concurrent IOVs #24612

wddgit · 2018-09-20T20:28:01Z

The following types of changes are included:
- changing return type of produce to unique_ptr
- removing unnecessary "dependsOn" callbacks
- some callback content moved to produce
- 4 callbacks were simply empty (did nothing)
- some objects do not need to be cached create on stack
- removing unneeded data members, includes, declarations
- fix potential assert failure if produce called more than once
- fix potential double delete if produce called more than once
- 2 memory leaks
- some general cleanup deleting unnecessary things

The following types of changes are included: - changing return type of produce to unique_ptr - removing unnecessary "dependsOn" callbacks - some callback content moved to produce - 4 callbacks were simply empty (did nothing) - some objects do not need to be cached create on stack - removing unneeded data members, includes, declarations - fix potential assert failure if produce called more than once - fix potential double delete if produce called more than once - 2 memory leaks - some general cleanup deleting unnecessary things

cmsbuild · 2018-09-20T20:28:21Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-09-20T20:30:16Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24612/6505

cmsbuild · 2018-09-20T20:30:36Z

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

It involves the following packages:

Geometry/EcalMapping
Geometry/HcalCommonData
Geometry/HcalEventSetup
Geometry/HcalTowerAlgo
Geometry/MuonNumbering

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks.
@ptcox, @argiro this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

wddgit · 2018-09-20T20:30:50Z

please test

cmsbuild · 2018-09-20T20:30:54Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30538/console Started: 2018/09/20 22:31

cmsbuild · 2018-09-20T22:44:48Z

+1
Tested at: 9ab1157
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24612/30538/summary.html

cmsbuild · 2018-09-20T22:44:50Z

Comparison job queued.

cmsbuild · 2018-09-21T00:18:43Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24612/30538/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3146746
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3146548
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 133 log files, 14 edm output root files, 32 DQM output files

ianna · 2018-09-21T10:30:06Z

Geometry/HcalCommonData/plugins/HcalDDDSimConstantsESModule.cc

 class HcalDDDSimConstantsESModule : public edm::ESProducer {

 public:
  HcalDDDSimConstantsESModule(const edm::ParameterSet&);
  ~HcalDDDSimConstantsESModule() override;

-  typedef std::shared_ptr<HcalDDDSimConstants> ReturnType;
+  typedef std::unique_ptr<HcalDDDSimConstants> ReturnType;


using ReturnType = std::unique_ptr<HcalDDDSimConstants>;

ianna · 2018-09-21T10:30:53Z

Geometry/EcalMapping/plugins/EcalElectronicsMappingBuilder.h

-
-// class EcalMappingRcd;
-
-
 // #include "FWCore/Framework/interface/EventSetupRecordIntervalFinder.h"


please, remove commented out includes

ianna · 2018-09-21T10:30:59Z

Geometry/EcalMapping/plugins/EcalElectronicsMappingBuilder.h

 // #include "FWCore/Framework/interface/EventSetupRecordIntervalFinder.h"
-
-#include "FWCore/Framework/interface/ESHandle.h"
-#include "FWCore/ParameterSet/interface/ParameterSet.h"
 // #include "Geometry/Records/interface/IdealGeometryRecord.h"


please, remove commented out includes

ianna · 2018-09-21T10:31:24Z

Geometry/EcalMapping/plugins/EcalElectronicsMappingBuilder.h

 private:
  void FillFromDatabase(const std::vector<EcalMappingElement>& ee,
                        EcalElectronicsMapping& theMap);

-
-  const EcalMappingElectronics* Mapping_ ;
  // void setIntervalFor(const edm::eventsetup::EventSetupRecordKey &, const edm::IOVSyncValue&, edm::ValidityInterval & );


please, remove commented out code

ianna · 2018-09-21T10:32:42Z

Geometry/HcalEventSetup/interface/HcalDDDGeometryEP.h

@@ -27,18 +27,15 @@ class HcalDDDGeometryEP : public edm::ESProducer {
  HcalDDDGeometryEP(const edm::ParameterSet&);
  ~HcalDDDGeometryEP() override;

-  typedef std::shared_ptr<CaloSubdetectorGeometry> ReturnType;
+  typedef std::unique_ptr<CaloSubdetectorGeometry> ReturnType;


using ReturnType = std::unique_ptr<CaloSubdetectorGeometry>;

ianna · 2018-09-21T10:33:22Z

Geometry/HcalEventSetup/interface/HcalHardcodeGeometryEP.h

@@ -19,13 +19,11 @@ class HcalHardcodeGeometryEP : public edm::ESProducer {
  HcalHardcodeGeometryEP(const edm::ParameterSet&);
  ~HcalHardcodeGeometryEP() override;

-  typedef std::shared_ptr<CaloSubdetectorGeometry> ReturnType;
+  typedef std::unique_ptr<CaloSubdetectorGeometry> ReturnType;


using ReturnType = std::unique_ptr<CaloSubdetectorGeometry>;

ianna · 2018-09-21T10:34:18Z

Geometry/HcalEventSetup/interface/HcalHardcodeGeometryEP.h


  ReturnType produceIdeal(const HcalRecNumberingRecord&);
  ReturnType produceAligned(const HcalGeometryRecord& );

-  void       idealRecordCallBack(const HcalRecNumberingRecord&) {}
-
 private:
  edm::ParameterSet ps0;


edm::ParameterSet ps0_;

ianna · 2018-09-21T10:34:48Z

Geometry/HcalEventSetup/interface/HcalTopologyIdealEP.h

@@ -28,18 +28,15 @@ class HcalTopologyIdealEP : public edm::ESProducer {
  HcalTopologyIdealEP(const edm::ParameterSet&);
  ~HcalTopologyIdealEP() override;

-  typedef std::shared_ptr<HcalTopology> ReturnType;
+  typedef std::unique_ptr<HcalTopology> ReturnType;


using ReturnType = std::unique_ptr<HcalTopology>;

ianna · 2018-09-21T10:42:24Z

Geometry/MuonNumbering/plugins/MuonNumberingInitialization.cc

@@ -39,19 +35,11 @@ class MuonNumberingInitialization : public edm::ESProducer
  typedef std::unique_ptr<MuonDDDConstants> ReturnType;


using ReturnType = std::unique_ptr<MuonDDDConstants>;

- deleting commented code and useless comments - deleting trailing whitespace - converting from typedef to using statements - removing an unused copy of a ParameterSet - removing unneeded blank lines Initially motivated by Ianna's comments. I fixed all those and did more. None of this should change any behavior

cmsbuild · 2018-09-21T19:01:55Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-09-21T19:03:56Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24612/6525

cmsbuild · 2018-09-21T19:05:35Z

Pull request #24612 was updated. @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please check and sign again.

wddgit · 2018-09-21T19:06:02Z

please test
I pushed a new fix that cleans up all the problems Ianna suggested I fix. Thanks for the suggestions. When I got going I also fixed some other similar things. So the second commit includes:

  - deleting commented code and useless comments
  - deleting trailing whitespace
  - converting from typedef to using statements
  - removing an unused copy of a ParameterSet
  - removing unneeded blank lines

Unless I made a typographical blunder, none of this should effect behavior in any way, although it does make the code easier to read.

cmsbuild · 2018-09-21T19:06:19Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30563/console Started: 2018/09/21 21:06

cmsbuild · 2018-09-21T21:36:30Z

+1
Tested at: b3ecea4
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24612/30563/summary.html

cmsbuild · 2018-09-21T21:36:33Z

Comparison job queued.

cmsbuild · 2018-09-21T22:51:48Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24612/30563/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3146746
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3146548
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 133 log files, 14 edm output root files, 32 DQM output files

ianna · 2018-09-25T13:34:06Z

+1

cmsbuild · 2018-09-25T13:34:27Z

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

fabiocos · 2018-09-25T20:49:45Z

+1

cmsbuild added this to the CMSSW_10_3_X milestone Sep 20, 2018

cmsbuild added code-checks-pending comparison-pending geometry-pending orp-pending pending-signatures tests-pending labels Sep 20, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 20, 2018

cmsbuild added tests-started and removed tests-pending labels Sep 20, 2018

cmsbuild added tests-approved and removed tests-started labels Sep 20, 2018

cmsbuild added comparison-available and removed comparison-pending labels Sep 21, 2018

ianna suggested changes Sep 21, 2018

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-approved comparison-available tests-approved labels Sep 21, 2018

cmsbuild added comparison-pending tests-pending labels Sep 21, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 21, 2018

cmsbuild added tests-started and removed tests-pending labels Sep 21, 2018

cmsbuild added tests-approved and removed tests-started labels Sep 21, 2018

cmsbuild added comparison-available and removed comparison-pending labels Sep 21, 2018

cmsbuild added fully-signed geometry-approved and removed geometry-pending pending-signatures labels Sep 25, 2018

cmsbuild added orp-approved and removed orp-pending labels Sep 25, 2018

cmsbuild merged commit 19d4bed into cms-sw:master Sep 25, 2018

wddgit deleted the geometryConcurrentIOVs branch January 14, 2019 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare some Geometry ESProducers for concurrent IOVs #24612

Prepare some Geometry ESProducers for concurrent IOVs #24612

wddgit commented Sep 20, 2018

cmsbuild commented Sep 20, 2018

cmsbuild commented Sep 20, 2018

cmsbuild commented Sep 20, 2018

wddgit commented Sep 20, 2018

cmsbuild commented Sep 20, 2018 •

edited

cmsbuild commented Sep 20, 2018

cmsbuild commented Sep 20, 2018

cmsbuild commented Sep 21, 2018

ianna Sep 21, 2018

ianna Sep 21, 2018

ianna Sep 21, 2018

ianna Sep 21, 2018

ianna Sep 21, 2018

ianna Sep 21, 2018

ianna Sep 21, 2018

ianna Sep 21, 2018

ianna Sep 21, 2018

cmsbuild commented Sep 21, 2018

cmsbuild commented Sep 21, 2018

cmsbuild commented Sep 21, 2018

wddgit commented Sep 21, 2018

cmsbuild commented Sep 21, 2018 •

edited

cmsbuild commented Sep 21, 2018

cmsbuild commented Sep 21, 2018

cmsbuild commented Sep 21, 2018

ianna commented Sep 25, 2018

cmsbuild commented Sep 25, 2018

fabiocos commented Sep 25, 2018


		// class EcalMappingRcd;


		// #include "FWCore/Framework/interface/EventSetupRecordIntervalFinder.h"

		@@ -39,19 +35,11 @@ class MuonNumberingInitialization : public edm::ESProducer
		typedef std::unique_ptr<MuonDDDConstants> ReturnType;

Prepare some Geometry ESProducers for concurrent IOVs #24612

Prepare some Geometry ESProducers for concurrent IOVs #24612

Conversation

wddgit commented Sep 20, 2018

cmsbuild commented Sep 20, 2018

cmsbuild commented Sep 20, 2018

cmsbuild commented Sep 20, 2018

wddgit commented Sep 20, 2018

cmsbuild commented Sep 20, 2018 • edited

cmsbuild commented Sep 20, 2018

cmsbuild commented Sep 20, 2018

cmsbuild commented Sep 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Sep 21, 2018

cmsbuild commented Sep 21, 2018

cmsbuild commented Sep 21, 2018

wddgit commented Sep 21, 2018

cmsbuild commented Sep 21, 2018 • edited

cmsbuild commented Sep 21, 2018

cmsbuild commented Sep 21, 2018

cmsbuild commented Sep 21, 2018

ianna commented Sep 25, 2018

cmsbuild commented Sep 25, 2018

fabiocos commented Sep 25, 2018

cmsbuild commented Sep 20, 2018 •

edited

cmsbuild commented Sep 21, 2018 •

edited