From 6d58c62fa428faa0a3ae56cf7efd463a020a5b8f Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Wed, 25 May 2016 15:56:07 +0200 Subject: [PATCH 1/8] Refer to TrackerTopology by pointer instead of by handle. The handle seems to silenty get invalid at some point. A copy would be even safer. --- .../src/GeometryInterface.cc | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/DQM/SiPixelPhase1Common/src/GeometryInterface.cc b/DQM/SiPixelPhase1Common/src/GeometryInterface.cc index 5cac1164c04e6..0dc411d27892c 100644 --- a/DQM/SiPixelPhase1Common/src/GeometryInterface.cc +++ b/DQM/SiPixelPhase1Common/src/GeometryInterface.cc @@ -175,7 +175,7 @@ void GeometryInterface::loadFromTopology(edm::EventSetup const& iSetup, const ed std::vector geomquantities; struct TTField { - edm::ESHandle tt; + const TrackerTopology* tt; TrackerTopology::DetIdFields field; Value operator()(InterestingQuantities const& iq) { if (tt->hasField(iq.sourceModule, field)) @@ -185,17 +185,20 @@ void GeometryInterface::loadFromTopology(edm::EventSetup const& iSetup, const ed }; }; - std::vector> namedPartitions { - {"PXEndcap" , {trackerTopologyHandle, TrackerTopology::PFSide}}, + const TrackerTopology* tt = trackerTopologyHandle.operator->(); - {"PXLayer" , {trackerTopologyHandle, TrackerTopology::PBLayer}}, - {"PXLadder" , {trackerTopologyHandle, TrackerTopology::PBLadder}}, - {"PXBModule", {trackerTopologyHandle, TrackerTopology::PBModule}}, - {"PXBlade" , {trackerTopologyHandle, TrackerTopology::PFBlade}}, - {"PXDisk" , {trackerTopologyHandle, TrackerTopology::PFDisk}}, - {"PXPanel" , {trackerTopologyHandle, TrackerTopology::PFPanel}}, - {"PXFModule", {trackerTopologyHandle, TrackerTopology::PFModule}}, + std::vector> namedPartitions { + {"PXEndcap" , {tt, TrackerTopology::PFSide}}, + + {"PXLayer" , {tt, TrackerTopology::PBLayer}}, + {"PXLadder" , {tt, TrackerTopology::PBLadder}}, + {"PXBModule", {tt, TrackerTopology::PBModule}}, + + {"PXBlade" , {tt, TrackerTopology::PFBlade}}, + {"PXDisk" , {tt, TrackerTopology::PFDisk}}, + {"PXPanel" , {tt, TrackerTopology::PFPanel}}, + {"PXFModule", {tt, TrackerTopology::PFModule}}, }; for (auto& e : namedPartitions) { From b4234ae3852807b356e05d51085c9f5264ec878e Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Wed, 25 May 2016 15:57:51 +0200 Subject: [PATCH 2/8] Always initialize the GeometryInterface, in case it is used outside of the HistogramManager. --- DQM/SiPixelPhase1Common/src/HistogramManager.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DQM/SiPixelPhase1Common/src/HistogramManager.cc b/DQM/SiPixelPhase1Common/src/HistogramManager.cc index 2393bf46d4c56..6311b7ba05967 100644 --- a/DQM/SiPixelPhase1Common/src/HistogramManager.cc +++ b/DQM/SiPixelPhase1Common/src/HistogramManager.cc @@ -224,10 +224,10 @@ std::string HistogramManager::makePath( void HistogramManager::book(DQMStore::IBooker& iBooker, edm::EventSetup const& iSetup) { - if (!enabled) return; if (!geometryInterface.loaded()) { geometryInterface.load(iSetup); } + if (!enabled) return; for (unsigned int i = 0; i < specs.size(); i++) { auto& s = specs[i]; From d1173149fe83cc8ecf238bcebc2fd08f6d718fc4 Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Wed, 25 May 2016 15:59:47 +0200 Subject: [PATCH 3/8] remove enable flag in Digis config to allow global disable. --- DQM/SiPixelPhase1Digis/python/SiPixelPhase1Digis_cfi.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/DQM/SiPixelPhase1Digis/python/SiPixelPhase1Digis_cfi.py b/DQM/SiPixelPhase1Digis/python/SiPixelPhase1Digis_cfi.py index 5cab271179716..a2962062cc5f0 100644 --- a/DQM/SiPixelPhase1Digis/python/SiPixelPhase1Digis_cfi.py +++ b/DQM/SiPixelPhase1Digis/python/SiPixelPhase1Digis_cfi.py @@ -4,7 +4,6 @@ from DQM.SiPixelPhase1Common.HistogramManager_cfi import * SiPixelPhase1DigisADC = DefaultHisto.clone( - enabled = True, # ADC name = "adc", title = "Digi ADC values", xlabel = "adc readout", @@ -24,7 +23,6 @@ ) SiPixelPhase1DigisNdigis = DefaultHisto.clone( - enabled = True, # Ndigis name = "digis", # 'Count of' added automatically title = "Digis", xlabel = "digis", @@ -51,7 +49,6 @@ ) SiPixelPhase1DigisNdigisPerFED = DefaultHisto.clone( - enabled = True, # Ndigis per FED name = "digis", # This is the same as above up to the ranges. maybe we title = "Digis", # should allow setting the range per spec, but OTOH a xlabel = "digis",# HistogramManager is almost free. @@ -73,7 +70,6 @@ ) SiPixelPhase1DigisEvents = DefaultHisto.clone( - enabled = True, # Event Rate name = "eventrate", title = "Rate of Pixel Events", xlabel = "Lumisection", @@ -88,7 +84,6 @@ ) SiPixelPhase1DigisHitmap = DefaultHisto.clone( - enabled = True, # hitmaps name = "hitmap", title = "Position of digis on module", ylabel = "#digis", @@ -128,7 +123,6 @@ ) SiPixelPhase1DigisDebug = DefaultHisto.clone( - enabled = True, # Geometry Debug name = "debug", xlabel = "ladder #", range_min = 1, @@ -141,7 +135,8 @@ .groupBy(parent(DefaultHisto.defaultGrouping), "EXTEND_X") .saveAll(), Specification().groupBy(parent(DefaultHisto.defaultGrouping)) # per-layer - .save() + .save(), + Specification(PerModule).groupBy(DefaultHisto.defaultPerModule).save(), ) ) From 5269edb6f61c5e87364dfc5f8eeac07978e51815 Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Fri, 27 May 2016 10:53:00 +0200 Subject: [PATCH 4/8] Preserve title on profiles --- DQM/SiPixelPhase1Common/src/HistogramManager.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DQM/SiPixelPhase1Common/src/HistogramManager.cc b/DQM/SiPixelPhase1Common/src/HistogramManager.cc index 6311b7ba05967..eada5fa174b69 100644 --- a/DQM/SiPixelPhase1Common/src/HistogramManager.cc +++ b/DQM/SiPixelPhase1Common/src/HistogramManager.cc @@ -494,7 +494,8 @@ void HistogramManager::executeReduce(SummationStep& step, Table& t) { edm::LogError("HistogramManager") << "+++ Reduction '" << step.arg << " not yet implemented\n"; } - new_histo.th1 = new TH1D(name.c_str(), (";;" + label).c_str(), 1, 0, 1); + new_histo.th1 = new TH1F(name.c_str(), (std::string("") + th1->GetTitle() + + ";;" + label).c_str(), 1, 0, 1); new_histo.th1->SetBinContent(1, reduced_quantity); } t.swap(out); From 54510d9d3b2624ebf3089cdec5e58fc6399a950e Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Fri, 3 Jun 2016 10:44:50 +0200 Subject: [PATCH 5/8] Fix two obvious bugs in the RecHit config --- DQM/SiPixelPhase1RecHits/python/SiPixelPhase1RecHits_cfi.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/DQM/SiPixelPhase1RecHits/python/SiPixelPhase1RecHits_cfi.py b/DQM/SiPixelPhase1RecHits/python/SiPixelPhase1RecHits_cfi.py index 691950eeab819..2d9630eb71691 100644 --- a/DQM/SiPixelPhase1RecHits/python/SiPixelPhase1RecHits_cfi.py +++ b/DQM/SiPixelPhase1RecHits/python/SiPixelPhase1RecHits_cfi.py @@ -66,8 +66,8 @@ ) ) -SiPixelPhase1RecHitsErrorY = DefaultHisto.clone( - name = "rechiterror_x", +SiPixelPhase1RecHitsErrorY = SiPixelPhase1RecHitsErrorX.clone( + name = "rechiterror_y", title = "RecHit Error in Y-direction", xlabel = "Y error" ) @@ -79,6 +79,7 @@ range_y_min = -4, range_y_max = 4, range_y_nbins = 100, xlabel = "x offset", ylabel = "y offset", + dimensions = 2, specs = cms.VPSet( Specification().groupBy(DefaultHisto.defaultGrouping).save(), Specification(PerModule).groupBy(DefaultHisto.defaultPerModule).save(), From 6e59e60db49ff13fba3aa47c99b68e283561eacc Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Fri, 3 Jun 2016 10:50:39 +0200 Subject: [PATCH 6/8] Override pythonTypeName, to fix dumpPython. --- DQM/SiPixelPhase1Common/python/SpecificationBuilder_cfi.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/DQM/SiPixelPhase1Common/python/SpecificationBuilder_cfi.py b/DQM/SiPixelPhase1Common/python/SpecificationBuilder_cfi.py index 5d0e422c44dca..d96828ac8a6ac 100644 --- a/DQM/SiPixelPhase1Common/python/SpecificationBuilder_cfi.py +++ b/DQM/SiPixelPhase1Common/python/SpecificationBuilder_cfi.py @@ -146,4 +146,7 @@ def saveAll(self): self.groupBy("/".join(cols), self._lastMode) self.save() return self - + + # this is used for serialization, and for that this is just a PSet. + def pythonTypeName(self): + return 'cms.PSet'; From e5dd360f507b86567a73ed8755c9a9582d7fa35b Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Thu, 2 Jun 2016 10:35:05 +0200 Subject: [PATCH 7/8] have the README refer to the twiki --- DQM/SiPixelPhase1Common/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/DQM/SiPixelPhase1Common/README.md b/DQM/SiPixelPhase1Common/README.md index 350af7d876f7e..4edfae87b2d2b 100644 --- a/DQM/SiPixelPhase1Common/README.md +++ b/DQM/SiPixelPhase1Common/README.md @@ -9,6 +9,8 @@ This document decomposes into three sections: 2. *How to actually use it* -- practical examples with explanation. 3. *How it really works* -- explanations on the implementation. +Further information can be found on the TWiki page: https://twiki.cern.ch/twiki/bin/viewauth/CMS/PixelDQMPhaseI . Note that parts migh be historical and not reflect the current state. + The Concept ----------- From 5b03e23a33ee37d8c44b8e2b0f710eb78ba7d2f7 Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Wed, 8 Jun 2016 14:54:10 +0200 Subject: [PATCH 8/8] Do not hold refs into the extractors vector. The refs got invalid when the vector grows, which caused #14800. --- DQM/SiPixelPhase1Common/src/GeometryInterface.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/DQM/SiPixelPhase1Common/src/GeometryInterface.cc b/DQM/SiPixelPhase1Common/src/GeometryInterface.cc index 0dc411d27892c..bf25d7db9fd14 100644 --- a/DQM/SiPixelPhase1Common/src/GeometryInterface.cc +++ b/DQM/SiPixelPhase1Common/src/GeometryInterface.cc @@ -212,7 +212,7 @@ void GeometryInterface::loadFromTopology(edm::EventSetup const& iSetup, const ed addExtractor(intern("PXForward"), pxforward, 0, 0); // Redefine the disk numbering to use the sign - auto& pxendcap = extractors[intern("PXEndcap")]; + auto pxendcap = extractors[intern("PXEndcap")]; auto diskid = intern("PXDisk"); auto pxdisk = extractors[diskid]; extractors[diskid] = [pxdisk, pxendcap] (InterestingQuantities const& iq) { @@ -228,8 +228,8 @@ void GeometryInterface::loadFromTopology(edm::EventSetup const& iSetup, const ed assert(trackerGeometryHandle.isValid()); // We need to track some extra stuff here for the Shells later. - auto& pxlayer = extractors[intern("PXLayer")]; - auto& pxladder = extractors[intern("PXLadder")]; + auto pxlayer = extractors[intern("PXLayer")]; + auto pxladder = extractors[intern("PXLadder")]; std::vector maxladders; // Now travrse the detector and collect whatever we need. @@ -260,7 +260,7 @@ void GeometryInterface::loadFromTopology(edm::EventSetup const& iSetup, const ed // of the code, but it might work for Phase0 as well. Value innerring = iConfig.getParameter("n_inner_ring_blades"); Value outerring = max_value[intern("PXBlade")] - innerring; - auto& pxblade = extractors[intern("PXBlade")]; + auto pxblade = extractors[intern("PXBlade")]; addExtractor(intern("PXRing"), [pxblade, innerring] (InterestingQuantities const& iq) { auto blade = pxblade(iq); @@ -270,7 +270,7 @@ void GeometryInterface::loadFromTopology(edm::EventSetup const& iSetup, const ed }, 1, 2 ); - auto& pxmodule = extractors[intern("PXBModule")]; + auto pxmodule = extractors[intern("PXBModule")]; Value maxmodule = max_value[intern("PXBModule")]; addExtractor(intern("HalfCylinder"), [pxendcap, pxblade, innerring, outerring] (InterestingQuantities const& iq) {