Merge branch 'main' into 1052-exception-emitted-in-a-jchainfactorytpr…

…ocess-gets-silently-absorbed

.github/ubsan.supp

@@ -0,0 +1,3 @@
+implicit-integer-sign-change:JsonMaterialDecorator
+implicit-integer-sign-change:/usr/local/include/eigen3/Eigen/src/Core/arch/SSE/PacketMath.h
+implicit-integer-sign-change:TObject

.github/workflows/linux-eic-shell.yml

@@ -18,6 +18,7 @@ concurrency:
 env:
   ASAN_OPTIONS: suppressions=${{ github.workspace }}/.github/asan.supp:malloc_context_size=20:detect_leaks=1:verify_asan_link_order=0:detect_stack_use_after_return=1:detect_odr_violation=1:new_delete_type_mismatch=0:intercept_tls_get_addr=0
   LSAN_OPTIONS: suppressions=${{ github.workspace }}/.github/lsan.supp
+  UBSAN_OPTIONS: suppressions=${{ github.workspace }}/.github/ubsan.supp:print_stacktrace=1
 
 jobs:
   build:
@@ -69,7 +70,7 @@ jobs:
         platform-release: "jug_xl:nightly"
         run: |
           # install this repo
-          CC="${{ matrix.CC }}" CXX="${{ matrix.CXX }}" CXXFLAGS="${{ matrix.CXXFLAGS }}" cmake -B build -S . -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_BUILD_TYPE=${{ matrix.CMAKE_BUILD_TYPE }} -DUSE_ASAN=ON -DUSE_TSAN=OFF -DUSE_UBSAN=OFF
+          CC="${{ matrix.CC }}" CXX="${{ matrix.CXX }}" CXXFLAGS="${{ matrix.CXXFLAGS }}" cmake -B build -S . -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_BUILD_TYPE=${{ matrix.CMAKE_BUILD_TYPE }} -DUSE_ASAN=ON -DUSE_TSAN=OFF -DUSE_UBSAN=ON
           cmake --build build -- -j 2 install
           ccache --show-stats --verbose
     - name: Check dynamic library loader paths
@@ -357,6 +358,39 @@ jobs:
         path: rec_${{ matrix.particle }}_1GeV_20GeV_${{ matrix.detector_config }}.edm4eic.root
         if-no-files-found: error
 
+  eicrecon-eicmkplugin:
+    runs-on: ubuntu-latest
+    needs:
+    - build
+    - npsim-gun
+    strategy:
+      matrix:
+        CC: [gcc]
+        particle: [e]
+        detector_config: [brycecanyon]
+    steps:
+    - uses: actions/download-artifact@v3
+      with:
+        name: install-${{ matrix.CC }}-eic-shell-Release
+    - uses: actions/download-artifact@v3
+      with:
+        name: sim_${{ matrix.particle }}_1GeV_20GeV_${{ matrix.detector_config }}.edm4hep.root
+    - uses: cvmfs-contrib/github-action-cvmfs@v3
+    - name: Run EICrecon
+      uses: eic/run-cvmfs-osg-eic-shell@main
+      with:
+        platform-release: "jug_xl:nightly"
+        setup: /opt/detector/setup.sh
+        run: |
+          export DETECTOR_CONFIG=${DETECTOR}_${{ matrix.detector_config }}
+          export LD_LIBRARY_PATH=$PWD/lib:$LD_LIBRARY_PATH
+          export JANA_PLUGIN_PATH=$PWD/lib/EICrecon/plugins${JANA_PLUGIN_PATH:+:${JANA_PLUGIN_PATH}}
+          chmod a+x bin/*
+          $PWD/bin/eicmkplugin.py MyCustomPlugin
+          cmake -S MyCustomPlugin -B MyCustomPlugin/build -DUSER_PLUGIN_OUTPUT_DIRECTORY=$PWD/lib/EICrecon/plugins
+          cmake --build MyCustomPlugin/build --target install
+          $PWD/bin/eicrecon -Pplugins=MyCustomPlugin -Ppodio:output_file=rec_${{ matrix.particle }}_1GeV_20GeV_${{ matrix.detector_config }}.edm4eic.root sim_${{ matrix.particle }}_1GeV_20GeV_${{ matrix.detector_config }}.edm4hep.root -Pplugins=dump_flags,janadot -Pdump_flags:json=${{ matrix.particle }}_${{ matrix.detector_config }}_flags.json -Pjana:warmup_timeout=0 -Pjana:timeout=0
+
   eicrecon-benchmarks-plugins:
     runs-on: ubuntu-latest
     needs:

.pre-commit-config.yaml

@@ -2,7 +2,7 @@ ci:
     skip: [clang-format]
 repos:
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.4.0
+  rev: v4.5.0
   hooks:
   - id: end-of-file-fixer
   - id: trailing-whitespace

CMakeLists.txt

@@ -116,6 +116,9 @@ find_package(podio REQUIRED)
 find_package(EDM4HEP REQUIRED)
 find_package(EDM4EIC 2.1 REQUIRED)
 
+# Guidelines Support Library
+find_package(Microsoft.GSL CONFIG)
+
 # Remove PODIO_JSON_OUTPUT (ref: https://github.com/AIDASoft/podio/issues/475)
 get_target_property(EDM4HEP_INTERFACE_COMPILE_DEFINITIONS EDM4HEP::edm4hep INTERFACE_COMPILE_DEFINITIONS)
 if(EDM4HEP_INTERFACE_COMPILE_DEFINITIONS)
@@ -171,13 +174,14 @@ endif()
 # Undefined behavior sanitizer
 option(USE_UBSAN "Compile with undefined behavior sanitizer" OFF)
 if (${USE_UBSAN})
-    foreach (sanitizer undefined nullability implicit-conversion)
+    foreach (sanitizer undefined float-divide-by-zero unsigned-integer-overflow implicit-conversion local-bounds nullability)
         check_cxx_compiler_flag("-fsanitize=${sanitizer}" CXX_COMPILER_HAS_sanitize_${sanitizer})
         if (CXX_COMPILER_HAS_sanitize_${sanitizer})
-            add_compile_options(-fsanitize=${sanitizer} -g -O1)
+            add_compile_options(-fsanitize=${sanitizer})
             add_link_options(-fsanitize=${sanitizer})
         endif()
     endforeach()
+    add_compile_options(-g -O1)
 endif()
 
 # Compress debug symbols if supported

cmake/jana_plugin.cmake

@@ -31,6 +31,9 @@ macro(plugin_add _name)
     # include fmt by default
     find_package(fmt REQUIRED)
 
+    # include gsl by default
+    find_package(Microsoft.GSL CONFIG)
+
     # Define plugin
     if(${_name}_WITH_PLUGIN)
         add_library(${_name}_plugin SHARED ${PLUGIN_SOURCES})
@@ -41,6 +44,7 @@ macro(plugin_add _name)
         set_target_properties(${_name}_plugin PROPERTIES PREFIX "" OUTPUT_NAME "${_name}" SUFFIX ".so")
         target_link_libraries(${_name}_plugin ${JANA_LIB} spdlog::spdlog)
         target_link_libraries(${_name}_plugin ${JANA_LIB} fmt::fmt)
+        target_link_libraries(${_name}_plugin Microsoft.GSL::GSL)
 
         # Install plugin
         install(TARGETS ${_name}_plugin DESTINATION ${PLUGIN_OUTPUT_DIRECTORY})
@@ -61,6 +65,7 @@ macro(plugin_add _name)
         target_include_directories(${_name}_library SYSTEM PUBLIC ${JANA_INCLUDE_DIR} )
         target_link_libraries(${_name}_library ${JANA_LIB} spdlog::spdlog)
         target_link_libraries(${_name}_library ${JANA_LIB} fmt::fmt)
+        target_link_libraries(${_name}_library Microsoft.GSL::GSL)
 
         # Install plugin
         install(TARGETS ${_name}_library DESTINATION ${PLUGIN_LIBRARY_OUTPUT_DIRECTORY})

src/algorithms/calorimetry/CalorimeterHitReco.cc

@@ -15,9 +15,9 @@ using namespace dd4hep;
 
 namespace eicrecon {
 
-void CalorimeterHitReco::init(const dd4hep::Detector* detector, std::shared_ptr<spdlog::logger>& logger) {
+void CalorimeterHitReco::init(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr<spdlog::logger>& logger) {
     m_detector = detector;
-    m_converter = std::make_shared<const dd4hep::rec::CellIDPositionConverter>(const_cast<dd4hep::Detector&>(*detector));
+    m_converter = converter;
     m_log = logger;
 
     // threshold for firing
@@ -118,7 +118,6 @@ std::unique_ptr<edm4eic::CalorimeterHitCollection> CalorimeterHitReco::process(c
     // number is encountered disable this algorithm. A useful message
     // indicating what is going on is printed below where the
     // error is detector.
-    auto decoder = m_detector->readout(m_cfg.readout).idSpec().decoder();
     if (NcellIDerrors >= MaxCellIDerrors) return std::move(recohits);
 
     for (const auto &rh: rawhits) {
@@ -129,22 +128,29 @@ std::unique_ptr<edm4eic::CalorimeterHitCollection> CalorimeterHitReco::process(c
             continue;
         }
 
+        // get layer and sector ID
+        const int lid =
+                id_dec != nullptr && !m_cfg.layerField.empty() ? static_cast<int>(id_dec->get(cellID, layer_idx)) : -1;
+        const int sid =
+                id_dec != nullptr && !m_cfg.sectorField.empty() ? static_cast<int>(id_dec->get(cellID, sector_idx)) : -1;
+
+        // determine sampling fraction
+        float sampFrac = m_cfg.sampFrac;
+        if (! m_cfg.sampFracLayer.empty()) {
+            if (0 <= lid && lid < m_cfg.sampFracLayer.size()) {
+                sampFrac = m_cfg.sampFracLayer[lid];
+            } else {
+                throw std::runtime_error(fmt::format("CalorimeterHitReco: layer-specific sampling fraction undefined for index {}", lid));
+            }
+        }
+
         // convert ADC to energy
         float energy = (((signed) rh.getAmplitude() - (signed) m_cfg.pedMeanADC)) / static_cast<float>(m_cfg.capADC) * m_cfg.dyRangeADC /
-                m_cfg.sampFrac;
-        if (m_cfg.readout == "LFHCALHits" && m_cfg.sampFracLayer[0] != 0.){
-          energy = (((signed) rh.getAmplitude() - (signed) m_cfg.pedMeanADC)) / static_cast<float>(m_cfg.capADC) * m_cfg.dyRangeADC /
-                    m_cfg.sampFracLayer[decoder->get(cellID, decoder->index("rlayerz"))]; // use readout layer depth information from decoder
-        }
+                sampFrac;
 
         const float time = rh.getTimeStamp() / stepTDC;
         m_log->trace("cellID {}, \t energy: {},  TDC: {}, time: ", cellID, energy, rh.getTimeStamp(), time);
 
-        const int lid =
-                id_dec != nullptr && !m_cfg.layerField.empty() ? static_cast<int>(id_dec->get(cellID, layer_idx)) : -1;
-        const int sid =
-                id_dec != nullptr && !m_cfg.sectorField.empty() ? static_cast<int>(id_dec->get(cellID, sector_idx)) : -1;
-
         dd4hep::Position gpos;
         try {
             // global positions

src/algorithms/calorimetry/CalorimeterHitReco.h

@@ -24,7 +24,7 @@ namespace eicrecon {
   class CalorimeterHitReco : public WithPodConfig<CalorimeterHitRecoConfig> {
 
   public:
-    void init(const dd4hep::Detector* detector, std::shared_ptr<spdlog::logger>& logger);
+    void init(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr<spdlog::logger>& logger);
     std::unique_ptr<edm4eic::CalorimeterHitCollection> process(const edm4hep::RawCalorimeterHitCollection &rawhits);
 
   private:
@@ -46,7 +46,7 @@ namespace eicrecon {
 
   private:
     const dd4hep::Detector* m_detector;
-    std::shared_ptr<const dd4hep::rec::CellIDPositionConverter> m_converter;
+    const dd4hep::rec::CellIDPositionConverter* m_converter;
     std::shared_ptr<spdlog::logger> m_log;
 
   };

src/algorithms/calorimetry/CalorimeterHitsMerger.cc

@@ -12,9 +12,9 @@
 
 namespace eicrecon {
 
-void CalorimeterHitsMerger::init(const dd4hep::Detector* detector, std::shared_ptr<spdlog::logger>& logger) {
+void CalorimeterHitsMerger::init(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr<spdlog::logger>& logger) {
     m_detector = detector;
-    m_converter = std::make_shared<const dd4hep::rec::CellIDPositionConverter>(const_cast<dd4hep::Detector&>(*detector));
+    m_converter = converter;
     m_log = logger;
 
     if (m_cfg.readout.empty()) {

src/algorithms/calorimetry/CalorimeterHitsMerger.h

@@ -31,15 +31,15 @@ namespace eicrecon {
   class CalorimeterHitsMerger : public WithPodConfig<CalorimeterHitsMergerConfig>  {
 
   public:
-    void init(const dd4hep::Detector* detector, std::shared_ptr<spdlog::logger>& logger);
+    void init(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr<spdlog::logger>& logger);
     std::unique_ptr<edm4eic::CalorimeterHitCollection> process(const edm4eic::CalorimeterHitCollection &input);
 
   private:
     uint64_t id_mask{0}, ref_mask{0};
 
   private:
     const dd4hep::Detector* m_detector;
-    std::shared_ptr<const dd4hep::rec::CellIDPositionConverter> m_converter;
+    const dd4hep::rec::CellIDPositionConverter* m_converter;
     std::shared_ptr<spdlog::logger> m_log;
 
   };

src/algorithms/digi/PhotoMultiplierHitDigi.cc

@@ -18,12 +18,12 @@
 //------------------------
 // AlgorithmInit
 //------------------------
-void eicrecon::PhotoMultiplierHitDigi::AlgorithmInit(dd4hep::Detector *detector, std::shared_ptr<spdlog::logger>& logger)
+void eicrecon::PhotoMultiplierHitDigi::AlgorithmInit(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr<spdlog::logger>& logger)
 {
     // services
     m_detector = detector;
-    m_cellid_converter = std::make_shared<const dd4hep::rec::CellIDPositionConverter>(*detector);
-    m_log=logger;
+    m_converter = converter;
+    m_log = logger;
 
     // print the configuration parameters
     m_cfg.Print(m_log, spdlog::level::debug);
@@ -132,7 +132,7 @@ eicrecon::PhotoMultiplierHitDigiResult eicrecon::PhotoMultiplierHitDigi::Algorit
             // cell time, signal amplitude
             double   amp  = m_cfg.speMean + m_rngNorm()*m_cfg.speError;
             TimeType time = m_cfg.noiseTimeWindow*m_rngUni() / dd4hep::ns;
-            dd4hep::Position pos_hit_global = m_cellid_converter->position(id);
+            dd4hep::Position pos_hit_global = m_converter->position(id);
 
             // insert in `hit_groups`, or if the pixel already has a hit, update `npe` and `signal`
             this->InsertHit(

src/algorithms/digi/PhotoMultiplierHitDigi.h

@@ -41,7 +41,7 @@ class PhotoMultiplierHitDigi : public WithPodConfig<PhotoMultiplierHitDigiConfig
 public:
     PhotoMultiplierHitDigi() = default;
     ~PhotoMultiplierHitDigi(){}
-    void AlgorithmInit(dd4hep::Detector *detector, std::shared_ptr<spdlog::logger>& logger);
+    void AlgorithmInit(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr<spdlog::logger>& logger);
     void AlgorithmChangeRun();
     PhotoMultiplierHitDigiResult AlgorithmProcess(
         const edm4hep::SimTrackerHitCollection* sim_hits
@@ -106,10 +106,10 @@ class PhotoMultiplierHitDigi : public WithPodConfig<PhotoMultiplierHitDigiConfig
         bool             is_noise_hit = false
         );
 
-    dd4hep::Detector *m_detector   = nullptr;
+    const dd4hep::Detector* m_detector = nullptr;
+    const dd4hep::rec::CellIDPositionConverter* m_converter;
 
     std::shared_ptr<spdlog::logger> m_log;
-    std::shared_ptr<const dd4hep::rec::CellIDPositionConverter> m_cellid_converter;
 
     // std::default_random_engine generator; // TODO: need something more appropriate here
     // std::normal_distribution<double> m_normDist; // defaults to mean=0, sigma=1

src/algorithms/fardetectors/MatrixTransferStatic.cc

@@ -5,13 +5,13 @@
 
 #include "MatrixTransferStatic.h"
 
-void eicrecon::MatrixTransferStatic::init(std::shared_ptr<const dd4hep::rec::CellIDPositionConverter> id_conv,
-                                          const dd4hep::Detector* det,
+void eicrecon::MatrixTransferStatic::init(const dd4hep::Detector* det,
+                                          const dd4hep::rec::CellIDPositionConverter* id_conv,
                                           std::shared_ptr<spdlog::logger> &logger) {
 
-  m_log              = logger;
-  m_detector         = det;
-  m_cellid_converter = id_conv;
+  m_log       = logger;
+  m_detector  = det;
+  m_converter = id_conv;
   //Calculate inverse of static transfer matrix
   std::vector<std::vector<double>> aX(m_cfg.aX);
   std::vector<std::vector<double>> aY(m_cfg.aY);
@@ -64,7 +64,7 @@ std::unique_ptr<edm4eic::ReconstructedParticleCollection> eicrecon::MatrixTransf
 
     auto cellID = h.getCellID();
     // The actual hit position in Global Coordinates
-    auto gpos = m_cellid_converter->position(cellID);
+    auto gpos = m_converter->position(cellID);
     // local positions
     auto volman = m_detector->volumeManager();
     auto local = volman.lookupDetElement(cellID);

src/algorithms/fardetectors/MatrixTransferStatic.h

@@ -26,7 +26,7 @@ namespace eicrecon {
     double aYinv[2][2] = {{0.0, 0.0},
                           {0.0, 0.0}};
 
-    void init(const std::shared_ptr<const dd4hep::rec::CellIDPositionConverter>,const dd4hep::Detector* det,std::shared_ptr<spdlog::logger> &logger);
+    void init(const dd4hep::Detector* det, const dd4hep::rec::CellIDPositionConverter* id_conv, std::shared_ptr<spdlog::logger> &logger);
 
     std::unique_ptr<edm4eic::ReconstructedParticleCollection> produce(const edm4hep::SimTrackerHitCollection &inputhits);
 
@@ -35,7 +35,7 @@ namespace eicrecon {
     /** algorithm logger */
     std::shared_ptr<spdlog::logger>   m_log;
     const dd4hep::Detector* m_detector{nullptr};
-    std::shared_ptr<const dd4hep::rec::CellIDPositionConverter> m_cellid_converter = nullptr;
+    const dd4hep::rec::CellIDPositionConverter* m_converter{nullptr};
 
   };
 }

src/algorithms/tracking/ActsGeometryProvider.cc

@@ -72,7 +72,7 @@ void draw_surfaces(std::shared_ptr<const Acts::TrackingGeometry> trk_geo, const
 }
 
 
-void ActsGeometryProvider::initialize(dd4hep::Detector *dd4hep_geo,
+void ActsGeometryProvider::initialize(const dd4hep::Detector* dd4hep_geo,
                                       std::string material_file,
                                       std::shared_ptr<spdlog::logger> log,
                                       std::shared_ptr<spdlog::logger> init_log) {

src/algorithms/tracking/ActsGeometryProvider.h

@@ -32,7 +32,6 @@ namespace dd4hep {
     class Detector;
 }
 namespace dd4hep::rec {
-    class CellIDPositionConverter;
     class Surface;
 }
 
@@ -47,12 +46,12 @@ class ActsGeometryProvider {
     ActsGeometryProvider() {}
     using VolumeSurfaceMap = std::unordered_map<uint64_t, const Acts::Surface *>;
 
-    virtual void initialize(dd4hep::Detector* dd4hep_geo,
+    virtual void initialize(const dd4hep::Detector* dd4hep_geo,
                             std::string material_file,
                             std::shared_ptr<spdlog::logger> log,
                             std::shared_ptr<spdlog::logger> init_log) final;
 
-    dd4hep::Detector*  dd4hepDetector() const {return m_dd4hepDetector; }
+    const dd4hep::Detector* dd4hepDetector() const { return m_dd4hepDetector; }
 
     /** Gets the ACTS tracking geometry.
      */
@@ -85,7 +84,7 @@ class ActsGeometryProvider {
      * This is the main dd4hep detector handle.
      * <a href="https://dd4hep.web.cern.ch/dd4hep/reference/classdd4hep_1_1Detector.html">See DD4hep Detector documentation</a>
      */
-    dd4hep::Detector *m_dd4hepDetector = nullptr;
+    const dd4hep::Detector* m_dd4hepDetector = nullptr;
 
     /// DD4hep surface map
     std::map<int64_t, dd4hep::rec::Surface *> m_surfaceMap;
@@ -105,12 +104,6 @@ class ActsGeometryProvider {
     /// ACTS surface lookup container for hit surfaces that generate smeared hits
     VolumeSurfaceMap m_surfaces;
 
-    /** DD4hep CellID tool.
-     *  Use to lookup geometry information for a hit with cellid number (int64_t).
-     *  <a href="https://dd4hep.web.cern.ch/dd4hep/reference/classdd4hep_1_1rec_1_1CellIDPositionConverter.html">See DD4hep CellIDPositionConverter documentation</a>
-     */
-    std::shared_ptr<const dd4hep::rec::CellIDPositionConverter> m_cellid_converter = nullptr;
-
     /// Acts magnetic field
     std::shared_ptr<const eicrecon::BField::DD4hepBField> m_magneticField = nullptr;