Iss1172 - Memory bugs and typos in DQM code

DQM/include/DQM/HCalDQM.h

@@ -1,6 +1,5 @@
 #ifndef _DQM_HCAL_DQM_H_
 #define _DQM_HCAL_DQM_H_
-
 //----------//
 //   STL    //
 //----------//
@@ -14,6 +13,8 @@
 /*~~~~~~~~~~~~~~~*/
 /*   Framework   */
 /*~~~~~~~~~~~~~~~*/
+#include <map>
+
 #include "DetDescr/HcalGeometry.h"
 #include "DetDescr/HcalID.h"
 #include "Framework/Configure/Parameters.h"
@@ -24,11 +25,10 @@
 #include "Hcal/Event/HcalVetoResult.h"
 #include "SimCore/Event/SimCalorimeterHit.h"
 #include "Tools/AnalysisUtils.h"
-#include <map>
 namespace dqm {
 
 class HCalDQM : public framework::Analyzer {
-public:
+ public:
   /** Constructor */
   HCalDQM(const std::string &name, framework::Process &process);
 
@@ -40,7 +40,7 @@ class HCalDQM : public framework::Analyzer {
    *
    * @param parameters Set of parameters used to configure this processor.
    */
-  void configure(framework::config::Parameters &parameters) final override;
+  void configure(framework::config::Parameters &parameters) override;
 
   /**
    * Process the event and make histograms ro summaries.
@@ -66,7 +66,7 @@ class HCalDQM : public framework::Analyzer {
     return false;
   }
 
-private:
+ private:
   /// Hcal Sim Hits collection name
   std::string sim_coll_name_;
 
@@ -90,6 +90,6 @@ class HCalDQM : public framework::Analyzer {
   double max_hit_time_;
 };
 
-} // namespace dqm
+}  // namespace dqm
 
-#endif // _DQM_HCAL_DQM_H_
+#endif  // _DQM_HCAL_DQM_H_

DQM/include/DQM/HcalGeometryVerifier.h

@@ -14,7 +14,6 @@
 #include "SimCore/Event/SimCalorimeterHit.h"
 
 namespace dqm {
-
 class HcalGeometryVerifier : public framework::Analyzer {
  public:
   HcalGeometryVerifier(const std::string &name, framework::Process &process)

DQM/include/DQM/PhotoNuclearDQM.h

@@ -6,17 +6,14 @@
 /*~~~~~~~~~~~~~~~*/
 #include "Framework/Configure/Parameters.h"
 #include "Framework/EventProcessor.h"
-
 #include "SimCore/Event/SimParticle.h"
-
 namespace dqm {
 
 // Forward declarations within the ldmx workspace
 class Event;
 class SimParticle;
 
 class PhotoNuclearDQM : public framework::Analyzer {
-
   // Classification schemes for PN events
   enum class CompactEventType {
     single_neutron = 0,
@@ -50,43 +47,41 @@ class PhotoNuclearDQM : public framework::Analyzer {
     multibody = 20,
   };
 
-public:
+ public:
   /// Constructor
   PhotoNuclearDQM(const std::string &name, framework::Process &process);
 
   /// Destructor
-  ~PhotoNuclearDQM();
+  virtual ~PhotoNuclearDQM();
 
   /**
    * Configure this analyzer using the user specified parameters.
    *
    * @param parameters Set of parameters used to configure this
    *                   analyzer.
    */
-  void configure(framework::config::Parameters &parameters) final override;
+  void configure(framework::config::Parameters &parameters) override;
 
   /**
    * Process the event and create the histogram summaries.
    *
    * @param event The event to analyze.
    */
-  void analyze(const framework::Event &event) final override;
+  void analyze(const framework::Event &event) override;
 
   /// Method executed before processing of events begins.
-  void onProcessStart();
+  void onProcessStart() override;
 
-private:
+ private:
   /** Method used to classify events. Note: Assumes that daughters is sorted by
    * kinetic energy. */
-  EventType
-  classifyEvent(const std::vector<const ldmx::SimParticle *> daughters,
-                double threshold);
+  EventType classifyEvent(
+      const std::vector<const ldmx::SimParticle *> daughters, double threshold);
 
   /** Method used to classify events in a compact manner. */
-  CompactEventType
-  classifyCompactEvent(const ldmx::SimParticle *pnGamma,
-                       const std::vector<const ldmx::SimParticle *> daughters,
-                       double threshold);
+  CompactEventType classifyCompactEvent(
+      const ldmx::SimParticle *pnGamma,
+      const std::vector<const ldmx::SimParticle *> daughters, double threshold);
 
   /**
    * Fill the recoil electron-histograms
@@ -105,9 +100,9 @@ class PhotoNuclearDQM : public framework::Analyzer {
    * The products are sorted by kinetic energy, in descending order.
    *
    **/
-  std::vector<const ldmx::SimParticle *>
-  findDaughters(const std::map<int, ldmx::SimParticle> particleMap,
-                const ldmx::SimParticle *parent) const;
+  std::vector<const ldmx::SimParticle *> findDaughters(
+      const std::map<int, ldmx::SimParticle> &particleMap,
+      const ldmx::SimParticle *parent) const;
 
   /**
    *
@@ -125,10 +120,10 @@ class PhotoNuclearDQM : public framework::Analyzer {
    **/
   void findSubleadingKinematics(
       const ldmx::SimParticle *pnGamma,
-      const std::vector<const ldmx::SimParticle *> &pnDaughters, //
+      const std::vector<const ldmx::SimParticle *> &pnDaughters,  //
       const EventType eventType);
 
-public:
+ public:
   /**
    *  Check if the PDG code corresponds to a light ion.
    *
@@ -140,9 +135,7 @@ class PhotoNuclearDQM : public framework::Analyzer {
    *
    */
   constexpr bool isLightIon(const int pdgCode) const {
-    //
-    // TODO: Is the < check necessary?
-    if (pdgCode > 1000000000 && pdgCode < 10000000000) {
+    if (pdgCode > 1000000000) {
       // Check if the atomic number is less than or equal to 4
       return ((pdgCode / 10) % 1000) <= 4;
     }
@@ -153,6 +146,6 @@ class PhotoNuclearDQM : public framework::Analyzer {
   bool count_light_ions_;
 };
 
-} // namespace dqm
+}  // namespace dqm
 
-#endif // _DQM_ECAL_PN_H_
+#endif  // _DQM_ECAL_PN_H_

DQM/python/dqm.py

@@ -198,7 +198,7 @@ def __init__(self,name="HcalInefficiencyAnalyzer", num_sections=5,
         for section in range(num_sections):
             section_name = section_names[section]
             self.build1DHistogram(f"inefficiency_{section_name}",
-                                  "fInefficiency ({section_name})",
+                                  f"Inefficiency ({section_name})",
                                   *inefficiency_layer_bins
                                   )
 

DQM/src/DQM/HCalDQM.cxx

@@ -24,19 +24,15 @@ void HCalDQM::analyze(const framework::Event &event) {
       sim_coll_name_, sim_pass_name_)};
   analyzeSimHits(hcalSimHits);
   analyzeRecHits(hcalHits);
-  const auto &geometry = getCondition<ldmx::HcalGeometry>(
-      ldmx::HcalGeometry::CONDITIONS_OBJECT_NAME);
 }
 void HCalDQM::analyzeSimHits(const std::vector<ldmx::SimCalorimeterHit> &hits) {
-
   const auto &geometry = getCondition<ldmx::HcalGeometry>(
       ldmx::HcalGeometry::CONDITIONS_OBJECT_NAME);
 
   std::map<ldmx::HcalID, double> simEnergyPerBar;
   int hitMultiplicity{0};
 
   for (const auto &hit : hits) {
-
     ldmx::HcalID id(hit.getID());
     if (skipHit(id)) {
       continue;
@@ -48,7 +44,6 @@ void HCalDQM::analyzeSimHits(const std::vector<ldmx::SimCalorimeterHit> &hits) {
       simEnergyPerBar[id] += energy;
     }
     const auto orientation{geometry.getScintillatorOrientation(id)};
-    const auto section{id.section()};
     const auto layer{id.layer()};
     const auto strip{id.strip()};
     const auto pos{hit.getPosition()};
@@ -62,15 +57,15 @@ void HCalDQM::analyzeSimHits(const std::vector<ldmx::SimCalorimeterHit> &hits) {
     histograms_.fill("sim_layer:strip", layer, strip);
     histograms_.fill("sim_energy", energy);
     switch (orientation) {
-    case ldmx::HcalGeometry::ScintillatorOrientation::horizontal:
-      histograms_.fill("sim_along_x", x);
-      break;
-    case ldmx::HcalGeometry::ScintillatorOrientation::vertical:
-      histograms_.fill("sim_along_y", y);
-      break;
-    case ldmx::HcalGeometry::ScintillatorOrientation::depth:
-      histograms_.fill("sim_along_z", z);
-      break;
+      case ldmx::HcalGeometry::ScintillatorOrientation::horizontal:
+        histograms_.fill("sim_along_x", x);
+        break;
+      case ldmx::HcalGeometry::ScintillatorOrientation::vertical:
+        histograms_.fill("sim_along_y", y);
+        break;
+      case ldmx::HcalGeometry::ScintillatorOrientation::depth:
+        histograms_.fill("sim_along_z", z);
+        break;
     }
   }
 
@@ -85,14 +80,12 @@ void HCalDQM::analyzeSimHits(const std::vector<ldmx::SimCalorimeterHit> &hits) {
   histograms_.fill("sim_total_energy", total_energy);
 }
 void HCalDQM::analyzeRecHits(const std::vector<ldmx::HcalHit> &hits) {
-
   const auto &geometry = getCondition<ldmx::HcalGeometry>(
       ldmx::HcalGeometry::CONDITIONS_OBJECT_NAME);
 
   float totalPE{0};
   float maxPE{-1};
   float maxPETime{-1};
-  float E{0};
   float totalE{0};
   int vetoableHitMultiplicity{0};
   int hitMultiplicity{0};
@@ -125,17 +118,16 @@ void HCalDQM::analyzeRecHits(const std::vector<ldmx::HcalHit> &hits) {
     const auto y{hit.getYPos()};
     const auto z{hit.getZPos()};
     switch (orientation) {
-    case ldmx::HcalGeometry::ScintillatorOrientation::horizontal:
-      histograms_.fill("along_x", x);
-      break;
-    case ldmx::HcalGeometry::ScintillatorOrientation::vertical:
-      histograms_.fill("along_y", y);
-      break;
-    case ldmx::HcalGeometry::ScintillatorOrientation::depth:
-      histograms_.fill("along_z", z);
-      break;
+      case ldmx::HcalGeometry::ScintillatorOrientation::horizontal:
+        histograms_.fill("along_x", x);
+        break;
+      case ldmx::HcalGeometry::ScintillatorOrientation::vertical:
+        histograms_.fill("along_y", y);
+        break;
+      case ldmx::HcalGeometry::ScintillatorOrientation::depth:
+        histograms_.fill("along_z", z);
+        break;
     }
-
     totalE += e;
     totalPE += pe;
 
@@ -159,6 +151,6 @@ void HCalDQM::analyzeRecHits(const std::vector<ldmx::HcalHit> &hits) {
   histograms_.fill("vetoable_hit_multiplicity", vetoableHitMultiplicity);
 }
 
-} // namespace dqm
+}  // namespace dqm
 
 DECLARE_ANALYZER_NS(dqm, HCalDQM)

DQM/src/DQM/HcalGeometryVerfifier.cxx

@@ -19,7 +19,7 @@ void HcalGeometryVerifier::analyze(const framework::Event &event) {
       hcalRecHitsCollection_, hcalRecHitsPassName_);
 
   for (const auto &hit : hcalSimHits) {
-    const ldmx::HcalID id{hit.getID()};
+    const ldmx::HcalID id{static_cast<unsigned int>(hit.getID())};
     const auto position{hit.getPosition()};
     auto ok{hit_ok(id, {position[0], position[1], position[2]})};
     histograms_.fill("passes_sim", ok);
@@ -42,7 +42,7 @@ void HcalGeometryVerifier::analyze(const framework::Event &event) {
     }
   }
   for (const auto &hit : hcalRecHits) {
-    const ldmx::HcalID id{hit.getID()};
+    const ldmx::HcalID id{static_cast<unsigned int>(hit.getID())};
     auto ok{hit_ok(id, {hit.getXPos(), hit.getYPos(), hit.getZPos()})};
     histograms_.fill("passes_rec", ok);
     switch (id.section()) {

DQM/src/DQM/HcalInefficiencyDQM.cxx

@@ -29,10 +29,14 @@ void HcalInefficiencyAnalyzer::analyze(const framework::Event &event) {
 
   bool vetoedByBack{firstLayersHit[ldmx::HcalID::HcalSection::BACK] !=
                     failedVeto};
-  bool vetoedByTop{firstLayersHit[ldmx::HcalID::HcalSection::TOP]};
-  bool vetoedByBottom{firstLayersHit[ldmx::HcalID::HcalSection::BOTTOM]};
-  bool vetoedByRight{firstLayersHit[ldmx::HcalID::HcalSection::RIGHT]};
-  bool vetoedByLeft{firstLayersHit[ldmx::HcalID::HcalSection::LEFT]};
+  bool vetoedByTop{firstLayersHit[ldmx::HcalID::HcalSection::TOP] !=
+                   failedVeto};
+  bool vetoedByBottom{firstLayersHit[ldmx::HcalID::HcalSection::BOTTOM] !=
+                      failedVeto};
+  bool vetoedByRight{firstLayersHit[ldmx::HcalID::HcalSection::RIGHT] !=
+                     failedVeto};
+  bool vetoedByLeft{firstLayersHit[ldmx::HcalID::HcalSection::LEFT] !=
+                    failedVeto};
   bool vetoedBySide{vetoedByTop || vetoedByBottom || vetoedByRight ||
                     vetoedByLeft};
 
@@ -61,7 +65,6 @@ void HcalInefficiencyAnalyzer::analyze(const framework::Event &event) {
 void HcalInefficiencyAnalyzer::configure(
 
     framework::config::Parameters &parameters) {
-
   hcalSimHitsCollection_ =
       parameters.getParameter<std::string>("sim_coll_name");
   hcalRecHitsCollection_ =
@@ -71,6 +74,6 @@ void HcalInefficiencyAnalyzer::configure(
   pe_veto_threshold = parameters.getParameter<double>("pe_veto_threshold");
   max_hit_time_ = parameters.getParameter<double>("max_hit_time");
 }
-} // namespace dqm
+}  // namespace dqm
 
 DECLARE_ANALYZER_NS(dqm, HcalInefficiencyAnalyzer);