Implement suggestions for CMSSW_14_1_X PR cms-sw#45665 (#137)

* Implement TH2Poly in DQM Services for HGCal DQM
* Apply changes from code quality checks
* Add a patch based on makortel's comments
* enum -> namespace + constexpr uint{8,16,32}_t
* Matti suggestion: remove '_t' postfix from HGCalFEDReadoutSequence_t, HGCalROCConfig_t, HGCalECONDConfig_t, HGCalFedConfig_t structs
* Matti suggestions & code checks: remove redundant comments; silence debugging cout or use MessageLogger; move includes; remove trailing '_' from function arguments
* Matti suggestion: reuse typecodeMap_.find() to avoid searching twice (TODO: throw exception if not found?)
* Matti suggestion: use SET_PORTABLEHOSTCOLLECTION_READ_RULES; add ClassVersion+checksum
* Matti suggestion: iRecord.getRecord<...>. -> iRecord.
* Matti suggestion: iRecord.getRecord<...>. -> iRecord.; rm redundant includes & couts; pass reference to fill_SoA_column to avoid copy
* Matti suggestions: use SET_PORTABLEHOSTCOLLECTION_READ_RULES for HGCalRecHitHost; Remove deprecated HGCalFlaggedECONDInfo* & comments;
* Matti suggestion: throw exception if type code not found; replace for-loop with std::accumulate
* Matti suggestion: remove TestObjects
* Matti suggestions: remove config as member data and produce unique_ptr instead of shared_ptr; NULL → nullptr; initialize optional override; memcp -> std::copy
* Matti suggestions: use file in path
* removing soafiller and updating buildfile (#139)
* address comments on HGCalRecHitCalibrationAlgorithms.dev.cc
* irot as uint8_t instead of char
* restore DQM without TH2Poly
* apply code formats

---------

Co-authored-by: ywkao <ywkao@hep1.phys.ntu.edu.tw>
Co-authored-by: Pedro <psilva@cern.ch>

CondFormats/HGCalObjects/interface/HGCalCalibrationParameterSoA.h

@@ -18,13 +18,11 @@ namespace hgcalrechit {
 
   // Generate structure of channel-level arrays (SoA) layout with RecHit dataformat
   GENERATE_SOA_LAYOUT(HGCalCalibParamSoALayout,
-                      //SOA_SCALAR(HGCalMappingModuleIndexer, map), // dense idx map: now redundant & NOT thread safe !?
                       SOA_COLUMN(float, ADC_ped),     // ADC pedestals, O(91)
                       SOA_COLUMN(float, Noise),       // noise, O(3)
                       SOA_COLUMN(float, CM_slope),    // common mode slope, O(0.25)
                       SOA_COLUMN(float, CM_ped),      // common mode pedestal (offset), O(92)
                       SOA_COLUMN(float, BXm1_slope),  // leakage correction from previous bunch, O(0.0)
-                      //SOA_COLUMN(float, BXm1_ped),  // redundant
                       SOA_COLUMN(float, ADCtofC),  // ADC conversion to charge (fC), depends on gain (80, 160, 320 fC)
                       SOA_COLUMN(float, TOTtofC),  // TOT conversion to charge (fC), depends on gain (80, 160, 320 fC)
                       SOA_COLUMN(float, TOT_ped),  // TOT pedestal (offset), O(9.0)
@@ -40,7 +38,6 @@ namespace hgcalrechit {
 
   // Generate structure of ROC-level arrays (SoA) layout with RecHit dataformat
   GENERATE_SOA_LAYOUT(HGCalConfigParamSoALayout,
-                      //SOA_SCALAR(HGCalMappingModuleIndexer, map), // dense idx map: now redundant & NOT thread safe !?
                       SOA_COLUMN(uint8_t, gain)  // for ADC to charge (fC) conversion (80, 160, 320 fC)
   )
   using HGCalConfigParamSoA = HGCalConfigParamSoALayout<>;

CondFormats/HGCalObjects/interface/HGCalConfiguration.h

@@ -8,7 +8,7 @@
 #include <vector>
 
 // @short configuration for ECON eRX (one half of HGROC)
-struct HGCalROCConfig_t {
+struct HGCalROCConfig {
   uint32_t charMode;  // characterization mode; determines data fields in ROC dataframe
   uint8_t gain;       // pre-amp gain used (1: 80 fC, 2: 160 fC, 4: 320 fC)
   //uint32_t clockPhase;     // fine adjustment of the phase within the 40 MHz
@@ -19,32 +19,32 @@ struct HGCalROCConfig_t {
 };
 
 // @short configuration for ECON-D module
-struct HGCalECONDConfig_t {
+struct HGCalECONDConfig {
   //std::string typecode;
   uint32_t headerMarker;  // begin of event marker/identifier for ECON-D
   uint32_t passThrough;   //pass through mode (this is just as check as it'll be in the ECON-D header anyway)
-  std::vector<HGCalROCConfig_t> rocs;
+  std::vector<HGCalROCConfig> rocs;
   COND_SERIALIZABLE;
 };
 
 // @short configuration for FED
-struct HGCalFedConfig_t {
+struct HGCalFedConfig {
   bool mismatchPassthroughMode;  // ignore ECON-D packet mismatches
   uint32_t cbHeaderMarker;       // begin of event marker/identifier for capture block
   uint32_t slinkHeaderMarker;    // begin of event marker/identifier for S-link
   //uint32_t delay; // delay
-  std::vector<HGCalECONDConfig_t> econds;
+  std::vector<HGCalECONDConfig> econds;
   COND_SERIALIZABLE;
 };
 
 /**
  *  @short Main HGCal configuration with a tree structure of vectors of
- *         HGCalFedConfig_t/HGCalECONDConfig_t/HGCalROCConfig_t structs as follows:
+ *         HGCalFedConfig/HGCalECONDConfig/HGCalROCConfig structs as follows:
  %         config.feds[dense_fed_idx].econds[dense_econd_idx].rocs[dense_eRx_idx]
  **/
 class HGCalConfiguration {
 public:
-  std::vector<HGCalFedConfig_t> feds;
+  std::vector<HGCalFedConfig> feds;
   //friend std::ostream& operator<< (std::ostream&, const HGCalConfiguration&);
 
 private:

CondFormats/HGCalObjects/interface/HGCalMappingModuleIndexer.h

@@ -4,8 +4,9 @@
 #include <cstdint>
 #include <vector>
 #include <map>
-#include <utility>  // for std::pair, std::make_pair
-#include <algorithm>
+#include <algorithm>  // for std::min
+#include <utility>    // for std::pair, std::make_pair
+#include <iterator>   // for std::next and std::advance
 
 #include "DataFormats/HGCalDigi/interface/HGCalElectronicsId.h"
 #include "CondFormats/Serialization/interface/Serializable.h"
@@ -19,7 +20,7 @@
    as the 12 capture blocks may not all be used and the each capture block may also be under-utilized
    a lookup table is used to hold the compact index
  */
-struct HGCalFEDReadoutSequence_t {
+struct HGCalFEDReadoutSequence {
   uint32_t id;
   ///>look-up table (capture block, econd idx) -> internal dense index
   std::vector<int> moduleLUT_;
@@ -57,7 +58,7 @@ class HGCalMappingModuleIndexer {
     if (fedid >= fedReadoutSequences_.size()) {
       fedReadoutSequences_.resize(fedid + 1);
     }
-    HGCalFEDReadoutSequence_t& frs = fedReadoutSequences_[fedid];
+    HGCalFEDReadoutSequence& frs = fedReadoutSequences_[fedid];
     frs.id = fedid;
 
     //assign position, resize if needed, and fill the type code
@@ -232,22 +233,22 @@ class HGCalMappingModuleIndexer {
     return getIndexForModuleData(fedid, modid, 0, 0);
   };
   std::pair<uint32_t, uint32_t> getIndexForFedAndModule(std::string const& typecode) const {
-    if (typecodeMap_.find(typecode) == typecodeMap_.end()) {  // did not find key
-      edm::LogWarning("HGCalMappingModuleIndexer")
-          << "Could not find typecode " << typecode << " in map (size=" << typecodeMap_.size()
-          << ")! Found following modules:";
-      int i = 1;
-      for (auto it = typecodeMap_.begin(); it != typecodeMap_.end(); it++) {
-        std::cout << it->first << (it != typecodeMap_.end() ? ", " : "") << std::endl;
-        if (i <= 100) {
-          std::cout << " ..." << std::endl;
-          break;
-        }  // stop sooner to avoid gigantic printout
-        i++;
-      }
-      //return {0,0};
+    auto it = typecodeMap_.find(typecode);
+    if (it == typecodeMap_.end()) {       // did not find key
+      std::size_t nmax = 100;             // maximum number of keys to print
+      auto maxit = typecodeMap_.begin();  // limit printout to prevent gigantic print out
+      std::advance(maxit, std::min(typecodeMap_.size(), nmax));
+      std::string allkeys = std::accumulate(
+          std::next(typecodeMap_.begin()), maxit, typecodeMap_.begin()->first, [](const std::string& a, const auto& b) {
+            return a + ',' + b.first;
+          });
+      if (typecodeMap_.size() > nmax)
+        allkeys += ", ...";
+      throw cms::Exception("HGCalMappingModuleIndexer")
+          << "Could not find typecode '" << typecode << "' in map (size=" << typecodeMap_.size()
+          << ")! Found the following modules (from the module locator file): " << allkeys;
     }
-    return typecodeMap_.at(typecode);  // (fedid,modid)
+    return it->second;  // (fedid,modid)
   };
 
   /**
@@ -290,7 +291,7 @@ class HGCalMappingModuleIndexer {
   ///< internal indexer
   HGCalDenseIndexerBase modFedIndexer_;
   ///< the sequence of FED readout sequence descriptors
-  std::vector<HGCalFEDReadoutSequence_t> fedReadoutSequences_;
+  std::vector<HGCalFEDReadoutSequence> fedReadoutSequences_;
   ///< global counters for types of modules, number of e-Rx and words
   std::vector<uint32_t> globalTypesCounter_, globalTypesNErx_, globalTypesNWords_;
   ///< base offsets to apply per module type with different granularity : module, e-Rx, channel data

CondFormats/HGCalObjects/interface/HGCalMappingParameterSoA.h

@@ -18,7 +18,7 @@ namespace hgcal {
                       SOA_COLUMN(int, plane),
                       SOA_COLUMN(int, i1),
                       SOA_COLUMN(int, i2),
-                      SOA_COLUMN(char, irot),
+                      SOA_COLUMN(uint8_t, irot),
                       SOA_COLUMN(int, celltype),
                       SOA_COLUMN(uint16_t, typeidx),
                       SOA_COLUMN(uint16_t, fedid),

CondFormats/HGCalObjects/src/classes_def.xml

@@ -8,7 +8,7 @@
     <field name="di_" mapping="blob"/>
   </class>
 
-  <class name="HGCalFEDReadoutSequence_t" class_version="0">
+  <class name="HGCalFEDReadoutSequence" class_version="0">
     <field name="readoutTypes_" mapping="blob"/>
     <field name="modOffsets_" mapping="blob"/>
     <field name="erxOffsets_" mapping="blob"/>
@@ -30,18 +30,18 @@
     <field name="feds" mapping="blob"/>
   </class>
 
-  <class name="HGCalFedConfig_t" class_version="0">
+  <class name="HGCalFedConfig" class_version="0">
     <field name="passthroughMode" mapping="blob"/>
     <field name="cbHeaderMarker" mapping="blob"/>
     <field name="slinkHeaderMarker" mapping="blob"/>
     <field name="delay" mapping="blob"/>
   </class>
 
-  <class name="HGCalECONDConfig_t" class_version="0">
+  <class name="HGCalECONDConfig" class_version="0">
     <field name="headerMarker" mapping="blob"/>
   </class>
 
-  <class name="HGCalROCConfig_t" class_version="0">
+  <class name="HGCalROCConfig" class_version="0">
     <field name="charMode" mapping="blob"/>
     <field name="clockPhase" mapping="blob"/>
     <field name="L1AcceptOffset" mapping="blob"/>

CondFormats/HGCalObjects/test/testSerializationHGCalCondDataFormats.cc

@@ -5,7 +5,7 @@ int main() {
   //dense indexers
   testSerialization<HGCalDenseIndexerBase>();
   testSerialization<HGCalMappingCellIndexer>();
-  testSerialization<HGCalFEDReadoutSequence_t>();
+  testSerialization<HGCalFEDReadoutSequence>();
   testSerialization<HGCalMappingModuleIndexer>();
 
   return 0;

DataFormats/HGCalDigi/interface/HGCROCChannelDataFrame.h

@@ -101,7 +101,7 @@ class HGCROCChannelDataFrame {
   uint32_t raw() const { return value_; }
   bool tc() const { return flag2(); }
   bool tp() const { return flag1(); }
-  uint16_t tctp() const { return (tc() << 1) | tp(); }
+  uint8_t tctp() const { return (tc() << 1) | tp(); }
   uint16_t adc(bool charMode = false) const { return charMode ? packet3() : (tc() ? 0 : packet2()); }
   uint16_t adcm1(bool charMode = false) const { return charMode ? 0 : packet3(); }
   uint16_t tot(bool charMode = false) const {

DataFormats/HGCalDigi/interface/HGCalDigiSoA.h

@@ -12,8 +12,6 @@ namespace hgcaldigi {
 
   // Generate structure of arrays (SoA) layout with Digi dataformat
   GENERATE_SOA_LAYOUT(HGCalDigiSoALayout,
-                      // columns: one value per element
-                      //SOA_COLUMN(uint32_t, electronicsId), // redundant since common dense indexing
                       SOA_COLUMN(uint8_t, tctp),
                       SOA_COLUMN(uint16_t, adcm1),
                       SOA_COLUMN(uint16_t, adc),

DataFormats/HGCalDigi/interface/HGCalECONDPacketInfoSoA.h

@@ -1,6 +1,8 @@
 #ifndef DataFormats_HGCalDigi_interface_HGCalECONDPacketInfoSoA_h
 #define DataFormats_HGCalDigi_interface_HGCalECONDPacketInfoSoA_h
 
+#include <cstdint>  // for uint8_t
+
 #include <Eigen/Core>
 #include <Eigen/Dense>
 
@@ -9,29 +11,26 @@
 #include "DataFormats/SoATemplate/interface/SoAView.h"
 
 namespace hgcaldigi {
+
   // enum for getting ECONDFlag
-  enum ECONDFlag {
-    BITT_POS = 0,
-    BITM_POS = 1,
-    EBO_POS = 2,
-    EBO_MASK = 0b11,
-    HT_POS = 4,
-    HT_MASK = 0b11,
-    BITE_POS = 6,
-    BITS_POS = 7
-  };
+  namespace ECONDFlag {
+    constexpr uint8_t BITT_POS = 0, BITM_POS = 1, EBO_POS = 2, EBO_MASK = 0b11, HT_POS = 4, HT_MASK = 0b11,
+                      BITE_POS = 6, BITS_POS = 7;
+  }  // namespace ECONDFlag
+
   // functions to parse ECONDFlag
-  bool truncatedFlag(uint8_t econdFlag) { return ((econdFlag >> hgcaldigi::ECONDFlag::BITT_POS) & 0b1); }
-  bool matchFlag(uint8_t econdFlag) { return ((econdFlag >> hgcaldigi::ECONDFlag::BITM_POS) & 0b1); }
-  uint8_t eboFlag(uint8_t econdFlag) {
+  inline bool truncatedFlag(uint8_t econdFlag) { return ((econdFlag >> hgcaldigi::ECONDFlag::BITT_POS) & 0b1); }
+  inline bool matchFlag(uint8_t econdFlag) { return ((econdFlag >> hgcaldigi::ECONDFlag::BITM_POS) & 0b1); }
+  inline uint8_t eboFlag(uint8_t econdFlag) {
     return ((econdFlag >> hgcaldigi::ECONDFlag::EBO_POS) & hgcaldigi::ECONDFlag::EBO_MASK);
   }
-  uint8_t htFlag(uint8_t econdFlag) {
+  inline uint8_t htFlag(uint8_t econdFlag) {
     return ((econdFlag >> hgcaldigi::ECONDFlag::HT_POS) & hgcaldigi::ECONDFlag::HT_MASK);
   }
-  bool expectedFlag(uint8_t econdFlag) { return ((econdFlag >> hgcaldigi::ECONDFlag::BITE_POS) & 0b1); }
-  bool StatFlag(uint8_t econdFlag) { return ((econdFlag >> hgcaldigi::ECONDFlag::BITS_POS) & 0b1); }
-  // Generate structure of arrays (SoA) layout with Digi dataformat
+  inline bool expectedFlag(uint8_t econdFlag) { return ((econdFlag >> hgcaldigi::ECONDFlag::BITE_POS) & 0b1); }
+  inline bool StatFlag(uint8_t econdFlag) { return ((econdFlag >> hgcaldigi::ECONDFlag::BITS_POS) & 0b1); }
+
+  // generate structure of arrays (SoA) layout with Digi dataformat
   GENERATE_SOA_LAYOUT(HGCalECONDPacketInfoSoALayout,
                       // Capture block information:
                       // 0b000: Normal packet