From aef9ecaf6207274726452162a23474b1b78a6ad0 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Thu, 5 Oct 2023 16:10:01 -0500 Subject: [PATCH 01/13] fix: remove adhoc LFHCAL treatment in CalorimeterHitReco (#1054) ### Briefly, what does this PR introduce? This removes adhoc LFHCAL treatment in CalorimeterHitReco and implements a configurable approach that uses previously implemented layer ID resolution. Instead of `if (m_cfg.readout == "LFHCALHits")`, this now: - uses the cellID decoder `id_dec` defined on init intsead of a local version, - doesn't depend on fragile testing `sampFracLayer[0] != 0.` but instead `sampFracLayer.empty()`, - single place to do the energy reconstruction based on the varying sampFrac, - single place to do the layer field id decoding. TODO: - [x] check diffs in CI: LFHCALRecHits.layer is now filled too ### What kind of change does this PR introduce? - [x] Bug fix (issue #796) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] Changes have been communicated to collaborators @FriederikeBock ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --- .../calorimetry/CalorimeterHitReco.cc | 28 +++++++++++-------- src/detectors/FHCAL/FHCAL.cc | 1 + 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/algorithms/calorimetry/CalorimeterHitReco.cc b/src/algorithms/calorimetry/CalorimeterHitReco.cc index 5c893f3e69..c144085afc 100644 --- a/src/algorithms/calorimetry/CalorimeterHitReco.cc +++ b/src/algorithms/calorimetry/CalorimeterHitReco.cc @@ -118,7 +118,6 @@ std::unique_ptr 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 CalorimeterHitReco::process(c continue; } + // get layer and sector ID + const int lid = + id_dec != nullptr && !m_cfg.layerField.empty() ? static_cast(id_dec->get(cellID, layer_idx)) : -1; + const int sid = + id_dec != nullptr && !m_cfg.sectorField.empty() ? static_cast(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(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(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(id_dec->get(cellID, layer_idx)) : -1; - const int sid = - id_dec != nullptr && !m_cfg.sectorField.empty() ? static_cast(id_dec->get(cellID, sector_idx)) : -1; - dd4hep::Position gpos; try { // global positions diff --git a/src/detectors/FHCAL/FHCAL.cc b/src/detectors/FHCAL/FHCAL.cc index 73968c2ba2..63d7ef3da0 100644 --- a/src/detectors/FHCAL/FHCAL.cc +++ b/src/detectors/FHCAL/FHCAL.cc @@ -254,6 +254,7 @@ extern "C" { 0.037, // 13 }, .readout = "LFHCALHits", + .layerField = "rlayerz", }, app // TODO: Remove me once fixed )); From 05967227fe64bc5660fea1d2ead8c625e064d35a Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Thu, 5 Oct 2023 17:24:25 -0500 Subject: [PATCH 02/13] fix: use unique_ptr for geometry in DD4hep_service, hand out not_null const pointers (#1045) ### Briefly, what does this PR introduce? While looking into #1032, #1035, #1036 I started realizing two things: - cell ID position converters are created by algorithms from geometry instead of doing this once in the service, - there is no const protection in the detector pointers passed to algorithms (i.e. an algorithm could change geometry), - there is no protection against null pointers being passed from geometry. The first two points are related since the interface for creating a cell ID position converter requires a non-const detector (see also https://github.com/AIDASoft/DD4hep/pull/1148). This PR centralizes the creation of cell ID position converters, clarifies ownership through unique_ptrs, and makes sure no null pointers are passed to the algorithms. In places where the constructor gets the const not_null pointer, we can also impose this in the class on the relevant member variable. ### What kind of change does this PR introduce? - [x] Bug fix (issue: avoid creation of cell ID position converters in algorithms) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] Changes have been communicated to collaborators: - @simonge reordered MatrixTransferStatic init arguments - @c-dilks @chchatte92 this also affects RichGeo_service etc. ### Does this PR introduce breaking changes? What changes might users need to make to their code? Yes, now requires GSL (which has always been in eic-shell and is a requirement). ### Does this PR change default behavior? No. --------- Co-authored-by: Christopher Dilks Co-authored-by: Dmitry Kalinkin --- CMakeLists.txt | 3 ++ .../calorimetry/CalorimeterHitReco.cc | 4 +-- .../calorimetry/CalorimeterHitReco.h | 4 +-- .../calorimetry/CalorimeterHitsMerger.cc | 4 +-- .../calorimetry/CalorimeterHitsMerger.h | 4 +-- src/algorithms/digi/PhotoMultiplierHitDigi.cc | 8 ++--- src/algorithms/digi/PhotoMultiplierHitDigi.h | 6 ++-- .../fardetectors/MatrixTransferStatic.cc | 12 +++---- .../fardetectors/MatrixTransferStatic.h | 4 +-- .../tracking/ActsGeometryProvider.cc | 2 +- .../tracking/ActsGeometryProvider.h | 13 ++----- src/algorithms/tracking/DD4hepBField.h | 5 +-- .../tracking/TrackerHitReconstruction.cc | 9 +++-- .../tracking/TrackerHitReconstruction.h | 5 ++- .../tracking/TrackerSourceLinker.cc | 9 ++--- src/algorithms/tracking/TrackerSourceLinker.h | 10 +++--- .../femc_studies/femc_studiesProcessor.cc | 4 +-- .../lfhcal_studies/lfhcal_studiesProcessor.cc | 4 +-- .../CalorimeterClusterRecoCoG_factoryT.h | 5 ++- .../calorimetry/CalorimeterHitReco_factoryT.h | 2 +- .../CalorimeterHitsMerger_factoryT.h | 2 +- .../MatrixTransferStatic_factoryT.h | 2 +- .../TrackerHitReconstruction_factoryT.h | 2 +- .../digi/PhotoMultiplierHitDigi_factory.cc | 2 +- .../tracking/TrackerSourceLinker_factory.cc | 6 ++-- src/services/geometry/acts/ACTSGeo_service.h | 2 +- .../geometry/dd4hep/DD4hep_service.cc | 34 +++++++++---------- src/services/geometry/dd4hep/DD4hep_service.h | 28 +++++---------- src/services/geometry/richgeo/ActsGeo.cc | 2 +- src/services/geometry/richgeo/ActsGeo.h | 7 ++-- src/services/geometry/richgeo/IrtGeo.cc | 15 ++++---- src/services/geometry/richgeo/IrtGeo.h | 9 +++-- src/services/geometry/richgeo/IrtGeoDRICH.h | 4 +-- src/services/geometry/richgeo/IrtGeoPFRICH.h | 4 +-- src/services/geometry/richgeo/ReadoutGeo.cc | 17 +++++----- src/services/geometry/richgeo/ReadoutGeo.h | 7 ++-- .../geometry/richgeo/RichGeo_service.cc | 7 ++-- .../geometry/richgeo/RichGeo_service.h | 5 +-- .../calorimetry_CalorimeterHitDigi.cc | 6 +++- .../calorimetry_CalorimeterIslandCluster.cc | 8 +++-- 40 files changed, 138 insertions(+), 148 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e02c610f39..e6a597b685 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/src/algorithms/calorimetry/CalorimeterHitReco.cc b/src/algorithms/calorimetry/CalorimeterHitReco.cc index c144085afc..fa012703ee 100644 --- a/src/algorithms/calorimetry/CalorimeterHitReco.cc +++ b/src/algorithms/calorimetry/CalorimeterHitReco.cc @@ -15,9 +15,9 @@ using namespace dd4hep; namespace eicrecon { -void CalorimeterHitReco::init(const dd4hep::Detector* detector, std::shared_ptr& logger) { +void CalorimeterHitReco::init(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr& logger) { m_detector = detector; - m_converter = std::make_shared(const_cast(*detector)); + m_converter = converter; m_log = logger; // threshold for firing diff --git a/src/algorithms/calorimetry/CalorimeterHitReco.h b/src/algorithms/calorimetry/CalorimeterHitReco.h index 3593cafb22..c6ff35e5cc 100644 --- a/src/algorithms/calorimetry/CalorimeterHitReco.h +++ b/src/algorithms/calorimetry/CalorimeterHitReco.h @@ -24,7 +24,7 @@ namespace eicrecon { class CalorimeterHitReco : public WithPodConfig { public: - void init(const dd4hep::Detector* detector, std::shared_ptr& logger); + void init(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr& logger); std::unique_ptr process(const edm4hep::RawCalorimeterHitCollection &rawhits); private: @@ -46,7 +46,7 @@ namespace eicrecon { private: const dd4hep::Detector* m_detector; - std::shared_ptr m_converter; + const dd4hep::rec::CellIDPositionConverter* m_converter; std::shared_ptr m_log; }; diff --git a/src/algorithms/calorimetry/CalorimeterHitsMerger.cc b/src/algorithms/calorimetry/CalorimeterHitsMerger.cc index 2767f331e0..d29fc3a696 100644 --- a/src/algorithms/calorimetry/CalorimeterHitsMerger.cc +++ b/src/algorithms/calorimetry/CalorimeterHitsMerger.cc @@ -12,9 +12,9 @@ namespace eicrecon { -void CalorimeterHitsMerger::init(const dd4hep::Detector* detector, std::shared_ptr& logger) { +void CalorimeterHitsMerger::init(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr& logger) { m_detector = detector; - m_converter = std::make_shared(const_cast(*detector)); + m_converter = converter; m_log = logger; if (m_cfg.readout.empty()) { diff --git a/src/algorithms/calorimetry/CalorimeterHitsMerger.h b/src/algorithms/calorimetry/CalorimeterHitsMerger.h index 3eb180420c..809b97d1a9 100644 --- a/src/algorithms/calorimetry/CalorimeterHitsMerger.h +++ b/src/algorithms/calorimetry/CalorimeterHitsMerger.h @@ -31,7 +31,7 @@ namespace eicrecon { class CalorimeterHitsMerger : public WithPodConfig { public: - void init(const dd4hep::Detector* detector, std::shared_ptr& logger); + void init(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr& logger); std::unique_ptr process(const edm4eic::CalorimeterHitCollection &input); private: @@ -39,7 +39,7 @@ namespace eicrecon { private: const dd4hep::Detector* m_detector; - std::shared_ptr m_converter; + const dd4hep::rec::CellIDPositionConverter* m_converter; std::shared_ptr m_log; }; diff --git a/src/algorithms/digi/PhotoMultiplierHitDigi.cc b/src/algorithms/digi/PhotoMultiplierHitDigi.cc index f3d0822bc7..5676c92d3f 100644 --- a/src/algorithms/digi/PhotoMultiplierHitDigi.cc +++ b/src/algorithms/digi/PhotoMultiplierHitDigi.cc @@ -18,12 +18,12 @@ //------------------------ // AlgorithmInit //------------------------ -void eicrecon::PhotoMultiplierHitDigi::AlgorithmInit(dd4hep::Detector *detector, std::shared_ptr& logger) +void eicrecon::PhotoMultiplierHitDigi::AlgorithmInit(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr& logger) { // services m_detector = detector; - m_cellid_converter = std::make_shared(*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( diff --git a/src/algorithms/digi/PhotoMultiplierHitDigi.h b/src/algorithms/digi/PhotoMultiplierHitDigi.h index 8d8b399879..757655eaf5 100644 --- a/src/algorithms/digi/PhotoMultiplierHitDigi.h +++ b/src/algorithms/digi/PhotoMultiplierHitDigi.h @@ -41,7 +41,7 @@ class PhotoMultiplierHitDigi : public WithPodConfig& logger); + void AlgorithmInit(const dd4hep::Detector* detector, const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr& logger); void AlgorithmChangeRun(); PhotoMultiplierHitDigiResult AlgorithmProcess( const edm4hep::SimTrackerHitCollection* sim_hits @@ -106,10 +106,10 @@ class PhotoMultiplierHitDigi : public WithPodConfig m_log; - std::shared_ptr m_cellid_converter; // std::default_random_engine generator; // TODO: need something more appropriate here // std::normal_distribution m_normDist; // defaults to mean=0, sigma=1 diff --git a/src/algorithms/fardetectors/MatrixTransferStatic.cc b/src/algorithms/fardetectors/MatrixTransferStatic.cc index f2faea5056..382ad1f4d3 100644 --- a/src/algorithms/fardetectors/MatrixTransferStatic.cc +++ b/src/algorithms/fardetectors/MatrixTransferStatic.cc @@ -5,13 +5,13 @@ #include "MatrixTransferStatic.h" -void eicrecon::MatrixTransferStatic::init(std::shared_ptr id_conv, - const dd4hep::Detector* det, +void eicrecon::MatrixTransferStatic::init(const dd4hep::Detector* det, + const dd4hep::rec::CellIDPositionConverter* id_conv, std::shared_ptr &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> aX(m_cfg.aX); std::vector> aY(m_cfg.aY); @@ -64,7 +64,7 @@ std::unique_ptr 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); diff --git a/src/algorithms/fardetectors/MatrixTransferStatic.h b/src/algorithms/fardetectors/MatrixTransferStatic.h index 46a4dadf7a..9cf4725467 100644 --- a/src/algorithms/fardetectors/MatrixTransferStatic.h +++ b/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::Detector* det,std::shared_ptr &logger); + void init(const dd4hep::Detector* det, const dd4hep::rec::CellIDPositionConverter* id_conv, std::shared_ptr &logger); std::unique_ptr produce(const edm4hep::SimTrackerHitCollection &inputhits); @@ -35,7 +35,7 @@ namespace eicrecon { /** algorithm logger */ std::shared_ptr m_log; const dd4hep::Detector* m_detector{nullptr}; - std::shared_ptr m_cellid_converter = nullptr; + const dd4hep::rec::CellIDPositionConverter* m_converter{nullptr}; }; } diff --git a/src/algorithms/tracking/ActsGeometryProvider.cc b/src/algorithms/tracking/ActsGeometryProvider.cc index 0d337e7372..c917690c16 100644 --- a/src/algorithms/tracking/ActsGeometryProvider.cc +++ b/src/algorithms/tracking/ActsGeometryProvider.cc @@ -72,7 +72,7 @@ void draw_surfaces(std::shared_ptr 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 log, std::shared_ptr init_log) { diff --git a/src/algorithms/tracking/ActsGeometryProvider.h b/src/algorithms/tracking/ActsGeometryProvider.h index e432503a25..d983cf61ea 100644 --- a/src/algorithms/tracking/ActsGeometryProvider.h +++ b/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; - virtual void initialize(dd4hep::Detector* dd4hep_geo, + virtual void initialize(const dd4hep::Detector* dd4hep_geo, std::string material_file, std::shared_ptr log, std::shared_ptr 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. * See DD4hep Detector documentation */ - dd4hep::Detector *m_dd4hepDetector = nullptr; + const dd4hep::Detector* m_dd4hepDetector = nullptr; /// DD4hep surface map std::map 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). - * See DD4hep CellIDPositionConverter documentation - */ - std::shared_ptr m_cellid_converter = nullptr; - /// Acts magnetic field std::shared_ptr m_magneticField = nullptr; diff --git a/src/algorithms/tracking/DD4hepBField.h b/src/algorithms/tracking/DD4hepBField.h index 0627f0f7bd..9e2e193d1b 100644 --- a/src/algorithms/tracking/DD4hepBField.h +++ b/src/algorithms/tracking/DD4hepBField.h @@ -8,6 +8,7 @@ #pragma once +#include #include #include @@ -35,7 +36,7 @@ namespace eicrecon::BField { */ class DD4hepBField final : public Acts::MagneticFieldProvider { public: - dd4hep::Detector* m_det; + gsl::not_null m_det; public: struct Cache { @@ -51,7 +52,7 @@ namespace eicrecon::BField { * * @param [in] DD4hep detector instance */ - explicit DD4hepBField(dd4hep::Detector* det) : m_det(det) {} + explicit DD4hepBField(gsl::not_null det) : m_det(det) {} /** retrieve magnetic field value. * diff --git a/src/algorithms/tracking/TrackerHitReconstruction.cc b/src/algorithms/tracking/TrackerHitReconstruction.cc index 17839ded71..fab70dfabd 100644 --- a/src/algorithms/tracking/TrackerHitReconstruction.cc +++ b/src/algorithms/tracking/TrackerHitReconstruction.cc @@ -16,12 +16,11 @@ namespace { } } // namespace -void TrackerHitReconstruction::init(const dd4hep::Detector* detector, std::shared_ptr& logger) { +void TrackerHitReconstruction::init(const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr& logger) { m_log = logger; - // Create CellID converter - m_cellid_converter = std::make_shared(const_cast(*detector)); + m_converter = converter; } std::unique_ptr TrackerHitReconstruction::process(const edm4eic::RawTrackerHitCollection& raw_hits) { @@ -34,8 +33,8 @@ std::unique_ptr TrackerHitReconstruction::process auto id = raw_hit.getCellID(); // Get position and dimension - auto pos = m_cellid_converter->position(id); - auto dim = m_cellid_converter->cellDimensions(id); + auto pos = m_converter->position(id); + auto dim = m_converter->cellDimensions(id); // >oO trace if(m_log->level() == spdlog::level::trace) { diff --git a/src/algorithms/tracking/TrackerHitReconstruction.h b/src/algorithms/tracking/TrackerHitReconstruction.h index 0d9642dd97..0913bfa896 100644 --- a/src/algorithms/tracking/TrackerHitReconstruction.h +++ b/src/algorithms/tracking/TrackerHitReconstruction.h @@ -10,7 +10,6 @@ #include "TrackerHitReconstructionConfig.h" #include "algorithms/interfaces/WithPodConfig.h" -#include #include namespace eicrecon { @@ -22,7 +21,7 @@ namespace eicrecon { public: /// Once in a lifetime initialization - void init(const dd4hep::Detector *detector, std::shared_ptr& logger); + void init(const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr& logger); /// Processes RawTrackerHit and produces a TrackerHit std::unique_ptr process(const edm4eic::RawTrackerHitCollection& raw_hits); @@ -35,6 +34,6 @@ namespace eicrecon { std::shared_ptr m_log; /// Cell ID position converter - std::shared_ptr m_cellid_converter; + const dd4hep::rec::CellIDPositionConverter* m_converter; }; } diff --git a/src/algorithms/tracking/TrackerSourceLinker.cc b/src/algorithms/tracking/TrackerSourceLinker.cc index 31f208b7fa..91d67553d6 100644 --- a/src/algorithms/tracking/TrackerSourceLinker.cc +++ b/src/algorithms/tracking/TrackerSourceLinker.cc @@ -24,13 +24,14 @@ #include -void eicrecon::TrackerSourceLinker::init(std::shared_ptr cellid_converter, +void eicrecon::TrackerSourceLinker::init(const dd4hep::Detector* detector, + const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr acts_context, std::shared_ptr logger) { - m_cellid_converter = std::move(cellid_converter); + m_dd4hepGeo = detector; + m_converter = converter; m_log = std::move(logger); m_acts_context = std::move(acts_context); - m_dd4hepGeo = m_acts_context->dd4hepDetector(); m_detid_b0tracker = m_dd4hepGeo->constant("B0Tracker_Station_1_ID"); } @@ -57,7 +58,7 @@ eicrecon::TrackerSourceLinkerResult *eicrecon::TrackerSourceLinker::produce(std: cov(1, 1) = hit->getPositionError().yy * mm_acts * mm_acts; - const auto* vol_ctx = m_cellid_converter->findContext(hit->getCellID()); + const auto* vol_ctx = m_converter->findContext(hit->getCellID()); auto vol_id = vol_ctx->identifier; diff --git a/src/algorithms/tracking/TrackerSourceLinker.h b/src/algorithms/tracking/TrackerSourceLinker.h index ce66181e8e..cb0c1e2860 100644 --- a/src/algorithms/tracking/TrackerSourceLinker.h +++ b/src/algorithms/tracking/TrackerSourceLinker.h @@ -21,7 +21,8 @@ namespace eicrecon { class TrackerSourceLinker { public: - void init(std::shared_ptr cellid_converter, + void init(const dd4hep::Detector* detector, + const dd4hep::rec::CellIDPositionConverter* converter, std::shared_ptr acts_context, std::shared_ptr logger); @@ -30,13 +31,12 @@ namespace eicrecon { private: std::shared_ptr m_log; - /// Cell ID position converter - std::shared_ptr m_cellid_converter; + /// Geometry and Cell ID position converter + const dd4hep::Detector* m_dd4hepGeo; + const dd4hep::rec::CellIDPositionConverter* m_converter; std::shared_ptr m_acts_context; - dd4hep::Detector* m_dd4hepGeo; - /// Detector-specific information int m_detid_b0tracker; }; diff --git a/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.cc b/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.cc index cece01e7f0..69809b3977 100644 --- a/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.cc +++ b/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.cc @@ -16,7 +16,6 @@ #include #include -#include // Include appropriate class headers. e.g. #include @@ -207,8 +206,7 @@ void femc_studiesProcessor::Init() { } std::cout << __PRETTY_FUNCTION__ << " " << __LINE__ << std::endl; - dd4hep::Detector* detector = dd4hep_service->detector(); - dd4hep::rec::CellIDPositionConverter cellid_converter(*detector); + auto detector = dd4hep_service->detector(); std::cout << "--------------------------\nID specification:\n"; try { m_decoder = detector->readout("EcalEndcapPHits").idSpec().decoder(); diff --git a/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc b/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc index c6604ae39a..796c49bdae 100644 --- a/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc +++ b/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc @@ -16,7 +16,6 @@ #include #include -#include // Include appropriate class headers. e.g. #include @@ -243,8 +242,7 @@ void lfhcal_studiesProcessor::Init() { } std::cout << __PRETTY_FUNCTION__ << " " << __LINE__ << std::endl; - dd4hep::Detector* detector = dd4hep_service->detector(); - dd4hep::rec::CellIDPositionConverter cellid_converter(*detector); + auto detector = dd4hep_service->detector(); std::cout << "--------------------------\nID specification:\n"; try { m_decoder = detector->readout("LFHCALHits").idSpec().decoder(); diff --git a/src/factories/calorimetry/CalorimeterClusterRecoCoG_factoryT.h b/src/factories/calorimetry/CalorimeterClusterRecoCoG_factoryT.h index 143532fb00..371f353bce 100644 --- a/src/factories/calorimetry/CalorimeterClusterRecoCoG_factoryT.h +++ b/src/factories/calorimetry/CalorimeterClusterRecoCoG_factoryT.h @@ -42,7 +42,7 @@ class CalorimeterClusterRecoCoG_factoryT : // Use DD4hep_service to get dd4hep::Detector auto geoSvc = app->template GetService(); - m_detector = geoSvc->detector(); + auto detector = geoSvc->detector(); // SpdlogMixin logger initialization, sets m_log InitLogger(app, GetPrefix(), "info"); @@ -58,7 +58,7 @@ class CalorimeterClusterRecoCoG_factoryT : app->SetDefaultParameter(param_prefix + ":enableEtaBounds", cfg.enableEtaBounds); m_algo.applyConfig(cfg); - m_algo.init(m_detector, logger()); + m_algo.init(detector, logger()); } //------------------------------------------ @@ -81,7 +81,6 @@ class CalorimeterClusterRecoCoG_factoryT : } private: - const dd4hep::Detector* m_detector; eicrecon::CalorimeterClusterRecoCoG m_algo; }; diff --git a/src/factories/calorimetry/CalorimeterHitReco_factoryT.h b/src/factories/calorimetry/CalorimeterHitReco_factoryT.h index 49d5133d89..a714118c1b 100644 --- a/src/factories/calorimetry/CalorimeterHitReco_factoryT.h +++ b/src/factories/calorimetry/CalorimeterHitReco_factoryT.h @@ -60,7 +60,7 @@ class CalorimeterHitReco_factoryT : app->SetDefaultParameter(param_prefix + ":localDetFields", cfg.localDetFields); m_algo.applyConfig(cfg); - m_algo.init(geoSvc->detector(), logger()); + m_algo.init(geoSvc->detector(), geoSvc->converter(), logger()); } void Process(const std::shared_ptr &event) override { diff --git a/src/factories/calorimetry/CalorimeterHitsMerger_factoryT.h b/src/factories/calorimetry/CalorimeterHitsMerger_factoryT.h index 9e104139ba..d2f932d356 100644 --- a/src/factories/calorimetry/CalorimeterHitsMerger_factoryT.h +++ b/src/factories/calorimetry/CalorimeterHitsMerger_factoryT.h @@ -49,7 +49,7 @@ class CalorimeterHitsMerger_factoryT : app->SetDefaultParameter(param_prefix + ":refs", cfg.refs); m_algo.applyConfig(cfg); - m_algo.init(geoSvc->detector(), logger()); + m_algo.init(geoSvc->detector(), geoSvc->converter(), logger()); } void Process(const std::shared_ptr &event) override { diff --git a/src/factories/fardetectors/MatrixTransferStatic_factoryT.h b/src/factories/fardetectors/MatrixTransferStatic_factoryT.h index e500442e1f..fc1605efc9 100644 --- a/src/factories/fardetectors/MatrixTransferStatic_factoryT.h +++ b/src/factories/fardetectors/MatrixTransferStatic_factoryT.h @@ -46,7 +46,7 @@ namespace eicrecon { m_geoSvc = app->GetService(); m_reco_algo.applyConfig(cfg); - m_reco_algo.init(m_geoSvc->cellIDPositionConverter(),m_geoSvc->detector(),logger()); + m_reco_algo.init(m_geoSvc->detector(), m_geoSvc->converter(), logger()); } diff --git a/src/factories/tracking/TrackerHitReconstruction_factoryT.h b/src/factories/tracking/TrackerHitReconstruction_factoryT.h index 68327927e3..3a52504e98 100644 --- a/src/factories/tracking/TrackerHitReconstruction_factoryT.h +++ b/src/factories/tracking/TrackerHitReconstruction_factoryT.h @@ -48,7 +48,7 @@ namespace eicrecon { app->SetDefaultParameter(param_prefix + ":timeResolution", cfg.timeResolution); m_algo.applyConfig(cfg); - m_algo.init(geoSvc->detector(), logger()); + m_algo.init(geoSvc->converter(), logger()); } void Process(const std::shared_ptr &event) override { diff --git a/src/global/digi/PhotoMultiplierHitDigi_factory.cc b/src/global/digi/PhotoMultiplierHitDigi_factory.cc index f381a14702..6e14d2cb77 100644 --- a/src/global/digi/PhotoMultiplierHitDigi_factory.cc +++ b/src/global/digi/PhotoMultiplierHitDigi_factory.cc @@ -48,7 +48,7 @@ void eicrecon::PhotoMultiplierHitDigi_factory::Init() { // Initialize digitization algorithm m_digi_algo.applyConfig(cfg); - m_digi_algo.AlgorithmInit(geo_service->detector(), m_log); + m_digi_algo.AlgorithmInit(geo_service->detector(), geo_service->converter(), m_log); // Initialize richgeo ReadoutGeo and set random CellID visitor lambda (if a RICH) if(use_richgeo) { diff --git a/src/global/tracking/TrackerSourceLinker_factory.cc b/src/global/tracking/TrackerSourceLinker_factory.cc index 2784c9555c..4a85b1888b 100644 --- a/src/global/tracking/TrackerSourceLinker_factory.cc +++ b/src/global/tracking/TrackerSourceLinker_factory.cc @@ -39,11 +39,11 @@ namespace eicrecon { // Get ACTS context from ACTSGeo service auto acts_service = GetApplication()->GetService(); - auto dd4hp_service = GetApplication()->GetService(); + // Get DD4hep geometry from DD4hep service + auto dd4hep_service = GetApplication()->GetService(); // Initialize algorithm - auto cellid_converter = std::make_shared(*dd4hp_service->detector()); - m_source_linker.init(cellid_converter, acts_service->actsGeoProvider(), m_log); + m_source_linker.init(dd4hep_service->detector(), dd4hep_service->converter(), acts_service->actsGeoProvider(), m_log); } diff --git a/src/services/geometry/acts/ACTSGeo_service.h b/src/services/geometry/acts/ACTSGeo_service.h index b3ede02d34..37f066f3e1 100644 --- a/src/services/geometry/acts/ACTSGeo_service.h +++ b/src/services/geometry/acts/ACTSGeo_service.h @@ -28,7 +28,7 @@ class ACTSGeo_service : public JService std::once_flag m_init_flag; JApplication *m_app = nullptr; - dd4hep::Detector* m_dd4hepGeo = nullptr; + const dd4hep::Detector* m_dd4hepGeo = nullptr; std::shared_ptr m_acts_provider; // General acts log diff --git a/src/services/geometry/dd4hep/DD4hep_service.cc b/src/services/geometry/dd4hep/DD4hep_service.cc index 8a21e4625e..214fcc455d 100644 --- a/src/services/geometry/dd4hep/DD4hep_service.cc +++ b/src/services/geometry/dd4hep/DD4hep_service.cc @@ -1,6 +1,5 @@ -// Copyright 2022, David Lawrence -// Subject to the terms in the LICENSE file found in the top-level directory. -// +// SPDX-License-Identifier: LGPL-3.0-or-later +// Copyright (C) 2022, 2023 Whitney Armstrong, Wouter Deconinck, David Lawrence // #include @@ -30,21 +29,22 @@ DD4hep_service::~DD4hep_service(){ /// Return pointer to the dd4hep::Detector object. /// Call Initialize if needed. //---------------------------------------------------------------- -dd4hep::Detector* DD4hep_service::detector() { +gsl::not_null +DD4hep_service::detector() { std::call_once(init_flag, &DD4hep_service::Initialize, this); - return (m_dd4hepGeo); + return m_dd4hepGeo.get(); } //---------------------------------------------------------------- -// cellIDPositionConverter +// converter // /// Return pointer to the cellIDPositionConverter object. /// Call Initialize if needed. //---------------------------------------------------------------- -std::shared_ptr -DD4hep_service::cellIDPositionConverter() { +gsl::not_null +DD4hep_service::converter() { std::call_once(init_flag, &DD4hep_service::Initialize, this); - return (m_cellid_converter); + return m_cellid_converter.get(); } //---------------------------------------------------------------- @@ -57,12 +57,10 @@ DD4hep_service::cellIDPositionConverter() { //---------------------------------------------------------------- void DD4hep_service::Initialize() { - if( m_dd4hepGeo ) { + if (m_dd4hepGeo) { LOG_WARN(default_cout_logger) << "DD4hep_service already initialized!" << LOG_END; } - m_dd4hepGeo = &(dd4hep::Detector::getInstance()); - // The current recommended way of getting the XML file is to use the environment variables // DETECTOR_PATH and DETECTOR_CONFIG or DETECTOR(deprecated). // Look for those first, so we can use it for the default @@ -104,6 +102,7 @@ void DD4hep_service::Initialize() { app->SetTicker( false ); // load geometry + auto detector = dd4hep::Detector::make_unique(""); try { dd4hep::setPrintLevel(static_cast(print_level)); LOG << "Loading DD4hep geometry from " << m_xml_files.size() << " files" << LOG_END; @@ -113,17 +112,18 @@ void DD4hep_service::Initialize() { LOG << " - loading geometry file: '" << resolved_filename << "' (patience ....)" << LOG_END; try { - m_dd4hepGeo->fromCompact(resolved_filename); + detector->fromCompact(resolved_filename); } catch(std::runtime_error &e) { // dd4hep throws std::runtime_error, no way to detail further throw JException(e.what()); } } - m_dd4hepGeo->volumeManager(); - m_dd4hepGeo->apply("DD4hepVolumeManager", 0, nullptr); - m_cellid_converter = std::make_shared(*m_dd4hepGeo); + detector->volumeManager(); + detector->apply("DD4hepVolumeManager", 0, nullptr); + m_cellid_converter = std::make_unique(*detector); + m_dd4hepGeo = std::move(detector); // const LOG << "Geometry successfully loaded." << LOG_END; - }catch(std::exception &e){ + } catch(std::exception &e) { LOG_ERROR(default_cerr_logger)<< "Problem loading geometry: " << e.what() << LOG_END; throw std::runtime_error(fmt::format("Problem loading geometry: {}", e.what())); } diff --git a/src/services/geometry/dd4hep/DD4hep_service.h b/src/services/geometry/dd4hep/DD4hep_service.h index eeb2005e5f..89562c3366 100644 --- a/src/services/geometry/dd4hep/DD4hep_service.h +++ b/src/services/geometry/dd4hep/DD4hep_service.h @@ -1,25 +1,12 @@ -// -// -// TODO: Fix the following: -// This class inspired by and benefited from the work done here at the following -// links. A couple of lines were outright copied. -// -// https://eicweb.phy.anl.gov/EIC/juggler/-/blob/master/JugBase/src/components/GeoSvc.h -// https://eicweb.phy.anl.gov/EIC/juggler/-/blob/master/JugBase/src/components/GeoSvc.cpp -// -// Copyright carried by that code: // SPDX-License-Identifier: LGPL-3.0-or-later -// Copyright (C) 2022 Whitney Armstrong, Wouter Deconinck -// Copyright 2022, David Lawrence -// Subject to the terms in the LICENSE file found in the top-level directory. -// +// Copyright (C) 2022, 2023 Whitney Armstrong, Wouter Deconinck, David Lawrence // - #pragma once -#include +#include #include +#include #include #include @@ -33,8 +20,9 @@ class DD4hep_service : public JService DD4hep_service( JApplication *app ) : app(app) {} virtual ~DD4hep_service(); - virtual dd4hep::Detector* detector(); - virtual std::shared_ptr cellIDPositionConverter(); + virtual gsl::not_null detector(); + virtual gsl::not_null converter(); + protected: void Initialize(); @@ -43,8 +31,8 @@ class DD4hep_service : public JService std::once_flag init_flag; JApplication *app = nullptr; - dd4hep::Detector* m_dd4hepGeo = nullptr; - std::shared_ptr m_cellid_converter = nullptr; + std::unique_ptr m_dd4hepGeo = nullptr; + std::unique_ptr m_cellid_converter = nullptr; std::vector m_xml_files; /// Ensures there is a geometry file that should be opened diff --git a/src/services/geometry/richgeo/ActsGeo.cc b/src/services/geometry/richgeo/ActsGeo.cc index 67f96d3a25..97a0d90e69 100644 --- a/src/services/geometry/richgeo/ActsGeo.cc +++ b/src/services/geometry/richgeo/ActsGeo.cc @@ -9,7 +9,7 @@ #include // constructor -richgeo::ActsGeo::ActsGeo(std::string detName_, dd4hep::Detector *det_, std::shared_ptr log_) +richgeo::ActsGeo::ActsGeo(std::string detName_, gsl::not_null det_, std::shared_ptr log_) : m_detName(detName_), m_det(det_), m_log(log_) { // capitalize m_detName diff --git a/src/services/geometry/richgeo/ActsGeo.h b/src/services/geometry/richgeo/ActsGeo.h index a7402d811d..a1ad0b8395 100644 --- a/src/services/geometry/richgeo/ActsGeo.h +++ b/src/services/geometry/richgeo/ActsGeo.h @@ -4,9 +4,10 @@ // bind IRT and DD4hep geometries for the RICHes #pragma once -#include #include +#include #include +#include // DD4Hep #include @@ -28,7 +29,7 @@ namespace richgeo { public: // constructor and destructor - ActsGeo(std::string detName_, dd4hep::Detector *det_, std::shared_ptr log_); + ActsGeo(std::string detName_, gsl::not_null det_, std::shared_ptr log_); ~ActsGeo() {} // generate list ACTS disc surfaces, for a given radiator @@ -40,7 +41,7 @@ namespace richgeo { protected: std::string m_detName; - dd4hep::Detector *m_det; + gsl::not_null m_det; std::shared_ptr m_log; private: diff --git a/src/services/geometry/richgeo/IrtGeo.cc b/src/services/geometry/richgeo/IrtGeo.cc index 4a85b2c373..3e8ff9f882 100644 --- a/src/services/geometry/richgeo/IrtGeo.cc +++ b/src/services/geometry/richgeo/IrtGeo.cc @@ -4,8 +4,8 @@ #include "IrtGeo.h" // constructor: creates IRT-DD4hep bindings using main `Detector` handle `*det_` -richgeo::IrtGeo::IrtGeo(std::string detName_, dd4hep::Detector *det_, std::shared_ptr log_) : - m_detName(detName_), m_det(det_), m_log(log_) +richgeo::IrtGeo::IrtGeo(std::string detName_, gsl::not_null det_, gsl::not_null conv_, std::shared_ptr log_) : + m_detName(detName_), m_det(det_), m_converter(conv_), m_log(log_) { Bind(); } @@ -20,9 +20,6 @@ void richgeo::IrtGeo::Bind() { // IRT geometry handles m_irtDetectorCollection = new CherenkovDetectorCollection(); m_irtDetector = m_irtDetectorCollection->AddNewDetector(m_detName.c_str()); - - // cellID conversion - m_cellid_converter = std::make_shared(*m_det); } // ------------------------------------------------ @@ -32,13 +29,13 @@ void richgeo::IrtGeo::SetReadoutIDToPositionLambda() { m_irtDetector->m_ReadoutIDToPosition = [ &m_log = this->m_log, // capture logger by reference // capture instance members by value, so those owned by `this` are not mutable here - cell_mask = this->m_irtDetector->GetReadoutCellMask(), - cellid_converter = this->m_cellid_converter, - sensor_info = this->m_sensor_info + cell_mask = this->m_irtDetector->GetReadoutCellMask(), + converter = this->m_converter, + sensor_info = this->m_sensor_info ] (auto cell_id) { // decode cell ID to get the sensor ID and pixel volume centroid auto sensor_id = cell_id & cell_mask; - auto pixel_volume_centroid = (1/dd4hep::mm) * cellid_converter->position(cell_id); + auto pixel_volume_centroid = (1/dd4hep::mm) * converter->position(cell_id); // get sensor info auto sensor_info_it = sensor_info.find(sensor_id); if(sensor_info_it == sensor_info.end()) { diff --git a/src/services/geometry/richgeo/IrtGeo.h b/src/services/geometry/richgeo/IrtGeo.h index d120872dfd..adb03ca33c 100644 --- a/src/services/geometry/richgeo/IrtGeo.h +++ b/src/services/geometry/richgeo/IrtGeo.h @@ -13,6 +13,9 @@ #include #include +// GSL +#include + // IRT #include #include @@ -28,7 +31,7 @@ namespace richgeo { public: // constructor: creates IRT-DD4hep bindings using main `Detector` handle `*det_` - IrtGeo(std::string detName_, dd4hep::Detector *det_, std::shared_ptr log_); + IrtGeo(std::string detName_, gsl::not_null det_, gsl::not_null conv_, std::shared_ptr log_); virtual ~IrtGeo(); // access the full IRT geometry @@ -50,12 +53,12 @@ namespace richgeo { std::string m_detName; // DD4hep geometry handles - dd4hep::Detector *m_det; + gsl::not_null m_det; dd4hep::DetElement m_detRich; dd4hep::Position m_posRich; // cell ID conversion - std::shared_ptr m_cellid_converter; + gsl::not_null m_converter; std::unordered_map m_sensor_info; // sensor ID -> sensor info // IRT geometry handles diff --git a/src/services/geometry/richgeo/IrtGeoDRICH.h b/src/services/geometry/richgeo/IrtGeoDRICH.h index 9e29730297..b2c10e36b0 100644 --- a/src/services/geometry/richgeo/IrtGeoDRICH.h +++ b/src/services/geometry/richgeo/IrtGeoDRICH.h @@ -10,8 +10,8 @@ namespace richgeo { class IrtGeoDRICH : public IrtGeo { public: - IrtGeoDRICH(dd4hep::Detector *det_, std::shared_ptr log_) : - IrtGeo("DRICH",det_,log_) { DD4hep_to_IRT(); } + IrtGeoDRICH(gsl::not_null det_, gsl::not_null conv_, std::shared_ptr log_) : + IrtGeo("DRICH",det_,conv_,log_) { DD4hep_to_IRT(); } ~IrtGeoDRICH(); protected: diff --git a/src/services/geometry/richgeo/IrtGeoPFRICH.h b/src/services/geometry/richgeo/IrtGeoPFRICH.h index 5a3e99cca1..af0cf4e98c 100644 --- a/src/services/geometry/richgeo/IrtGeoPFRICH.h +++ b/src/services/geometry/richgeo/IrtGeoPFRICH.h @@ -10,8 +10,8 @@ namespace richgeo { class IrtGeoPFRICH : public IrtGeo { public: - IrtGeoPFRICH(dd4hep::Detector *det_, std::shared_ptr log_) : - IrtGeo("PFRICH",det_,log_) { DD4hep_to_IRT(); } + IrtGeoPFRICH(gsl::not_null det_, gsl::not_null conv_, std::shared_ptr log_) : + IrtGeo("PFRICH",det_,conv_,log_) { DD4hep_to_IRT(); } ~IrtGeoPFRICH(); protected: diff --git a/src/services/geometry/richgeo/ReadoutGeo.cc b/src/services/geometry/richgeo/ReadoutGeo.cc index c55ea109c8..f82b1dc29e 100644 --- a/src/services/geometry/richgeo/ReadoutGeo.cc +++ b/src/services/geometry/richgeo/ReadoutGeo.cc @@ -6,8 +6,8 @@ #include "ReadoutGeo.h" // constructor -richgeo::ReadoutGeo::ReadoutGeo(std::string detName_, dd4hep::Detector *det_, std::shared_ptr log_) - : m_detName(detName_), m_det(det_), m_log(log_) +richgeo::ReadoutGeo::ReadoutGeo(std::string detName_, gsl::not_null det_, gsl::not_null conv_, std::shared_ptr log_) + : m_detName(detName_), m_det(det_), m_conv(conv_), m_log(log_) { // capitalize m_detName std::transform(m_detName.begin(), m_detName.end(), m_detName.begin(), ::toupper); @@ -22,10 +22,9 @@ richgeo::ReadoutGeo::ReadoutGeo(std::string detName_, dd4hep::Detector *det_, st m_rngCellIDs = [] (std::function lambda, float p) { return; }; // common objects - m_readoutCoder = m_det->readout(m_detName+"Hits").idSpec().decoder(); - m_detRich = m_det->detector(m_detName); - m_systemID = m_detRich.id(); - m_cellid_converter = std::make_shared(*m_det); + m_readoutCoder = m_det->readout(m_detName+"Hits").idSpec().decoder(); + m_detRich = m_det->detector(m_detName); + m_systemID = m_detRich.id(); // dRICH readout -------------------------------------------------------------------- if(m_detName=="DRICH") { @@ -101,7 +100,7 @@ richgeo::ReadoutGeo::ReadoutGeo(std::string detName_, dd4hep::Detector *det_, st // pixel gap mask // FIXME: generalize; this assumes the segmentation is `CartesianGridXY` bool richgeo::ReadoutGeo::PixelGapMask(CellIDType cellID, dd4hep::Position pos_hit_global) { - auto pos_pixel_global = m_cellid_converter->position(cellID); + auto pos_pixel_global = m_conv->position(cellID); auto pos_pixel_local = GetSensorLocalPosition(cellID, pos_pixel_global); auto pos_hit_local = GetSensorLocalPosition(cellID, pos_hit_global); return ! ( @@ -116,7 +115,7 @@ bool richgeo::ReadoutGeo::PixelGapMask(CellIDType cellID, dd4hep::Position pos_h dd4hep::Position richgeo::ReadoutGeo::GetSensorLocalPosition(CellIDType cellID, dd4hep::Position pos) { // get the VolumeManagerContext for this sensitive detector - auto context = m_cellid_converter->findContext(cellID); + auto context = m_conv->findContext(cellID); // transformation vector buffers double xyz_l[3], xyz_e[3], xyz_g[3]; @@ -153,7 +152,7 @@ dd4hep::Position richgeo::ReadoutGeo::GetSensorLocalPosition(CellIDType cellID, print_pos("input position", pos); print_pos("sensor position", pos_sensor); print_pos("output position", pos_transformed); - // auto dim = m_cellid_converter->cellDimensions(cellID); + // auto dim = m_conv->cellDimensions(cellID); // for (std::size_t j = 0; j < std::size(dim); ++j) // m_log->trace(" - dimension {:<5} size: {:.2}", j, dim[j]); } diff --git a/src/services/geometry/richgeo/ReadoutGeo.h b/src/services/geometry/richgeo/ReadoutGeo.h index 5db918c0b4..c0799ffe56 100644 --- a/src/services/geometry/richgeo/ReadoutGeo.h +++ b/src/services/geometry/richgeo/ReadoutGeo.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -26,7 +27,7 @@ namespace richgeo { public: // constructor - ReadoutGeo(std::string detName_, dd4hep::Detector *det_, std::shared_ptr log_); + ReadoutGeo(std::string detName_, gsl::not_null det_, gsl::not_null conv_, std::shared_ptr log_); ~ReadoutGeo() {} // define cellID encoding @@ -66,10 +67,10 @@ namespace richgeo { // common objects std::shared_ptr m_log; std::string m_detName; - dd4hep::Detector* m_det; + gsl::not_null m_det; + gsl::not_null m_conv; dd4hep::DetElement m_detRich; dd4hep::BitFieldCoder* m_readoutCoder; - std::shared_ptr m_cellid_converter; int m_systemID; int m_num_sec; int m_num_pdus; diff --git a/src/services/geometry/richgeo/RichGeo_service.cc b/src/services/geometry/richgeo/RichGeo_service.cc index d806eaeb3f..3c16463959 100644 --- a/src/services/geometry/richgeo/RichGeo_service.cc +++ b/src/services/geometry/richgeo/RichGeo_service.cc @@ -17,6 +17,7 @@ void RichGeo_service::acquire_services(JServiceLocator *srv_locator) { // DD4Hep geometry service auto dd4hep_service = srv_locator->get(); m_dd4hepGeo = dd4hep_service->detector(); + m_converter = dd4hep_service->converter(); } // IrtGeo ----------------------------------------------------------- @@ -30,8 +31,8 @@ richgeo::IrtGeo *RichGeo_service::GetIrtGeo(std::string detector_name) { // instantiate IrtGeo-derived object, depending on detector auto which_rich = detector_name; std::transform(which_rich.begin(), which_rich.end(), which_rich.begin(), ::toupper); - if ( which_rich=="DRICH" ) m_irtGeo = new richgeo::IrtGeoDRICH(m_dd4hepGeo, m_log); - else if( which_rich=="PFRICH" ) m_irtGeo = new richgeo::IrtGeoPFRICH(m_dd4hepGeo, m_log); + if ( which_rich=="DRICH" ) m_irtGeo = new richgeo::IrtGeoDRICH(m_dd4hepGeo, m_converter, m_log); + else if( which_rich=="PFRICH" ) m_irtGeo = new richgeo::IrtGeoPFRICH(m_dd4hepGeo, m_converter, m_log); else throw JException(fmt::format("IrtGeo is not defined for detector '{}'",detector_name)); }; std::call_once(m_init_irt, initialize); @@ -67,7 +68,7 @@ std::shared_ptr RichGeo_service::GetReadoutGeo(std::string m_log->debug("Call RichGeo_service::GetReadoutGeo initializer"); auto initialize = [this,&detector_name] () { if(!m_dd4hepGeo) throw JException("RichGeo_service m_dd4hepGeo==null which should never be!"); - m_readoutGeo = std::make_shared(detector_name, m_dd4hepGeo, m_log); + m_readoutGeo = std::make_shared(detector_name, m_dd4hepGeo, m_converter, m_log); }; std::call_once(m_init_readout, initialize); } diff --git a/src/services/geometry/richgeo/RichGeo_service.h b/src/services/geometry/richgeo/RichGeo_service.h index 3b9ed4a726..ff31d63fc6 100644 --- a/src/services/geometry/richgeo/RichGeo_service.h +++ b/src/services/geometry/richgeo/RichGeo_service.h @@ -30,7 +30,7 @@ class RichGeo_service : public JService { virtual ~RichGeo_service(); // return pointer to the main DD4hep Detector - virtual dd4hep::Detector *GetDD4hepGeo() { return m_dd4hepGeo; }; + virtual const dd4hep::Detector* GetDD4hepGeo() { return m_dd4hepGeo; }; // return pointers to geometry bindings; initializes the bindings upon the first time called virtual richgeo::IrtGeo *GetIrtGeo(std::string detector_name); @@ -45,7 +45,8 @@ class RichGeo_service : public JService { std::once_flag m_init_acts; std::once_flag m_init_readout; JApplication *m_app = nullptr; - dd4hep::Detector *m_dd4hepGeo = nullptr; + const dd4hep::Detector* m_dd4hepGeo = nullptr; + const dd4hep::rec::CellIDPositionConverter* m_converter = nullptr; richgeo::IrtGeo *m_irtGeo = nullptr; richgeo::ActsGeo *m_actsGeo = nullptr; std::shared_ptr m_readoutGeo; diff --git a/src/tests/algorithms_test/calorimetry_CalorimeterHitDigi.cc b/src/tests/algorithms_test/calorimetry_CalorimeterHitDigi.cc index 24cb26309c..5477cb2525 100644 --- a/src/tests/algorithms_test/calorimetry_CalorimeterHitDigi.cc +++ b/src/tests/algorithms_test/calorimetry_CalorimeterHitDigi.cc @@ -6,6 +6,8 @@ #include #include +#include + #include "algorithms/calorimetry/CalorimeterHitDigi.h" using eicrecon::CalorimeterHitDigi; @@ -17,6 +19,8 @@ TEST_CASE( "the clustering algorithm runs", "[CalorimeterHitDigi]" ) { std::shared_ptr logger = spdlog::default_logger()->clone("CalorimeterHitDigi"); logger->set_level(spdlog::level::trace); + auto detector = dd4hep::Detector::make_unique(""); + CalorimeterHitDigiConfig cfg; cfg.threshold = 0. /* GeV */; cfg.corrMeanScale = 1.; @@ -32,7 +36,7 @@ TEST_CASE( "the clustering algorithm runs", "[CalorimeterHitDigi]" ) { cfg.pedMeanADC = 123; cfg.resolutionTDC = 1.0 * dd4hep::ns; algo.applyConfig(cfg); - algo.init(nullptr, logger); + algo.init(detector.get(), logger); edm4hep::CaloHitContributionCollection calohits; edm4hep::SimCalorimeterHitCollection simhits; diff --git a/src/tests/algorithms_test/calorimetry_CalorimeterIslandCluster.cc b/src/tests/algorithms_test/calorimetry_CalorimeterIslandCluster.cc index 9fdeee2d34..f532124595 100644 --- a/src/tests/algorithms_test/calorimetry_CalorimeterIslandCluster.cc +++ b/src/tests/algorithms_test/calorimetry_CalorimeterIslandCluster.cc @@ -7,6 +7,8 @@ #include #include +#include + #include "algorithms/calorimetry/CalorimeterIslandCluster.h" using eicrecon::CalorimeterIslandCluster; @@ -22,11 +24,13 @@ TEST_CASE( "the clustering algorithm runs", "[CalorimeterIslandCluster]" ) { cfg.minClusterHitEdep = 0. * dd4hep::GeV; cfg.minClusterCenterEdep = 0. * dd4hep::GeV; + auto detector = dd4hep::Detector::make_unique(""); + SECTION( "without splitting" ) { cfg.splitCluster = false; cfg.localDistXY = {1 * dd4hep::mm, 1 * dd4hep::mm}; algo.applyConfig(cfg); - algo.init(nullptr, logger); + algo.init(detector.get(), logger); SECTION( "on a single cell" ) { edm4eic::CalorimeterHitCollection hits_coll; @@ -129,7 +133,7 @@ TEST_CASE( "the clustering algorithm runs", "[CalorimeterIslandCluster]" ) { } cfg.localDistXY = {1 * dd4hep::mm, 1 * dd4hep::mm}; algo.applyConfig(cfg); - algo.init(nullptr, logger); + algo.init(detector.get(), logger); edm4eic::CalorimeterHitCollection hits_coll; hits_coll.create( From 6d65a87d051f419d46d94a3a7a922587e2964c02 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Fri, 6 Oct 2023 08:28:44 -0500 Subject: [PATCH 03/13] feat(ci): test eicmkplugin in CI (#1031) ### Briefly, what does this PR introduce? This adds a CI test for the eicmkplugin script (under the philosophy that if it's not tested, it is probably broken). ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [x] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --- .github/workflows/linux-eic-shell.yml | 33 +++++++++++++++++++++++++++ src/scripts/eicmkplugin.py | 7 +++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/.github/workflows/linux-eic-shell.yml b/.github/workflows/linux-eic-shell.yml index 694ec2d21c..9605b833f9 100644 --- a/.github/workflows/linux-eic-shell.yml +++ b/.github/workflows/linux-eic-shell.yml @@ -357,6 +357,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: diff --git a/src/scripts/eicmkplugin.py b/src/scripts/eicmkplugin.py index f60c882e78..97c7caf8f1 100644 --- a/src/scripts/eicmkplugin.py +++ b/src/scripts/eicmkplugin.py @@ -35,6 +35,7 @@ add_library({0}_plugin SHARED ${{{0}_PLUGIN_SOURCES}}) target_link_libraries({0}_plugin ${{JANA_LIB}} ${{ROOT_LIBRARIES}} spdlog::spdlog) set_target_properties({0}_plugin PROPERTIES PREFIX "" OUTPUT_NAME "{0}" SUFFIX ".so") +target_compile_definitions({0}_plugin PUBLIC HAVE_PODIO) # Install plugin USER_PLUGIN_OUTPUT_DIRECTORY is set depending on EICrecon_MY envar. install(TARGETS {0}_plugin DESTINATION ${{USER_PLUGIN_OUTPUT_DIRECTORY}} ) @@ -50,10 +51,6 @@ #include #include -// Include appropirate class headers. e.g. -// #include - - class {0}: public JEventProcessorSequentialRoot {{ private: @@ -78,6 +75,8 @@ class {0}: public JEventProcessorSequentialRoot {{ #include "{0}.h" #include "services/rootfile/RootFile_service.h" +// Include appropriate class headers. e.g. +#include // The following just makes this a JANA plugin extern "C" {{ From 087d115e97cfcc6cf12a9c286dcc4283c66b703b Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Fri, 6 Oct 2023 17:40:59 -0500 Subject: [PATCH 04/13] fix: reenable storing geometryId into TrackPoint surface field (#1055) ### Briefly, what does this PR introduce? This reenables the geometryId call to store the surface field of the TrackPoint. It was previously having issues with ASAN. ### What kind of change does this PR introduce? - [x] Bug fix (issue #930) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] Changes have been communicated to collaborators @veprbl ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --- src/algorithms/tracking/TrackProjector.cc | 2 +- src/algorithms/tracking/TrackPropagation.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/algorithms/tracking/TrackProjector.cc b/src/algorithms/tracking/TrackProjector.cc index cc152be803..eec3e5d0b9 100644 --- a/src/algorithms/tracking/TrackProjector.cc +++ b/src/algorithms/tracking/TrackProjector.cc @@ -148,7 +148,7 @@ namespace eicrecon { const float pathLength = static_cast(trackstate.pathLength()); const float pathLengthError = 0; - uint64_t surface = 0; // trackstate.referenceSurface().geometryId().value(); FIXME - ASAN is not happy with this + uint64_t surface = trackstate.referenceSurface().geometryId().value(); uint32_t system = 0; // Store track point diff --git a/src/algorithms/tracking/TrackPropagation.cc b/src/algorithms/tracking/TrackPropagation.cc index c363214523..7d78d6766b 100644 --- a/src/algorithms/tracking/TrackPropagation.cc +++ b/src/algorithms/tracking/TrackPropagation.cc @@ -292,7 +292,7 @@ namespace eicrecon { m_log->trace(" loc err = {:.4f}", static_cast(covariance(Acts::eBoundLoc0, Acts::eBoundLoc1))); #if EDM4EIC_VERSION_MAJOR >= 3 - uint64_t surface = 0; // targetSurf->geometryId().value(); // FIXME - ASAN is not happy with this + uint64_t surface = targetSurf->geometryId().value(); uint32_t system = 0; // default value...will be set in TrackPropagation factory #endif From 7163ee8ce3d2c4673d17723dec03e78318f136dc Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Fri, 6 Oct 2023 20:58:16 -0500 Subject: [PATCH 05/13] fix: make sure Microsoft.GSL::GSL is provided to plugins/libraries (#1059) ### Briefly, what does this PR introduce? Hotfix. Depending on Microsoft.GSL through just a top-level find_package(Microsoft.GSL CONFIG) doesn't work in the eic-shell nightlies, where GSL is in a completely different prefix, e.g. https://eicweb.phy.anl.gov/containers/eic_container/-/jobs/2013019. ### What kind of change does this PR introduce? - [x] Bug fix (issue: https://eicweb.phy.anl.gov/containers/eic_container/-/jobs/2013019) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --- cmake/jana_plugin.cmake | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmake/jana_plugin.cmake b/cmake/jana_plugin.cmake index 4de05ab5d5..2da32755df 100644 --- a/cmake/jana_plugin.cmake +++ b/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}) From 301734653a0ff507dc15ffa83500297d4c6f3092 Mon Sep 17 00:00:00 2001 From: Dmitry Kalinkin Date: Sat, 7 Oct 2023 15:12:16 -0400 Subject: [PATCH 06/13] TrackPropagation_factory: prefix Tag:LogLevel with plugin_name: in parameter name (#1049) Currently, we have `tracking:CalorimeterTrackPropagator:InputTags` and `tracking:CalorimeterTrackPropagator:OutputTags` as parameter name, but also `CalorimeterTrackPropagator:LogLevel`. This change brings the missing boilerplate that fixes the inconsistency. --- src/global/tracking/TrackPropagation_factory.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/global/tracking/TrackPropagation_factory.cc b/src/global/tracking/TrackPropagation_factory.cc index d0afd9c6e2..01b2686052 100644 --- a/src/global/tracking/TrackPropagation_factory.cc +++ b/src/global/tracking/TrackPropagation_factory.cc @@ -22,8 +22,12 @@ void eicrecon::TrackPropagation_factory::Init() { auto app = GetApplication(); - // SpdlogMixin logger initialization, sets m_log - InitLogger(app, GetTag()); + // This prefix will be used for parameters + std::string plugin_name = GetPluginName(); + std::string param_prefix = plugin_name+ ":" + GetTag(); + + // Initialize logger + InitLogger(app, param_prefix, "info"); auto acts_service = GetApplication()->GetService(); m_track_propagation_algo.init(acts_service->actsGeoProvider(), logger()); From 6bef773f4f840593f96477fa23dd622a1f876e81 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Sat, 7 Oct 2023 14:23:46 -0500 Subject: [PATCH 07/13] feat: add system and layer ID to Acts::Surface GeometryIdentifier (#1057) ### Briefly, what does this PR introduce? The Acts::Surface geometry identifier is intended to store information we are currently passing through two other lists in the TrackPropagation factory. This fills the geometry identifier field with: - the 'surface' as layer ID, - the 'detector' as extra ID (which is inteded for subsystem identifiers). These changes will allow us to get rid of the extra vectors that are passed to the algorithm, which in turn allows us to use the same algorithm for real and constructed surfaces. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --- .../tracking/TrackPropagation_factory.cc | 66 ++++++++++++------- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/src/global/tracking/TrackPropagation_factory.cc b/src/global/tracking/TrackPropagation_factory.cc index 01b2686052..231d9c16c1 100644 --- a/src/global/tracking/TrackPropagation_factory.cc +++ b/src/global/tracking/TrackPropagation_factory.cc @@ -82,12 +82,15 @@ void eicrecon::TrackPropagation_factory::SetPropagationSurfaces() { const double BEMC_halfz = (std::max(m_geoSvc->detector()->constant("EcalBarrelBackward_zmax"), m_geoSvc->detector()->constant("EcalBarrelForward_zmax")) / dd4hep::mm) * extend * Acts::UnitConstants::mm; auto BEMC_Trf = transform * Acts::Translation3(Acts::Vector3(0, 0, 0)); - auto m_BEMC_prop_surface1 = Acts::Surface::makeShared(BEMC_Trf, BEMC_R, BEMC_halfz); - auto m_BEMC_prop_surface2 = Acts::Surface::makeShared(BEMC_Trf, BEMC_R + ECAL_avgClusterDepth, BEMC_halfz); - m_target_surface_list.push_back(m_BEMC_prop_surface1); + auto BEMC_prop_surface1 = Acts::Surface::makeShared(BEMC_Trf, BEMC_R, BEMC_halfz); + auto BEMC_prop_surface2 = Acts::Surface::makeShared(BEMC_Trf, BEMC_R + ECAL_avgClusterDepth, BEMC_halfz); + auto BEMC_system_id = m_geoSvc->detector()->constant("ECalBarrel_ID"); + BEMC_prop_surface1->assignGeometryId(Acts::GeometryIdentifier().setExtra(BEMC_system_id).setLayer(1)); + BEMC_prop_surface2->assignGeometryId(Acts::GeometryIdentifier().setExtra(BEMC_system_id).setLayer(2)); + m_target_surface_list.push_back(BEMC_prop_surface1); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("ECalBarrel_ID")); m_target_surface_ID.push_back(1); - m_target_surface_list.push_back(m_BEMC_prop_surface2); + m_target_surface_list.push_back(BEMC_prop_surface2); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("ECalBarrel_ID")); m_target_surface_ID.push_back(2); @@ -98,12 +101,15 @@ void eicrecon::TrackPropagation_factory::SetPropagationSurfaces() { auto FEMC_Bounds = std::make_shared(FEMC_MinR, FEMC_MaxR); auto FEMC_Trf1 = transform * Acts::Translation3(Acts::Vector3(0, 0, FEMC_Z)); auto FEMC_Trf2 = transform * Acts::Translation3(Acts::Vector3(0, 0, FEMC_Z + ECAL_avgClusterDepth)); - auto m_FEMC_prop_surface1 = Acts::Surface::makeShared(FEMC_Trf1, FEMC_Bounds); - auto m_FEMC_prop_surface2 = Acts::Surface::makeShared(FEMC_Trf2, FEMC_Bounds); - m_target_surface_list.push_back(m_FEMC_prop_surface1); + auto FEMC_prop_surface1 = Acts::Surface::makeShared(FEMC_Trf1, FEMC_Bounds); + auto FEMC_prop_surface2 = Acts::Surface::makeShared(FEMC_Trf2, FEMC_Bounds); + auto FEMC_system_id = m_geoSvc->detector()->constant("ECalEndcapP_ID"); + FEMC_prop_surface1->assignGeometryId(Acts::GeometryIdentifier().setExtra(FEMC_system_id).setLayer(1)); + FEMC_prop_surface2->assignGeometryId(Acts::GeometryIdentifier().setExtra(FEMC_system_id).setLayer(2)); + m_target_surface_list.push_back(FEMC_prop_surface1); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("ECalEndcapP_ID")); m_target_surface_ID.push_back(1); - m_target_surface_list.push_back(m_FEMC_prop_surface2); + m_target_surface_list.push_back(FEMC_prop_surface2); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("ECalEndcapP_ID")); m_target_surface_ID.push_back(2); @@ -114,12 +120,15 @@ void eicrecon::TrackPropagation_factory::SetPropagationSurfaces() { auto EEMC_Bounds = std::make_shared(EEMC_MinR, EEMC_MaxR); auto EEMC_Trf1 = transform * Acts::Translation3(Acts::Vector3(0, 0, EEMC_Z)); auto EEMC_Trf2 = transform * Acts::Translation3(Acts::Vector3(0, 0, EEMC_Z - ECAL_avgClusterDepth)); - auto m_EEMC_prop_surface1 = Acts::Surface::makeShared(EEMC_Trf1, EEMC_Bounds); - auto m_EEMC_prop_surface2 = Acts::Surface::makeShared(EEMC_Trf2, EEMC_Bounds); - m_target_surface_list.push_back(m_EEMC_prop_surface1); + auto EEMC_prop_surface1 = Acts::Surface::makeShared(EEMC_Trf1, EEMC_Bounds); + auto EEMC_prop_surface2 = Acts::Surface::makeShared(EEMC_Trf2, EEMC_Bounds); + auto EEMC_system_id = m_geoSvc->detector()->constant("ECalEndcapN_ID"); + EEMC_prop_surface1->assignGeometryId(Acts::GeometryIdentifier().setExtra(EEMC_system_id).setLayer(1)); + EEMC_prop_surface2->assignGeometryId(Acts::GeometryIdentifier().setExtra(EEMC_system_id).setLayer(2)); + m_target_surface_list.push_back(EEMC_prop_surface1); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("ECalEndcapN_ID")); m_target_surface_ID.push_back(1); - m_target_surface_list.push_back(m_EEMC_prop_surface2); + m_target_surface_list.push_back(EEMC_prop_surface2); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("ECalEndcapN_ID")); m_target_surface_ID.push_back(2); @@ -128,12 +137,15 @@ void eicrecon::TrackPropagation_factory::SetPropagationSurfaces() { const double OHCAL_halfz = (std::max(m_geoSvc->detector()->constant("HcalBarrelBackward_zmax"), m_geoSvc->detector()->constant("HcalBarrelForward_zmax")) / dd4hep::mm) * extend * Acts::UnitConstants::mm; auto OHCAL_Trf = transform * Acts::Translation3(Acts::Vector3(0, 0, 0)); - auto m_OHCAL_prop_surface1 = Acts::Surface::makeShared(OHCAL_Trf, OHCAL_R, OHCAL_halfz); - auto m_OHCAL_prop_surface2 = Acts::Surface::makeShared(OHCAL_Trf, OHCAL_R + HCAL_avgClusterDepth, OHCAL_halfz); - m_target_surface_list.push_back(m_OHCAL_prop_surface1); + auto OHCAL_prop_surface1 = Acts::Surface::makeShared(OHCAL_Trf, OHCAL_R, OHCAL_halfz); + auto OHCAL_prop_surface2 = Acts::Surface::makeShared(OHCAL_Trf, OHCAL_R + HCAL_avgClusterDepth, OHCAL_halfz); + auto OHCAL_system_id = m_geoSvc->detector()->constant("HcalBarrel_ID"); + OHCAL_prop_surface1->assignGeometryId(Acts::GeometryIdentifier().setExtra(OHCAL_system_id).setLayer(1)); + OHCAL_prop_surface2->assignGeometryId(Acts::GeometryIdentifier().setExtra(OHCAL_system_id).setLayer(2)); + m_target_surface_list.push_back(OHCAL_prop_surface1); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("HCalBarrel_ID")); m_target_surface_ID.push_back(1); - m_target_surface_list.push_back(m_OHCAL_prop_surface2); + m_target_surface_list.push_back(OHCAL_prop_surface2); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("HCalBarrel_ID")); m_target_surface_ID.push_back(2); @@ -144,12 +156,15 @@ void eicrecon::TrackPropagation_factory::SetPropagationSurfaces() { auto LFHCAL_Bounds = std::make_shared(LFHCAL_MinR, LFHCAL_MaxR); auto LFHCAL_Trf1 = transform * Acts::Translation3(Acts::Vector3(0, 0, LFHCAL_Z)); auto LFHCAL_Trf2 = transform * Acts::Translation3(Acts::Vector3(0, 0, LFHCAL_Z + HCAL_avgClusterDepth)); - auto m_LFHCAL_prop_surface1 = Acts::Surface::makeShared(LFHCAL_Trf1, LFHCAL_Bounds); - auto m_LFHCAL_prop_surface2 = Acts::Surface::makeShared(LFHCAL_Trf2, LFHCAL_Bounds); - m_target_surface_list.push_back(m_LFHCAL_prop_surface1); + auto LFHCAL_prop_surface1 = Acts::Surface::makeShared(LFHCAL_Trf1, LFHCAL_Bounds); + auto LFHCAL_prop_surface2 = Acts::Surface::makeShared(LFHCAL_Trf2, LFHCAL_Bounds); + auto LFHCAL_system_id = m_geoSvc->detector()->constant("HCalEndcapN_ID"); + LFHCAL_prop_surface1->assignGeometryId(Acts::GeometryIdentifier().setExtra(LFHCAL_system_id).setLayer(1)); + LFHCAL_prop_surface2->assignGeometryId(Acts::GeometryIdentifier().setExtra(LFHCAL_system_id).setLayer(2)); + m_target_surface_list.push_back(LFHCAL_prop_surface1); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("HCalEndcapP_ID")); m_target_surface_ID.push_back(1); - m_target_surface_list.push_back(m_LFHCAL_prop_surface2); + m_target_surface_list.push_back(LFHCAL_prop_surface2); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("HCalEndcapP_ID")); m_target_surface_ID.push_back(2); @@ -160,12 +175,15 @@ void eicrecon::TrackPropagation_factory::SetPropagationSurfaces() { auto EHCAL_Bounds = std::make_shared(EHCAL_MinR, EHCAL_MaxR); auto EHCAL_Trf1 = transform * Acts::Translation3(Acts::Vector3(0, 0, EHCAL_Z)); auto EHCAL_Trf2 = transform * Acts::Translation3(Acts::Vector3(0, 0, EHCAL_Z - HCAL_avgClusterDepth)); - auto m_EHCAL_prop_surface1 = Acts::Surface::makeShared(EHCAL_Trf1, EHCAL_Bounds); - auto m_EHCAL_prop_surface2 = Acts::Surface::makeShared(EHCAL_Trf2, EHCAL_Bounds); - m_target_surface_list.push_back(m_EHCAL_prop_surface1); + auto EHCAL_prop_surface1 = Acts::Surface::makeShared(EHCAL_Trf1, EHCAL_Bounds); + auto EHCAL_prop_surface2 = Acts::Surface::makeShared(EHCAL_Trf2, EHCAL_Bounds); + auto EHCAL_system_id = m_geoSvc->detector()->constant("HCalEndcapN_ID"); + EHCAL_prop_surface1->assignGeometryId(Acts::GeometryIdentifier().setExtra(EHCAL_system_id).setLayer(1)); + EHCAL_prop_surface2->assignGeometryId(Acts::GeometryIdentifier().setExtra(EHCAL_system_id).setLayer(2)); + m_target_surface_list.push_back(EHCAL_prop_surface1); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("HCalEndcapN_ID")); m_target_surface_ID.push_back(1); - m_target_surface_list.push_back(m_EHCAL_prop_surface2); + m_target_surface_list.push_back(EHCAL_prop_surface2); m_target_detector_ID.push_back(m_geoSvc->detector()->constant("HCalEndcapN_ID")); m_target_surface_ID.push_back(2); From cea19eb8807f3a7a4e7a2e3070b4d8e04233ff44 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Sun, 8 Oct 2023 20:00:32 -0500 Subject: [PATCH 08/13] fix: correct number of arguments for fmt string in processors (#1060) ### Briefly, what does this PR introduce? In C++20 / fmt-10.1.0 the format string is constexpr and parsed on compilation time. That means that format string mismatches with the number of arguments are now compilation errors. This fixes (the only) two such cases. ### What kind of change does this PR introduce? - [x] Bug fix (issue: incomplete C++20 / fmt-10 support) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --- .../reconstruction/femc_studies/femc_studiesProcessor.cc | 2 +- .../reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.cc b/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.cc index 69809b3977..c3be688e0e 100644 --- a/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.cc +++ b/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.cc @@ -553,7 +553,7 @@ void femc_studiesProcessor::Process(const std::shared_ptr& event) if (enableTree){ t_fEMC_towers_N = (int)input_tower_recSav.size(); for (int iCell = 0; iCell < (int)input_tower_recSav.size(); iCell++){ - m_log->trace("{} \t {} \t {} \t {} \t {} \t {}", input_tower_recSav.at(iCell).cellIDx, input_tower_recSav.at(iCell).cellIDy , input_tower_recSav.at(iCell).energy, input_tower_recSav.at(iCell).tower_clusterIDA, input_tower_recSav.at(iCell).tower_clusterIDB ); + m_log->trace("{} \t {} \t {} \t {} \t {}", input_tower_recSav.at(iCell).cellIDx, input_tower_recSav.at(iCell).cellIDy , input_tower_recSav.at(iCell).energy, input_tower_recSav.at(iCell).tower_clusterIDA, input_tower_recSav.at(iCell).tower_clusterIDB ); t_fEMC_towers_cellE[iCell] = (float)input_tower_recSav.at(iCell).energy; t_fEMC_towers_cellT[iCell] = (float)input_tower_recSav.at(iCell).time; diff --git a/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc b/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc index 796c49bdae..7111094231 100644 --- a/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc +++ b/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.cc @@ -671,7 +671,7 @@ void lfhcal_studiesProcessor::Process(const std::shared_ptr& event if (enableTree){ t_lFHCal_towers_N = (int)input_tower_recSav.size(); for (int iCell = 0; iCell < (int)input_tower_recSav.size(); iCell++){ - m_log->trace("{} \t {} \t {} \t {} \t {} \t {} \t {}", input_tower_recSav.at(iCell).cellIDx, input_tower_recSav.at(iCell).cellIDy, input_tower_recSav.at(iCell).cellIDz , input_tower_recSav.at(iCell).energy, input_tower_recSav.at(iCell).tower_clusterIDA, input_tower_recSav.at(iCell).tower_clusterIDB ); + m_log->trace("{} \t {} \t {} \t {} \t {} \t {}", input_tower_recSav.at(iCell).cellIDx, input_tower_recSav.at(iCell).cellIDy, input_tower_recSav.at(iCell).cellIDz , input_tower_recSav.at(iCell).energy, input_tower_recSav.at(iCell).tower_clusterIDA, input_tower_recSav.at(iCell).tower_clusterIDB ); t_lFHCal_towers_cellE[iCell] = (float)input_tower_recSav.at(iCell).energy; t_lFHCal_towers_cellT[iCell] = (float)input_tower_recSav.at(iCell).time; From 9ade2daac3d481fa156871011bac32fbadcdde3e Mon Sep 17 00:00:00 2001 From: Dmitry Kalinkin Date: Mon, 9 Oct 2023 14:33:26 -0400 Subject: [PATCH 09/13] TrackPropagation_factory: HcalEndcapN_* is not defined anymore, use LFHCAL (#1061) https://github.com/eic/EICrecon/actions/runs/6421965316/job/17439067008?pr=1053#step:5:577 as discovered in https://github.com/eic/EICrecon/pull/1053 This fixes track propagations. --- src/global/tracking/TrackPropagation_factory.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/global/tracking/TrackPropagation_factory.cc b/src/global/tracking/TrackPropagation_factory.cc index 231d9c16c1..74a787d9a6 100644 --- a/src/global/tracking/TrackPropagation_factory.cc +++ b/src/global/tracking/TrackPropagation_factory.cc @@ -150,9 +150,9 @@ void eicrecon::TrackPropagation_factory::SetPropagationSurfaces() { m_target_surface_ID.push_back(2); // Create propagation surface for LFHCAL - const double LFHCAL_Z = (m_geoSvc->detector()->constant("HcalEndcapP_zmin") / dd4hep::mm) * Acts::UnitConstants::mm; + const double LFHCAL_Z = (m_geoSvc->detector()->constant("LFHCAL_zmin") / dd4hep::mm) * Acts::UnitConstants::mm; const double LFHCAL_MinR = 0.0; - const double LFHCAL_MaxR = (m_geoSvc->detector()->constant("HcalEndcapP_rmax") / dd4hep::mm) * extend * Acts::UnitConstants::mm; + const double LFHCAL_MaxR = (m_geoSvc->detector()->constant("LFHCAL_rmax") / dd4hep::mm) * extend * Acts::UnitConstants::mm; auto LFHCAL_Bounds = std::make_shared(LFHCAL_MinR, LFHCAL_MaxR); auto LFHCAL_Trf1 = transform * Acts::Translation3(Acts::Vector3(0, 0, LFHCAL_Z)); auto LFHCAL_Trf2 = transform * Acts::Translation3(Acts::Vector3(0, 0, LFHCAL_Z + HCAL_avgClusterDepth)); From a7665de1c75c82f356d809909e81add11dab44cc Mon Sep 17 00:00:00 2001 From: Dmitry Kalinkin Date: Mon, 9 Oct 2023 14:33:46 -0400 Subject: [PATCH 10/13] Remove factories for HcalEndcapP (#1062) That detector was removed from the geometry configurations long time ago. cc #1053 --- src/detectors/FHCAL/FHCAL.cc | 101 +----------------- src/services/io/podio/JEventProcessorPODIO.cc | 7 -- 2 files changed, 2 insertions(+), 106 deletions(-) diff --git a/src/detectors/FHCAL/FHCAL.cc b/src/detectors/FHCAL/FHCAL.cc index 63d7ef3da0..5f8c315ff1 100644 --- a/src/detectors/FHCAL/FHCAL.cc +++ b/src/detectors/FHCAL/FHCAL.cc @@ -1,7 +1,5 @@ -// Copyright 2022, David Lawrence -// Subject to the terms in the LICENSE file found in the top-level directory. -// -// +// SPDX-License-Identifier: LGPL-3.0-or-later +// Copyright (C) 2023 Friederike Bock, Wouter Deconinck #include "extensions/jana/JChainMultifactoryGeneratorT.h" @@ -19,101 +17,6 @@ extern "C" { InitJANAPlugin(app); - app->Add(new JChainMultifactoryGeneratorT( - "HcalEndcapPRawHits", {"HcalEndcapPHits"}, {"HcalEndcapPRawHits"}, - { - .eRes = {}, - .tRes = 0.001 * dd4hep::ns, - .capADC = 65536, - .dyRangeADC = 1 * dd4hep::GeV, - .pedMeanADC = 20, - .pedSigmaADC = 0.8, - .resolutionTDC = 10 * dd4hep::picosecond, - .corrMeanScale = 1.0, - }, - app // TODO: Remove me once fixed - )); - app->Add(new JChainMultifactoryGeneratorT( - "HcalEndcapPRecHits", {"HcalEndcapPRawHits"}, {"HcalEndcapPRecHits"}, - { - .capADC = 65536, - .dyRangeADC = 1 * dd4hep::GeV, - .pedMeanADC = 20, - .pedSigmaADC = 0.8, - .resolutionTDC = 10 * dd4hep::picosecond, - .thresholdFactor = 1.0, - .thresholdValue = 3.0, - .sampFrac = 0.033, - .readout = "HcalEndcapPHits", - }, - app // TODO: Remove me once fixed - )); - app->Add(new JChainMultifactoryGeneratorT( - "HcalEndcapPMergedHits", {"HcalEndcapPRecHits"}, {"HcalEndcapPMergedHits"}, - { - .readout = "HcalEndcapPHits", - .fields = {"layer", "slice"}, - .refs = {1, 0}, - }, - app // TODO: Remove me once fixed - )); - app->Add(new JChainMultifactoryGeneratorT( - "HcalEndcapPTruthProtoClusters", {"HcalEndcapPRecHits", "HcalEndcapPHits"}, {"HcalEndcapPTruthProtoClusters"}, - app // TODO: Remove me once fixed - )); - app->Add(new JChainMultifactoryGeneratorT( - "HcalEndcapPIslandProtoClusters", {"HcalEndcapPRecHits"}, {"HcalEndcapPIslandProtoClusters"}, - { - .sectorDist = 5.0 * dd4hep::cm, - .localDistXY = {15.0*dd4hep::mm, 15.0*dd4hep::mm}, - .dimScaledLocalDistXY = {15.0*dd4hep::mm, 15.0*dd4hep::mm}, - .splitCluster = true, - .minClusterHitEdep = 0.0 * dd4hep::MeV, - .minClusterCenterEdep = 30.0 * dd4hep::MeV, - .transverseEnergyProfileMetric = "globalDistEtaPhi", - .transverseEnergyProfileScale = 1., - }, - app // TODO: Remove me once fixed - )); - - app->Add( - new JChainMultifactoryGeneratorT( - "HcalEndcapPTruthClusters", - {"HcalEndcapPTruthProtoClusters", // edm4eic::ProtoClusterCollection - "HcalEndcapPHits"}, // edm4hep::SimCalorimeterHitCollection - {"HcalEndcapPTruthClusters", // edm4eic::Cluster - "HcalEndcapPTruthClusterAssociations"}, // edm4eic::MCRecoClusterParticleAssociation - { - .energyWeight = "log", - .moduleDimZName = "", - .sampFrac = 1.0, - .logWeightBase = 3.6, - .depthCorrection = 0.0, - .enableEtaBounds = false - }, - app // TODO: Remove me once fixed - ) - ); - - app->Add( - new JChainMultifactoryGeneratorT( - "HcalEndcapPClusters", - {"HcalEndcapPIslandProtoClusters", // edm4eic::ProtoClusterCollection - "HcalEndcapPHits"}, // edm4hep::SimCalorimeterHitCollection - {"HcalEndcapPClusters", // edm4eic::Cluster - "HcalEndcapPClusterAssociations"}, // edm4eic::MCRecoClusterParticleAssociation - { - .energyWeight = "log", - .moduleDimZName = "", - .sampFrac = 0.033, - .logWeightBase = 6.2, - .depthCorrection = 0.0, - .enableEtaBounds = false, - }, - app // TODO: Remove me once fixed - ) - ); - app->Add(new JChainMultifactoryGeneratorT( "HcalEndcapPInsertRawHits", {"HcalEndcapPInsertHits"}, {"HcalEndcapPInsertRawHits"}, { diff --git a/src/services/io/podio/JEventProcessorPODIO.cc b/src/services/io/podio/JEventProcessorPODIO.cc index a3d5bc9795..b58540e2a2 100644 --- a/src/services/io/podio/JEventProcessorPODIO.cc +++ b/src/services/io/podio/JEventProcessorPODIO.cc @@ -152,13 +152,6 @@ JEventProcessorPODIO::JEventProcessorPODIO() { "HcalEndcapNMergedHits", "HcalEndcapNClusters", "HcalEndcapNClusterAssociations", - "HcalEndcapPRawHits", // this causes premature exit of eicrecon - "HcalEndcapPRecHits", - "HcalEndcapPMergedHits", - "HcalEndcapPTruthClusters", - "HcalEndcapPTruthClusterAssociations", - "HcalEndcapPClusters", - "HcalEndcapPClusterAssociations", "HcalEndcapPInsertRawHits", "HcalEndcapPInsertRecHits", "HcalEndcapPInsertMergedHits", From 5b14144763e134b23c32f4491ea48ee57f8b08a7 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Tue, 10 Oct 2023 11:12:58 -0500 Subject: [PATCH 11/13] feat: enable ubsan by default in CI, with print_stacktrace=1 and suppressions (#987) This enables ubsan by default in CI, with some more undefined behaviors (float-divide-by-zero, unsigned-integer-overflow, local-bounds). It has two suppressions already, but more will no doubt be needed to limit the output to what we control. There is no way to have an exitcode != 0 when undefined behavior is found, so this depends a bit on people checking the output periodically. TODO: - [x] #989 --------- Co-authored-by: Dmitry Kalinkin --- .github/ubsan.supp | 3 +++ .github/workflows/linux-eic-shell.yml | 3 ++- CMakeLists.txt | 5 +++-- 3 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 .github/ubsan.supp diff --git a/.github/ubsan.supp b/.github/ubsan.supp new file mode 100644 index 0000000000..94a63e2c4c --- /dev/null +++ b/.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 diff --git a/.github/workflows/linux-eic-shell.yml b/.github/workflows/linux-eic-shell.yml index 9605b833f9..014254a591 100644 --- a/.github/workflows/linux-eic-shell.yml +++ b/.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 diff --git a/CMakeLists.txt b/CMakeLists.txt index e6a597b685..ae60849bfe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -174,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 From 53aaa9a34f65e0734d8d2e40d0f861a9c7bcbf06 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Tue, 10 Oct 2023 13:03:40 -0500 Subject: [PATCH 12/13] fix: remove unused TrackPropagation header with indirect dd4hep::BitFieldCoder definition (#1066) ### Briefly, what does this PR introduce? Minor fix that at some point iwyu will be able to catch on its own... but for now requires manual intervention. ### What kind of change does this PR introduce? - [x] Bug fix (issue: unused include) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --- .../reconstruction/femc_studies/femc_studiesProcessor.h | 4 +--- .../reconstruction/lfhcal_studies/lfhcal_studiesProcessor.h | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.h b/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.h index 457c4830d6..3f2e989f41 100644 --- a/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.h +++ b/src/benchmarks/reconstruction/femc_studies/femc_studiesProcessor.h @@ -12,8 +12,6 @@ #include #include -#include "extensions/spdlog/SpdlogMixin.h" -#include "algorithms/tracking/TrackPropagation.h" #include class femc_studiesProcessor: public JEventProcessor { @@ -78,7 +76,7 @@ class femc_studiesProcessor: public JEventProcessor { int nEventsWithCaloHits = 0; std::shared_ptr m_log; - dd4hep::BitFieldCoder* m_decoder; + dd4hep::DDSegmentation::BitFieldCoder* m_decoder; std::string nameSimHits = "EcalEndcapPHits"; std::string nameRecHits = "EcalEndcapPRecHits"; std::string nameClusters = "EcalEndcapPClusters"; diff --git a/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.h b/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.h index c2449232f3..e95d606f31 100644 --- a/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.h +++ b/src/benchmarks/reconstruction/lfhcal_studies/lfhcal_studiesProcessor.h @@ -12,8 +12,6 @@ #include #include -#include "extensions/spdlog/SpdlogMixin.h" -#include "algorithms/tracking/TrackPropagation.h" #include class lfhcal_studiesProcessor: public JEventProcessor { @@ -95,7 +93,7 @@ class lfhcal_studiesProcessor: public JEventProcessor { int nEventsWithCaloHits = 0; bool isLFHCal = true; std::shared_ptr m_log; - dd4hep::BitFieldCoder* m_decoder; + dd4hep::DDSegmentation::BitFieldCoder* m_decoder; std::string nameSimHits = "LFHCALHits"; std::string nameRecHits = "LFHCALRecHits"; std::string nameClusters = "LFHCALClusters"; From 546ea2073b4a09b55f0865bd6487e1378bbc00c6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 10 Oct 2023 15:14:45 -0400 Subject: [PATCH 13/13] [pre-commit.ci] pre-commit autoupdate (#1063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](https://github.com/pre-commit/pre-commit-hooks/compare/v4.4.0...v4.5.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 759c334963..392891a807 100644 --- a/.pre-commit-config.yaml +++ b/.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