Fix SiStripHashedDetId constructor / operator and add more SiStripLorentzAngle PCL unit testings

CalibFormats/SiStripObjects/interface/SiStripHashedDetId.h

@@ -30,7 +30,21 @@ class SiStripHashedDetId {
   SiStripHashedDetId(const SiStripHashedDetId &);
 
   /** Assignment operator. */
-  SiStripHashedDetId &operator=(const SiStripHashedDetId &) = default;
+  SiStripHashedDetId &operator=(const SiStripHashedDetId &other) {
+    if (this != &other) {  // Self-assignment check
+      this->id_ = 0;
+      this->iter_ = other.begin();
+      // auxilliary vector to store the list of raw IDs
+      std::vector<uint32_t> raw_ids;
+      raw_ids.reserve(other.size());
+
+      // Copy elements from input vector to detIds_ vector
+      std::copy(other.begin(), other.end(), std::back_inserter(raw_ids));
+
+      this->init(raw_ids);
+    }
+    return *this;
+  }
 
   /** Public default constructor. */
   SiStripHashedDetId();
@@ -59,6 +73,8 @@ class SiStripHashedDetId {
 
   inline const_iterator end() const;
 
+  inline const size_t size() const { return detIds_.size(); }
+
 private:
   void init(const std::vector<uint32_t> &);
 

CalibFormats/SiStripObjects/src/SiStripHashedDetId.cc

@@ -36,8 +36,15 @@ SiStripHashedDetId::SiStripHashedDetId(const std::vector<DetId> &det_ids) : detI
 SiStripHashedDetId::SiStripHashedDetId(const SiStripHashedDetId &input) : detIds_(), id_(0), iter_(detIds_.begin()) {
   LogTrace(mlCabling_) << "[SiStripHashedDetId::" << __func__ << "]"
                        << " Constructing object...";
-  detIds_.reserve(input.end() - input.begin());
-  std::copy(input.begin(), input.end(), detIds_.begin());
+
+  // auxilliary vector to store the list of raw IDs
+  std::vector<uint32_t> raw_ids;
+  raw_ids.reserve(input.size());
+
+  // Copy elements from input vector to detIds_ vector
+  std::copy(input.begin(), input.end(), std::back_inserter(raw_ids));
+
+  init(raw_ids);
 }
 
 // -----------------------------------------------------------------------------

CalibFormats/SiStripObjects/test/BuildFile.xml

@@ -15,4 +15,11 @@
   <use name="cppunit"/>
 </bin>
 
+<bin file="test_catch2_*.cc" name="test_catch2_SiStripHashedDetId">
+  <use name="CalibFormats/SiStripObjects"/>
+  <use name="CalibTracker/SiStripCommon"/>
+  <use name="FWCore/ParameterSet"/>
+  <use name="catch2"/>
+</bin>
+
 <test name="testSiStripHashedDetId" command="testSiStripHashedDetId.sh"/>

CalibFormats/SiStripObjects/test/test_catch2_SiStripHashedDetId.cc

@@ -0,0 +1,247 @@
+#include "CalibFormats/SiStripObjects/interface/SiStripHashedDetId.h"
+#include "CalibFormats/SiStripObjects/interface/SiStripDetInfo.h"
+#include "CalibTracker/SiStripCommon/interface/SiStripDetInfoFileReader.h"
+#include "FWCore/ParameterSet/interface/FileInPath.h"
+#include "catch.hpp"
+
+#include <set>
+#include <vector>
+#include <algorithm>
+#include <iostream>
+
+TEST_CASE("SiStripHashedDetId testing", "[SiStripHashedDetId]") {
+  //_____________________________________________________________
+  SECTION("Check constructing SiStripHashedDetId from DetId list") {
+    const auto& detInfo =
+        SiStripDetInfoFileReader::read(edm::FileInPath(SiStripDetInfoFileReader::kDefaultFile).fullPath());
+    const auto& detIds = detInfo.getAllDetIds();
+    SiStripHashedDetId hash(detIds);
+    std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+              << " Successfully created hash!" << std::endl;
+    REQUIRE(true);
+  }
+
+  //_____________________________________________________________
+  SECTION("Check SiStripHashedDetId copy constructor") {
+    const auto& detInfo =
+        SiStripDetInfoFileReader::read(edm::FileInPath(SiStripDetInfoFileReader::kDefaultFile).fullPath());
+    const auto& dets = detInfo.getAllDetIds();
+
+    std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+              << " dets.size(): " << dets.size() << std::endl;
+
+    SiStripHashedDetId hash(dets);
+
+    std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+              << " hash.size(): " << hash.size() << std::endl;
+
+    // Retrieve hashed indices
+    std::vector<uint32_t> hashes;
+    hashes.clear();
+    hashes.reserve(dets.size());
+    for (const auto& idet : dets) {
+      hashes.push_back(hash.hashedIndex(idet));
+    }
+
+    std::sort(hashes.begin(), hashes.end());
+
+    SiStripHashedDetId hash2(hash);
+
+    std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+              << " Successfully copied hash map!" << std::endl;
+
+    // Retrieve hashed indices
+    std::vector<uint32_t> hashes2;
+
+    std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+              << " hashs2.size(): " << hash2.size() << std::endl;
+
+    hashes2.clear();
+    hashes2.reserve(dets.size());
+    for (const auto& idet : dets) {
+      hashes2.push_back(hash2.hashedIndex(idet));
+    }
+
+    std::sort(hashes2.begin(), hashes2.end());
+
+    std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+              << " Successfully sorted second hash map!" << std::endl;
+
+    // Convert vectors to sets for easy set operations
+    std::set<uint32_t> set1(hashes.begin(), hashes.end());
+    std::set<uint32_t> set2(hashes2.begin(), hashes2.end());
+
+    std::vector<uint32_t> diff1to2, diff2to1;
+
+    // Find elements in vec1 that are not in vec2
+    std::set_difference(set1.begin(), set1.end(), set2.begin(), set2.end(), std::inserter(diff1to2, diff1to2.begin()));
+
+    // Find elements in vec2 that are not in vec1
+    std::set_difference(set2.begin(), set2.end(), set1.begin(), set1.end(), std::inserter(diff2to1, diff2to1.begin()));
+
+    // Output the differences
+    if (!diff1to2.empty()) {
+      std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+                << " Elements in hash that are not in hash2: ";
+      for (const auto& elem : diff1to2) {
+        std::cout << elem << " ";
+      }
+      std::cout << std::endl;
+    }
+
+    if (!diff2to1.empty()) {
+      std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+                << " Elements in hash2 that are not in hash: ";
+      for (const auto& elem : diff2to1) {
+        std::cout << elem << " ";
+      }
+      std::cout << std::endl;
+    }
+
+    REQUIRE(hashes == hashes2);
+  }
+
+  //_____________________________________________________________
+  SECTION("Check SiStripHashedDetId assignment operator") {
+    const auto& detInfo =
+        SiStripDetInfoFileReader::read(edm::FileInPath(SiStripDetInfoFileReader::kDefaultFile).fullPath());
+    const auto& dets = detInfo.getAllDetIds();
+
+    SiStripHashedDetId hash(dets);
+    SiStripHashedDetId hash2;
+
+    // Retrieve hashed indices
+    std::vector<uint32_t> hashes;
+    hashes.clear();
+    hashes.reserve(dets.size());
+    for (const auto& idet : dets) {
+      hashes.push_back(hash.hashedIndex(idet));
+    }
+
+    std::sort(hashes.begin(), hashes.end());
+
+    // assign hash to hash2
+    hash2 = hash;
+
+    // Retrieve hashed indices
+    std::vector<uint32_t> hashes2;
+    hashes2.clear();
+    hashes2.reserve(dets.size());
+    for (const auto& idet : dets) {
+      hashes2.push_back(hash2.hashedIndex(idet));
+    }
+
+    std::sort(hashes2.begin(), hashes2.end());
+
+    if (hashes == hashes2) {
+      std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+                << " Assigned SiStripHashedDetId matches original one!" << std::endl;
+    } else {
+      std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+                << " Assigned SiStripHashedDetId does not match the original one!" << std::endl;
+    }
+
+    REQUIRE(hashes == hashes2);
+  }
+
+  //_____________________________________________________________
+  SECTION("Check manipulating SiStripHashedDetId") {
+    const auto& detInfo =
+        SiStripDetInfoFileReader::read(edm::FileInPath(SiStripDetInfoFileReader::kDefaultFile).fullPath());
+
+    const auto& unsortedDets = detInfo.getAllDetIds();
+
+    // unfortunately SiStripDetInfo::getAllDetIds() returns a const vector
+    // so in order to sory we're gonna need to copy it first
+
+    std::vector<uint32_t> dets;
+    dets.reserve(unsortedDets.size());
+    std::copy(unsortedDets.begin(), unsortedDets.end(), std::back_inserter(dets));
+
+    // sort the vector of detIds (otherwise the test won't work!)
+    std::sort(dets.begin(), dets.end());
+
+    SiStripHashedDetId hash(dets);
+
+    // Retrieve hashed indices
+    std::vector<uint32_t> hashes;
+    uint32_t istart = time(NULL);
+    hashes.clear();
+    hashes.reserve(dets.size());
+    for (const auto& idet : dets) {
+      hashes.push_back(hash.hashedIndex(idet));
+    }
+
+    // Some debug
+    std::stringstream ss;
+    ss << "[testSiStripHashedDetId::" << __func__ << "]";
+    uint16_t cntr1 = 0;
+    for (const auto& ii : hashes) {
+      if (ii == sistrip::invalid32_) {
+        cntr1++;
+        ss << std::endl << " Invalid index " << ii;
+        continue;
+      }
+      uint32_t detid = hash.unhashIndex(ii);
+      std::vector<uint32_t>::const_iterator iter = find(dets.begin(), dets.end(), detid);
+      if (iter == dets.end()) {
+        cntr1++;
+        ss << std::endl << " Did not find value " << detid << " at index " << ii - *(hashes.begin()) << " in vector!";
+      } else if (ii != static_cast<uint32_t>(iter - dets.begin())) {
+        cntr1++;
+        ss << std::endl
+           << " Found same value " << detid << " at different indices " << ii << " and " << iter - dets.begin();
+      }
+    }
+
+    if (cntr1) {
+      ss << std::endl << " Found " << cntr1 << " incompatible values!";
+    } else {
+      ss << " Found no incompatible values!";
+    }
+    std::cout << ss.str() << std::endl;
+
+    std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+              << " Processed " << hashes.size() << " DetIds in " << (time(NULL) - istart) << " seconds" << std::endl;
+
+    REQUIRE(cntr1 == 0);
+
+    // Retrieve DetIds
+    std::vector<uint32_t> detids;
+    uint32_t jstart = time(NULL);
+    // meaasurement!
+    detids.clear();
+    detids.reserve(dets.size());
+    for (uint16_t idet = 0; idet < dets.size(); ++idet) {
+      detids.push_back(hash.unhashIndex(idet));
+    }
+
+    // Some debug
+    std::stringstream sss;
+    sss << "[testSiStripHashedDetId::" << __func__ << "]";
+    uint16_t cntr2 = 0;
+    std::vector<uint32_t>::const_iterator iii = detids.begin();
+    for (; iii != detids.end(); ++iii) {
+      if (*iii != dets.at(iii - detids.begin())) {
+        cntr2++;
+        sss << std::endl
+            << " Diff values " << *iii << " and " << dets.at(iii - detids.begin()) << " found at index "
+            << iii - detids.begin() << " ";
+      }
+    }
+    if (cntr2) {
+      sss << std::endl << " Found " << cntr2 << " incompatible values!";
+    } else {
+      sss << " Found no incompatible values!";
+    }
+    std::cout << sss.str() << std::endl;
+
+    std::cout << "[testSiStripHashedDetId::" << __func__ << "]"
+              << " Processed " << detids.size() << " hashed indices in " << (time(NULL) - jstart) << " seconds"
+              << std::endl;
+
+    REQUIRE(cntr2 == 0);
+
+    REQUIRE(true);
+  }
+}

CalibFormats/SiStripObjects/test/test_catch2_main.cc

@@ -0,0 +1,2 @@
+#define CATCH_CONFIG_MAIN  // This tells Catch to provide a main() - only do this in one cpp file
+#include "catch.hpp"

CalibTracker/SiStripLorentzAngle/interface/SiStripLorentzAngleCalibrationStruct.h

@@ -7,6 +7,7 @@
 
 // user includes
 #include "DQMServices/Core/interface/DQMStore.h"
+#include "CalibFormats/SiStripObjects/interface/SiStripHashedDetId.h"
 
 struct SiStripLorentzAngleCalibrationHistograms {
 public:
@@ -28,9 +29,11 @@ struct SiStripLorentzAngleCalibrationHistograms {
   std::map<std::string, dqm::reco::MonitorElement*> p_;
 
   // These are vectors since std:map::find is expensive
-  // we're going to profi of the dense indexing offered by
+  // we're going to profit of the dense indexing offered by
   // SiStripHashedDetId and index the histogram position
   // with the natural booking order
+  SiStripHashedDetId hash_;
+
   std::vector<dqm::reco::MonitorElement*> h2_ct_w_m_;
   std::vector<dqm::reco::MonitorElement*> h2_ct_var2_m_;
   std::vector<dqm::reco::MonitorElement*> h2_ct_var3_m_;