feat: Logger passed to Gbts and Orthogonal seeders (acts-project#3807)

Passing logger also to the GBTS and Orthogonal seeders

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

- **New Features**
	- Enhanced logging capabilities across various classes, including `GbtsTrackingFilter`, `SeedFinderGbts`, and `SeedFinderOrthogonal`, allowing for better tracking and debugging.
	- Introduced logger parameters in constructors for `SeedFinderGbts`, `SeedFinderOrthogonal`, and `SeedingOrthogonalAlgorithm`, improving configurability.

- **Bug Fixes**
	- Improved error handling and logging for negative covariance values in `GbtsTrackingFilter`.
	- Adjusted warning messages in `GbtsSeedingAlgorithm` for better clarity.

- **Refactor**
	- Updated member variable types to use smart pointers for better memory management in `SeedFinderGbts` and `SeedFinderOrthogonal`.
	- Streamlined initialization processes in `SeedingOrthogonalAlgorithm` and `GbtsSeedingAlgorithm` to enhance code clarity and efficiency.
	- Renamed logger instances for consistency across `SeedFilter` and `SeedFinder`.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Core/include/Acts/Seeding/GbtsTrackingFilter.hpp

@@ -10,6 +10,7 @@
 
 // TODO: update to C++17 style
 #include "Acts/Seeding/GbtsDataStorage.hpp"  //includes geo which has trigindetsilayer, may move this to trigbase
+#include "Acts/Utilities/Logger.hpp"
 
 #include <algorithm>
 #include <cmath>
@@ -112,8 +113,11 @@ template <typename external_spacepoint_t>
 class GbtsTrackingFilter {
  public:
   GbtsTrackingFilter(const std::vector<Acts::TrigInDetSiLayer>& g,
-                     std::vector<Acts::GbtsEdge<external_spacepoint_t>>& sb)
-      : m_geo(g), m_segStore(sb) {}
+                     std::vector<Acts::GbtsEdge<external_spacepoint_t>>& sb,
+                     std::unique_ptr<const Acts::Logger> logger =
+                         Acts::getDefaultLogger("Filter",
+                                                Acts::Logging::Level::INFO))
+      : m_geo(g), m_segStore(sb), m_logger(std::move(logger)) {}
 
   void followTrack(Acts::GbtsEdge<external_spacepoint_t>* pS,
                    GbtsEdgeState<external_spacepoint_t>& output) {
@@ -233,11 +237,15 @@ class GbtsTrackingFilter {
     const float add_hit = 14.0;
 
     if (ts.m_Cx[2][2] < 0.0 || ts.m_Cx[1][1] < 0.0 || ts.m_Cx[0][0] < 0.0) {
-      std::cout << "Negative cov_x" << std::endl;
+      ACTS_WARNING("Negative covariance detected in X components: "
+                   << "cov[2][2]=" << ts.m_Cx[2][2] << " cov[1][1]="
+                   << ts.m_Cx[1][1] << " cov[0][0]=" << ts.m_Cx[0][0]);
     }
 
     if (ts.m_Cy[1][1] < 0.0 || ts.m_Cy[0][0] < 0.0) {
-      std::cout << "Negative cov_y" << std::endl;
+      ACTS_WARNING("Negative covariance detected in Y components: "
+                   << "cov[1][1]=" << ts.m_Cy[1][1]
+                   << " cov[0][0]=" << ts.m_Cy[0][0]);
     }
 
     // add ms.
@@ -380,4 +388,7 @@ class GbtsTrackingFilter {
   GbtsEdgeState<external_spacepoint_t> m_stateStore[MAX_EDGE_STATE];
 
   int m_globalStateCounter{0};
+
+  const Acts::Logger& logger() const { return *m_logger; }
+  std::unique_ptr<const Acts::Logger> m_logger{nullptr};
 };

Core/include/Acts/Seeding/SeedFilter.hpp

@@ -107,7 +107,7 @@ class SeedFilter final {
 
   const SeedFilterConfig m_cfg;
   std::unique_ptr<const Acts::Logger> m_logger =
-      Acts::getDefaultLogger("SeedFilter", Logging::Level::INFO);
+      Acts::getDefaultLogger("Filter", Logging::Level::INFO);
   const IExperimentCuts<external_spacepoint_t>* m_experimentCuts;
 };
 }  // namespace Acts

Core/include/Acts/Seeding/SeedFinder.hpp

@@ -91,7 +91,7 @@ class SeedFinder {
   /// @param logger the ACTS logger
   SeedFinder(const Acts::SeedFinderConfig<external_spacepoint_t>& config,
              std::unique_ptr<const Acts::Logger> logger =
-                 getDefaultLogger("SeedFinder", Logging::Level::INFO));
+                 getDefaultLogger("Finder", Logging::Level::INFO));
   SeedFinder(SeedFinder<external_spacepoint_t, grid_t, platform_t>&&) noexcept =
       default;
   SeedFinder& operator=(SeedFinder<external_spacepoint_t, grid_t,

Core/include/Acts/Seeding/SeedFinderGbts.hpp

@@ -15,7 +15,7 @@
 #include "Acts/Seeding/SeedFinderConfig.hpp"
 #include "Acts/Seeding/SeedFinderGbtsConfig.hpp"
 #include "Acts/TrackFinding/RoiDescriptor.hpp"
-#include "Acts/Utilities/KDTree.hpp"
+#include "Acts/Utilities/Logger.hpp"
 
 #include <array>
 #include <iostream>
@@ -46,14 +46,15 @@ class SeedFinderGbts {
   static constexpr std::size_t NDims = 3;
 
   using seed_t = Seed<external_spacepoint_t>;
-  //   using internal_sp_t = InternalSpacePoint<external_spacepoint_t>;
-  //   using tree_t = KDTree<NDims, internal_sp_t *, ActsScalar, std::array, 4>;
 
   // constructors
   SeedFinderGbts(const SeedFinderGbtsConfig<external_spacepoint_t> &config,
-                 const GbtsGeometry<external_spacepoint_t> &gbtsgeo);
+                 const GbtsGeometry<external_spacepoint_t> &gbtsgeo,
+                 std::unique_ptr<const Acts::Logger> logger =
+                     Acts::getDefaultLogger("Finder",
+                                            Acts::Logging::Level::INFO));
 
-  ~SeedFinderGbts();  //!!! is it dangerous not to use default? got def in ipp
+  ~SeedFinderGbts() = default;
   SeedFinderGbts() = default;
   SeedFinderGbts(const SeedFinderGbts<external_spacepoint_t> &) = delete;
   SeedFinderGbts<external_spacepoint_t> &operator=(
@@ -85,10 +86,14 @@ class SeedFinderGbts {
       const Acts::GbtsGeometry<external_spacepoint_t> &gbtsgeo);
 
   // needs to be member of class so can accessed by all member functions
-  GbtsDataStorage<external_spacepoint_t> *m_storage;
+  std::unique_ptr<GbtsDataStorage<external_spacepoint_t>> m_storage{nullptr};
 
   // for create seeds:
   std::vector<TrigInDetTriplet<external_spacepoint_t>> m_triplets;
+
+  const Acts::Logger &logger() const { return *m_logger; }
+  std::unique_ptr<const Acts::Logger> m_logger =
+      Acts::getDefaultLogger("Finder", Acts::Logging::Level::INFO);
 };
 
 }  // namespace Acts

Core/include/Acts/Seeding/SeedFinderGbts.ipp

@@ -34,22 +34,18 @@ namespace Acts {
 template <typename external_spacepoint_t>
 SeedFinderGbts<external_spacepoint_t>::SeedFinderGbts(
     const SeedFinderGbtsConfig<external_spacepoint_t>& config,
-    const GbtsGeometry<external_spacepoint_t>& gbtsGeo)
-    : m_config(config) {
-  m_storage = new GbtsDataStorage(gbtsGeo);
-}
-
-template <typename external_spacepoint_t>
-SeedFinderGbts<external_spacepoint_t>::~SeedFinderGbts() {
-  delete m_storage;
-
-  m_storage = nullptr;
-}
+    const GbtsGeometry<external_spacepoint_t>& gbtsGeo,
+    std::unique_ptr<const Acts::Logger> logger)
+    : m_config(config),
+      m_storage(
+          std::make_unique<GbtsDataStorage<external_spacepoint_t>>(gbtsGeo)),
+      m_logger(std::move(logger)) {}
 
 // define loadspace points function
 template <typename external_spacepoint_t>
 void SeedFinderGbts<external_spacepoint_t>::loadSpacePoints(
     const std::vector<GbtsSP<external_spacepoint_t>>& gbtsSPvect) {
+  ACTS_VERBOSE("Loading space points");
   for (const auto& gbtssp : gbtsSPvect) {
     bool is_Pixel = gbtssp.isPixel();
     if (!is_Pixel) {
@@ -70,6 +66,7 @@ void SeedFinderGbts<external_spacepoint_t>::runGbts_TrackFinder(
     std::vector<GbtsTrigTracklet<external_spacepoint_t>>& vTracks,
     const Acts::RoiDescriptor& roi,
     const Acts::GbtsGeometry<external_spacepoint_t>& gbtsGeo) {
+  ACTS_VERBOSE("Running GBTS Track Finder");
   const float min_z0 = roi.zedMinus();
   const float max_z0 = roi.zedPlus();
   const float cut_zMinU = min_z0 + m_config.maxOuterRadius * roi.dzdrMinus();
@@ -328,6 +325,7 @@ void SeedFinderGbts<external_spacepoint_t>::runGbts_TrackFinder(
   m_storage->getConnectingNodes(vNodes);
 
   if (vNodes.empty()) {
+    ACTS_VERBOSE("No nodes");
     return;
   }
 
@@ -506,8 +504,9 @@ void SeedFinderGbts<external_spacepoint_t>::runGbts_TrackFinder(
 
   // backtracking
 
-  GbtsTrackingFilter<external_spacepoint_t> tFilter(m_config.m_layerGeometry,
-                                                    edgeStorage);
+  GbtsTrackingFilter<external_spacepoint_t> tFilter(
+      m_config.m_layerGeometry, edgeStorage,
+      logger().cloneWithSuffix("GbtsFilter"));
 
   for (auto pS : vSeeds) {
     if (pS->m_level == -1) {
@@ -651,6 +650,7 @@ void SeedFinderGbts<external_spacepoint_t>::createSeeds(
     const Acts::RoiDescriptor& roi,
     const Acts::GbtsGeometry<external_spacepoint_t>& gbtsGeo,
     output_container_t& out_cont) {
+  ACTS_VERBOSE("Creating seeds");
   std::vector<GbtsTrigTracklet<external_spacepoint_t>>
       vTracks;  // make empty vector
 

Core/include/Acts/Seeding/SeedFinderOrthogonal.hpp

@@ -13,6 +13,7 @@
 #include "Acts/Seeding/SeedFinderConfig.hpp"
 #include "Acts/Seeding/SeedFinderOrthogonalConfig.hpp"
 #include "Acts/Utilities/KDTree.hpp"
+#include "Acts/Utilities/Logger.hpp"
 
 #include <array>
 #include <iostream>
@@ -53,20 +54,26 @@ class SeedFinderOrthogonal {
    * @brief Construct a new orthogonal seed finder.
    *
    * @param config The configuration parameters for this seed finder.
+   * @param logger The ACTS logger.
    */
   SeedFinderOrthogonal(
-      const Acts::SeedFinderOrthogonalConfig<external_spacepoint_t> &config);
-
+      const Acts::SeedFinderOrthogonalConfig<external_spacepoint_t> &config,
+      std::unique_ptr<const Acts::Logger> logger =
+          Acts::getDefaultLogger("Finder", Logging::Level::INFO));
   /**
    * @brief Destroy the orthogonal seed finder object.
    */
   ~SeedFinderOrthogonal() = default;
 
-  SeedFinderOrthogonal() = default;
+  SeedFinderOrthogonal() = delete;
   SeedFinderOrthogonal(const SeedFinderOrthogonal<external_spacepoint_t> &) =
       delete;
   SeedFinderOrthogonal<external_spacepoint_t> &operator=(
-      const SeedFinderOrthogonal<external_spacepoint_t> &) = default;
+      const SeedFinderOrthogonal<external_spacepoint_t> &) = delete;
+  SeedFinderOrthogonal(
+      SeedFinderOrthogonal<external_spacepoint_t> &&) noexcept = default;
+  SeedFinderOrthogonal<external_spacepoint_t> &operator=(
+      SeedFinderOrthogonal<external_spacepoint_t> &&) noexcept = default;
 
   /**
    * @brief Perform seed finding, appending seeds to a container.
@@ -238,6 +245,16 @@ class SeedFinderOrthogonal {
    * @brief The configuration for the seeding algorithm.
    */
   Acts::SeedFinderOrthogonalConfig<external_spacepoint_t> m_config;
+
+  /**
+   * @brief Get the logger.
+   */
+  const Logger &logger() const { return *m_logger; }
+
+  /**
+   * @brief The logger
+   */
+  std::unique_ptr<const Acts::Logger> m_logger{nullptr};
 };
 }  // namespace Acts
 

Core/include/Acts/Seeding/SeedFinderOrthogonal.ipp

@@ -244,8 +244,9 @@ bool SeedFinderOrthogonal<external_spacepoint_t>::validTuple(
 
 template <typename external_spacepoint_t>
 SeedFinderOrthogonal<external_spacepoint_t>::SeedFinderOrthogonal(
-    const SeedFinderOrthogonalConfig<external_spacepoint_t> &config)
-    : m_config(config) {
+    const SeedFinderOrthogonalConfig<external_spacepoint_t> &config,
+    std::unique_ptr<const Acts::Logger> logger)
+    : m_config(config), m_logger(std::move(logger)) {
   if (!config.isInInternalUnits) {
     throw std::runtime_error(
         "SeedFinderOrthogonalConfig not in ACTS internal units in "
@@ -687,6 +688,7 @@ auto SeedFinderOrthogonal<external_spacepoint_t>::createTree(
     const std::vector<const external_spacepoint_t *> &spacePoints) const
     -> tree_t {
   std::vector<typename tree_t::pair_t> points;
+  points.reserve(spacePoints.size());
 
   /*
    * For every input point, we create a coordinate-pointer pair, which we then
@@ -703,6 +705,8 @@ auto SeedFinderOrthogonal<external_spacepoint_t>::createTree(
     points.emplace_back(point, sp);
   }
 
+  ACTS_VERBOSE("Created k-d tree populated with " << points.size()
+                                                  << " space points");
   return tree_t(std::move(points));
 }
 
@@ -711,6 +715,7 @@ template <typename input_container_t, typename output_container_t>
 void SeedFinderOrthogonal<external_spacepoint_t>::createSeeds(
     const Acts::SeedFinderOptions &options,
     const input_container_t &spacePoints, output_container_t &out_cont) const {
+  ACTS_VERBOSE("Creating seeds with Orthogonal strategy");
   if (!options.isInInternalUnits) {
     throw std::runtime_error(
         "SeedFinderOptions not in ACTS internal units in "
@@ -734,6 +739,7 @@ void SeedFinderOrthogonal<external_spacepoint_t>::createSeeds(
    * take each external spacepoint, allocate a corresponding internal space
    * point, and save it in a vector.
    */
+  ACTS_VERBOSE("Running on " << spacePoints.size() << " input space points");
   Acts::Extent rRangeSPExtent;
   std::vector<const external_spacepoint_t *> internal_sps;
   internal_sps.reserve(spacePoints.size());
@@ -746,6 +752,8 @@ void SeedFinderOrthogonal<external_spacepoint_t>::createSeeds(
     rRangeSPExtent.extend({p.x(), p.y(), p.z()});
     internal_sps.push_back(&p);
   }
+  ACTS_VERBOSE(rRangeSPExtent);
+
   // variable middle SP radial region of interest
   const Acts::Range1D<float> rMiddleSPRange(
       std::floor(rRangeSPExtent.min(Acts::BinningValue::binR) / 2) * 2 +

Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingOrthogonalAlgorithm.hpp

@@ -70,7 +70,7 @@ class SeedingOrthogonalAlgorithm final : public IAlgorithm {
 
  private:
   Config m_cfg;
-  Acts::SeedFinderOrthogonal<proxy_type> m_finder;
+  std::unique_ptr<Acts::SeedFinderOrthogonal<proxy_type>> m_finder{nullptr};
 
   std::vector<std::unique_ptr<ReadDataHandle<SimSpacePointContainer>>>
       m_inputSpacePoints{};

Examples/Algorithms/TrackFinding/src/GbtsSeedingAlgorithm.cpp

@@ -88,20 +88,20 @@ ActsExamples::ProcessCode ActsExamples::GbtsSeedingAlgorithm::execute(
       MakeGbtsSpacePoints(ctx, m_cfg.ActsGbtsMap);
 
   for (auto sp : GbtsSpacePoints) {
-    ACTS_DEBUG("Gbts space points: " << " Gbts_id: " << sp.gbtsID << " z: "
-                                     << sp.SP->z() << " r: " << sp.SP->r()
-                                     << " ACTS volume:  "
-                                     << sp.SP->sourceLinks()
-                                            .front()
-                                            .get<IndexSourceLink>()
-                                            .geometryId()
-                                            .volume()
-                                     << "\n");
+    ACTS_DEBUG("Gbts space points:  Gbts_id: "
+               << sp.gbtsID << " z: " << sp.SP->z() << " r: " << sp.SP->r()
+               << " ACTS volume:  "
+               << sp.SP->sourceLinks()
+                      .front()
+                      .get<IndexSourceLink>()
+                      .geometryId()
+                      .volume());
   }
 
   // this is now calling on a core algorithm
-  Acts::SeedFinderGbts<SimSpacePoint> finder(m_cfg.seedFinderConfig,
-                                             *m_gbtsGeo);
+  Acts::SeedFinderGbts<SimSpacePoint> finder(
+      m_cfg.seedFinderConfig, *m_gbtsGeo,
+      logger().cloneWithSuffix("GbtdFinder"));
 
   // output of function needed for seed
 

Examples/Algorithms/TrackFinding/src/SeedingOrthogonalAlgorithm.cpp

@@ -67,9 +67,11 @@ ActsExamples::SeedingOrthogonalAlgorithm::SeedingOrthogonalAlgorithm(
 
   // construct seed filter
   m_cfg.seedFinderConfig.seedFilter =
-      std::make_unique<Acts::SeedFilter<proxy_type>>(m_cfg.seedFilterConfig);
+      std::make_unique<Acts::SeedFilter<proxy_type>>(
+          m_cfg.seedFilterConfig, logger().cloneWithSuffix("Filter"));
 
-  m_finder = Acts::SeedFinderOrthogonal<proxy_type>(m_cfg.seedFinderConfig);
+  m_finder = std::make_unique<Acts::SeedFinderOrthogonal<proxy_type>>(
+      m_cfg.seedFinderConfig, logger().cloneWithSuffix("Finder"));
 }
 
 ActsExamples::ProcessCode ActsExamples::SeedingOrthogonalAlgorithm::execute(
@@ -95,9 +97,8 @@ ActsExamples::ProcessCode ActsExamples::SeedingOrthogonalAlgorithm::execute(
 
   ACTS_INFO("About to process " << spContainer.size() << " space points ...");
 
-  Acts::SeedFinderOrthogonal<proxy_type> finder(m_cfg.seedFinderConfig);
   std::vector<Acts::Seed<proxy_type>> seeds =
-      finder.createSeeds(m_cfg.seedFinderOptions, spContainer);
+      m_finder->createSeeds(m_cfg.seedFinderOptions, spContainer);
 
   ACTS_INFO("Created " << seeds.size() << " track seeds from "
                        << spacePoints.size() << " space points");