Johnfreeman/trgdataformats issue59 review impl

CMakeLists.txt

@@ -8,20 +8,13 @@ daq_setup_environment()
 
 find_package(Boost COMPONENTS unit_test_framework REQUIRED)
 
-##############################################################################
-
 daq_add_library( LINK_LIBRARIES ) 
 
-##############################################################################
-
 daq_add_python_bindings(*.cpp )
 
-##############################################################################
-
 daq_add_unit_test(Trgdataformats_test LINK_LIBRARIES)
 daq_add_unit_test(TriggerCandidateData_test LINK_LIBRARIES)
-
-##############################################################################
+daq_add_unit_test(TriggerDataStructures_test LINK_LIBRARIES)
 
 daq_install()
 

include/trgdataformats/TriggerActivityData.hpp

@@ -1,6 +1,12 @@
 /**
  * @file TriggerActivityData.hpp
  *
+ * This header defines the TriggerActivityData struct, which
+ * aggregates information about a found set of associate trigger
+ * primitives (algorithm used, channels and times involved, etc.). It
+ * does *not* include per-trigger-primitive information, which need to
+ * be associated with TriggerActivityData in a higher level object.
+ *
  * This is part of the DUNE DAQ Application Framework, copyright 2020.
  * Licensing/copyright details are in the COPYING file that you should have
  * received with this code.
@@ -16,14 +22,14 @@ namespace dunedaq::trgdataformats {
 
 struct TriggerActivityData
 {
-  enum class Type
+  enum class Type : int
   {
     kUnknown = 0,
     kTPC = 1,
     kPDS = 2,
   };
 
-  enum class Algorithm
+  enum class Algorithm : int
   {
     kUnknown = 0,
     kSupernova = 1,
@@ -39,24 +45,32 @@ struct TriggerActivityData
     kSWIFT = 11
   };
 
-  // Update this version number if there are any changes to the in-memory representation of this class!
-  static constexpr version_t s_trigger_activity_version = 2; // NOLINT(build/unsigned)
-  
-  version_t version = s_trigger_activity_version; // NOLINT(build/unsigned)
-  timestamp_t time_start = INVALID_TIMESTAMP;
-  timestamp_t time_end = INVALID_TIMESTAMP;
-  timestamp_t time_peak = INVALID_TIMESTAMP;
-  timestamp_t time_activity = INVALID_TIMESTAMP;
-  channel_t channel_start = INVALID_CHANNEL; // NOLINT(build/unsigned)
-  channel_t channel_end = INVALID_CHANNEL;   // NOLINT(build/unsigned)
-  channel_t channel_peak = INVALID_CHANNEL;  // NOLINT(build/unsigned)
+  static constexpr version_t s_trigger_activity_version = 2;
+  static constexpr channel_t s_invalid_channel = std::numeric_limits<channel_t>::max();
+
+  version_t version = s_trigger_activity_version;
+  timestamp_t time_start = TypeDefaults::s_invalid_timestamp;
+  timestamp_t time_end = TypeDefaults::s_invalid_timestamp;
+  timestamp_t time_peak = TypeDefaults::s_invalid_timestamp;
+  timestamp_t time_activity = TypeDefaults::s_invalid_timestamp;
+  channel_t channel_start = s_invalid_channel;
+  channel_t channel_end = s_invalid_channel;
+  channel_t channel_peak = s_invalid_channel;
   uint64_t adc_integral = 0;                 // NOLINT(build/unsigned)
   uint16_t adc_peak = 0;                     // NOLINT(build/unsigned)
-  detid_t detid = INVALID_DETID;             // NOLINT(build/unsigned)
-  Type type = Type::kUnknown;                // NOLINT(build/unsigned)
-  Algorithm algorithm = Algorithm::kUnknown; // NOLINT(build/unsigned)
+  detid_t detid = TypeDefaults::s_invalid_detid;
+  Type type = Type::kUnknown;
+  Algorithm algorithm = Algorithm::kUnknown;
 };
-
+  
 } // namespace dunedaq::trgdataformats
 
+// This static_assert is meant to alert the developer to bump the
+// version if variables are added or removed
+static_assert(
+	      dunedaq::trgdataformats::TriggerActivityData::s_trigger_activity_version == 2 &&
+	      sizeof(dunedaq::trgdataformats::TriggerActivityData) == 80
+	      );
+
+
 #endif // TRGDATAFORMATS_INCLUDE_TRGDATAFORMATS_TRIGGERACTIVITYDATA_HPP_

include/trgdataformats/TriggerCandidateData.hpp

@@ -1,5 +1,12 @@
 /**
- * @file TriggerCandidate.hpp
+ * @file TriggerCandidateData.hpp
+ *
+ * This header defines the TriggerCandidateData struct, which
+ * aggregates information about a found set of associated trigger
+ * activities (general type of candidate, algorithm used, times
+ * involved, etc.). It does *not* include per-trigger-activity
+ * information, which need to be associated with TriggerCandidateData
+ * in a higher level object.
  *
  * This is part of the DUNE DAQ Application Framework, copyright 2020.
  * Licensing/copyright details are in the COPYING file that you should have
@@ -11,6 +18,7 @@
 
 #include "trgdataformats/Types.hpp"
 
+#include <array>
 #include <cstdint>
 #include <map>
 #include <string>
@@ -19,7 +27,11 @@ namespace dunedaq::trgdataformats {
 
 struct TriggerCandidateData
 {
-  enum class Type
+  // If you add an enum to TriggerCandidateData::Type, declare it
+  // second-to-last (just above kFinalEnum) and ensure its value is +1
+  // greater than the enum above it
+
+  enum class Type : int
   {
     kUnknown = 0,
     kTiming = 1,
@@ -60,9 +72,10 @@ struct TriggerCandidateData
     kDTSPulser = 36,
     kDTSCosmic = 37,
     kSSPLEDCalibration = 38,
+    kFinalEnum
   };
 
-  enum class Algorithm
+  enum class Algorithm : int
   {
     kUnknown = 0,
     kSupernova = 1,
@@ -79,91 +92,36 @@ struct TriggerCandidateData
     kChannelAdjacency = 12,
   };
 
-  // Update this version number if there are any changes to the in-memory representation of this class!
-  static constexpr version_t s_trigger_candidate_version = 3; // NOLINT(build/unsigned)
+  static constexpr version_t s_trigger_candidate_version = 3;
 
-  version_t version = s_trigger_candidate_version; // NOLINT(build/unsigned)
-  timestamp_t time_start = INVALID_TIMESTAMP;
-  timestamp_t time_end = INVALID_TIMESTAMP;
-  timestamp_t time_candidate = INVALID_TIMESTAMP;
+  version_t version = s_trigger_candidate_version;
+  timestamp_t time_start = TypeDefaults::s_invalid_timestamp;
+  timestamp_t time_end = TypeDefaults::s_invalid_timestamp;
+  timestamp_t time_candidate = TypeDefaults::s_invalid_timestamp;
   // TODO P. Rodrigues 2021-01-06: This was originally a
   // std::vector<detid_t> but that messes up the overlay scheme, so
   // I've changed it for now to be just a detid_t. Need to work out
   // what to do longer term
-  detid_t detid; // NOLINT(build/unsigned)
+  detid_t detid = TypeDefaults::s_invalid_detid;
   Type type = Type::kUnknown;
-  Algorithm algorithm = Algorithm::kUnknown; // NOLINT(build/unsigned)
+  Algorithm algorithm = Algorithm::kUnknown;
 };
 
-// This map needs to be updated for each new TC type, as this is used when configuring Trigger Bitwords, affecting
-// trigger logic in trigger::MLT
-inline std::map<TriggerCandidateData::Type, std::string>
-get_trigger_candidate_type_names()
-{
-  return {
-    { TriggerCandidateData::Type::kUnknown, "kUnknown" },
-    { TriggerCandidateData::Type::kTiming, "kTiming" },
-    { TriggerCandidateData::Type::kTPCLowE, "kTPCLowE" },
-    { TriggerCandidateData::Type::kSupernova, "kSupernova" },
-    { TriggerCandidateData::Type::kRandom, "kRandom" },
-    { TriggerCandidateData::Type::kPrescale, "kPrescale" },
-    { TriggerCandidateData::Type::kADCSimpleWindow, "kADCSimpleWindow" },
-    { TriggerCandidateData::Type::kHorizontalMuon, "kHorizontalMuon" },
-    { TriggerCandidateData::Type::kMichelElectron, "kMichelElectron" },
-    { TriggerCandidateData::Type::kPlaneCoincidence, "kPlaneCoincidence" },
-    { TriggerCandidateData::Type::kDBSCAN, "kDBSCAN" },
-    { TriggerCandidateData::Type::kChannelDistance, "kChannelDistance" },
-    { TriggerCandidateData::Type::kBundle, "kBundle" },
-    { TriggerCandidateData::Type::kCTBFakeTrigger, "kCTBFakeTrigger" },
-    { TriggerCandidateData::Type::kCTBBeam, "kCTBBeam" },
-    { TriggerCandidateData::Type::kCTBBeamChkvHL, "kCTBBeamChkvHL" },
-    { TriggerCandidateData::Type::kCTBCustomD, "kCTBCustomD" },
-    { TriggerCandidateData::Type::kCTBCustomE, "kCTBCustomE" },
-    { TriggerCandidateData::Type::kCTBCustomF, "kCTBCustomF" },
-    { TriggerCandidateData::Type::kCTBCustomG, "kCTBCustomG" },
-    { TriggerCandidateData::Type::kCTBBeamChkvHLx, "kCTBBeamChkvHLx" },
-    { TriggerCandidateData::Type::kCTBBeamChkvHxL, "kCTBBeamChkvHxL" },
-    { TriggerCandidateData::Type::kCTBBeamChkvHxLx, "kCTBBeamChkvHxLx" },
-    { TriggerCandidateData::Type::kNeutronSourceCalib, "kNeutronSourceCalib" },
-    { TriggerCandidateData::Type::kChannelAdjacency, "kChannelAdjacency" },
-    { TriggerCandidateData::Type::kCIBFakeTrigger, "kCIBFakeTrigger" },
-    { TriggerCandidateData::Type::kCIBLaserTriggerP1, "kCIBLaserTriggerP1" },
-    { TriggerCandidateData::Type::kCIBLaserTriggerP2, "kCIBLaserTriggerP2" },
-    { TriggerCandidateData::Type::kCIBLaserTriggerP3, "kCIBLaserTriggerP3" },
-    { TriggerCandidateData::Type::kCTBOffSpillSnapshot, "kCTBOffSpillSnapshot" },
-    { TriggerCandidateData::Type::kCTBOffSpillCosmicJura, "kCTBOffSpillCosmicJura" },
-    { TriggerCandidateData::Type::kCTBOffSpillCRTCosmic, "kCTBOffSpillCRTCosmic" },
-    { TriggerCandidateData::Type::kCTBBeamSpillStart, "kCTBBeamSpillStart" },
-    { TriggerCandidateData::Type::kCTBBeamSpillSnapshot, "kCTBBeamSpillSnapshot" },
-    { TriggerCandidateData::Type::kCTBCustomC, "kCTBCustomC" },
-    { TriggerCandidateData::Type::kCTBCustomPulseTrain, "kCTBCustomPulseTrain" },
-    { TriggerCandidateData::Type::kDTSPulser, "kDTSPulser" },
-    { TriggerCandidateData::Type::kDTSCosmic, "kDTSCosmic" },
-    { TriggerCandidateData::Type::kSSPLEDCalibration, "kSSPLEDCalibration" },
-  };
-}
+  inline TriggerCandidateData::Type
+  string_to_trigger_candidate_type(const std::string& name);
 
-inline int
-string_to_trigger_candidate_type(const std::string& name)
-{
-  for (auto& it : get_trigger_candidate_type_names()) {
-    if (it.second == name)
-      return static_cast<int>(it.first);
-  }
-  return static_cast<int>(TriggerCandidateData::Type::kUnknown);
-}
-
-inline std::string
-trigger_candidate_type_to_string(const TriggerCandidateData::Type& type)
-{
-  try {
-    return get_trigger_candidate_type_names().at(type);
-  }
-  catch(std::exception &e) {
-  }
-  return "kUnknown";
-}
+  inline std::string
+  trigger_candidate_type_to_string(const TriggerCandidateData::Type type);
 
 } // namespace dunedaq::trgdataformats
 
+#include "detail/TriggerCandidateData.hxx"
+
+// This static_assert is meant to alert the developer to bump the
+// version if variables are added or removed in TriggerCandidateData
+static_assert(
+	      dunedaq::trgdataformats::TriggerCandidateData::s_trigger_candidate_version == 3 &&
+	      sizeof(dunedaq::trgdataformats::TriggerCandidateData) == 48
+	      );
+
 #endif // TRGDATAFORMATS_INCLUDE_TRGDATAFORMATS_TRIGGERCANDIDATEDATA_HPP_

include/trgdataformats/TriggerObjectOverlay.hpp

@@ -1,6 +1,16 @@
 /**
  * @file TriggerObjectOverlay.hpp
  *
+ * This header defines the "TriggerActivity" and "TriggerCandidate"
+ * structs, which are instantiations of the TriggerObjectOverlay
+ * template. Continguous in memory, a TriggerActivity instance is an
+ * instance of TriggerActivityData followed by n_inputs
+ * TriggerPrimitive instances. Likewise, a TriggerCandidate instance
+ * is an instance of TriggerCandidateData followed by n_inputs
+ * TriggerActivityData (not to be confused with TriggerActivity)
+ * instances.
+ *
+ *
  * This is part of the DUNE DAQ Application Framework, copyright 2020.
  * Licensing/copyright details are in the COPYING file that you should have
  * received with this code.
@@ -13,6 +23,9 @@
 #include "trgdataformats/TriggerCandidateData.hpp"
 #include "trgdataformats/TriggerPrimitive.hpp"
 
+#include <algorithm>
+#include <span>
+
 namespace dunedaq::trgdataformats {
 
 template<class DataType, class InputType>
@@ -21,11 +34,17 @@ struct TriggerObjectOverlay
   using data_t = DataType;
   using input_t = InputType;
   data_t data;
-  uint64_t n_inputs;
+  uint64_t n_inputs; // NOLINT(build/unsigned)
   #pragma GCC diagnostic push
   #pragma GCC diagnostic ignored "-Wpedantic"
   input_t inputs[]; // Non-standard flexible array member, but alternatives are worse
   #pragma GCC diagnostic pop
+
+  void set_inputs(std::span<const input_t> in)
+  {
+    n_inputs = static_cast<uint64_t>(in.size());
+    std::copy(in.begin(), in.end(), inputs);
+  }
 };
 
 using TriggerActivity = TriggerObjectOverlay<TriggerActivityData, TriggerPrimitive>;

include/trgdataformats/TriggerPrimitive.hpp

@@ -1,6 +1,9 @@
 /**
  * @file TriggerPrimitive.hpp
  *
+ * This header defines an overlay struct for trigger primitives, a
+ * single energy deposition on a TPC or PDS channel.
+ *
  * This is part of the DUNE DAQ Application Framework, copyright 2020.
  * Licensing/copyright details are in the COPYING file that you should have
  * received with this code.
@@ -18,14 +21,21 @@
 #include <limits>
 #include <type_traits>
 
+// NOLINTBEGIN(build/unsigned)
+
 namespace dunedaq::trgdataformats {
 
 /**
  * @brief A single energy deposition on a TPC or PDS channel
  */
 struct TriggerPrimitive
 {
-  static constexpr uint8_t s_trigger_primitive_version = 2;
+  static constexpr version_t s_trigger_primitive_version = 2;
+  static constexpr std::size_t s_expected_bytes = 24;
+
+  static constexpr uint16_t s_invalid_samples_over_threshold = std::numeric_limits<uint16_t>::max();
+  static constexpr uint16_t s_invalid_samples_to_peak = std::numeric_limits<uint16_t>::max();
+  static constexpr channel_t s_invalid_tp_channel = 0xFFFFFF; // TP channel limit is at 24 bytes
 
   // Metadata.
   uint64_t version : 8;
@@ -45,16 +55,55 @@ struct TriggerPrimitive
   TriggerPrimitive()
     : version(s_trigger_primitive_version)
     , flag(0)
-    , detid(INVALID_DETID)
-    , channel(INVALID_TP_CHANNEL)
-    , samples_over_threshold(INVALID_SAMPLES_OVER_THRESHOLD)
-    , time_start(INVALID_TIMESTAMP)
-    , samples_to_peak(INVALID_SAMPLES_TO_PEAK)
+    , detid(TypeDefaults::s_invalid_detid)
+    , channel(s_invalid_tp_channel)
+    , samples_over_threshold(s_invalid_samples_over_threshold)
+    , time_start(TypeDefaults::s_invalid_timestamp)
+    , samples_to_peak(s_invalid_samples_to_peak)
     , adc_integral(0)
     , adc_peak(0)
   {}
+
+  // Despite this being a struct, the getter and setter for timestamp
+  // brings the struct closer into compliance with the standard
+  // overlay
+
+  timestamp_t get_timestamp() const
+  {
+    return time_start;
+  }
+
+  void set_timestamp(timestamp_t ts)
+  {
+    time_start = ts;
+  }
+
+  bool operator<(const TriggerPrimitive& other) const {
+    return std::tie(time_start, channel) < std::tie(other.time_start, other.channel);
+  }
 };
 
+// Basic checks that the bits are arranged as we hope they're arranged
+
+// TODO John Freeman (jcfree@fnal.gov), Aug-1-2026
+
+// In the next two months, take these static_asserts and write up a
+// concept which encompasses them - something like
+// SafeBitLayoutConcept - and figure out the right package to put it
+// in, so that both this and the far detector-specific overlay classes
+// can use them
+
+static_assert(sizeof(TriggerPrimitive) == TriggerPrimitive::s_expected_bytes,
+	      "The actual size of the TriggerPrimitive isn't the expected size; the compiler is likely inserting padding");
+
+static_assert(std::endian::native == std::endian::little,
+              "The TriggerPrimitive bitfield layout assumes little-endian architecture");
+
+static_assert(std::is_standard_layout_v<TriggerPrimitive>);
+static_assert(std::is_trivially_copyable_v<TriggerPrimitive>);
+
 } // namespace dunedaq::trgdataformats
 
+// NOLINTEND(build/unsigned)
+
 #endif // TRGDATAFORMATS_INCLUDE_TRGDATAFORMATS_TRIGGERPRIMITIVE_HPP_