refactor: Improve log in seeding (acts-project#3792)

Adding some `VERBOSE` statements in the  seeding, from Grid creation and Filling to seed finder and filter.

Core/include/Acts/Seeding/SeedFilter.hpp

@@ -13,6 +13,7 @@
 #include "Acts/Seeding/CandidatesForMiddleSp.hpp"
 #include "Acts/Seeding/IExperimentCuts.hpp"
 #include "Acts/Seeding/SeedFilterConfig.hpp"
+#include "Acts/Utilities/Logger.hpp"
 
 #include <memory>
 #include <mutex>
@@ -37,8 +38,15 @@ struct SeedFilterState {
 template <typename external_spacepoint_t>
 class SeedFilter final {
  public:
-  SeedFilter(SeedFilterConfig config,
+  SeedFilter(const SeedFilterConfig& config,
              IExperimentCuts<external_spacepoint_t>* expCuts = nullptr);
+  SeedFilter(const SeedFilterConfig& config,
+             std::unique_ptr<const Acts::Logger> logger,
+             IExperimentCuts<external_spacepoint_t>* expCuts = nullptr);
+  SeedFilter(const SeedFilter<external_spacepoint_t>&) = delete;
+  SeedFilter& operator=(const SeedFilter<external_spacepoint_t>&) = delete;
+  SeedFilter(SeedFilter<external_spacepoint_t>&&) noexcept = default;
+  SeedFilter& operator=(SeedFilter<external_spacepoint_t>&&) noexcept = default;
 
   SeedFilter() = delete;
   ~SeedFilter() = default;
@@ -95,7 +103,11 @@ class SeedFilter final {
   }
 
  private:
+  const Logger& logger() const { return *m_logger; }
+
   const SeedFilterConfig m_cfg;
+  std::unique_ptr<const Acts::Logger> m_logger =
+      Acts::getDefaultLogger("SeedFilter", Logging::Level::INFO);
   const IExperimentCuts<external_spacepoint_t>* m_experimentCuts;
 };
 }  // namespace Acts

Core/include/Acts/Seeding/SeedFilter.ipp

@@ -16,14 +16,26 @@ namespace Acts {
 // constructor
 template <typename external_spacepoint_t>
 SeedFilter<external_spacepoint_t>::SeedFilter(
-    SeedFilterConfig config,
+    const SeedFilterConfig& config,
     IExperimentCuts<external_spacepoint_t>* expCuts /* = 0*/)
     : m_cfg(config), m_experimentCuts(expCuts) {
   if (!config.isInInternalUnits) {
     throw std::runtime_error(
         "SeedFilterConfig not in ACTS internal units in SeedFilter");
   }
 }
+
+template <typename external_spacepoint_t>
+SeedFilter<external_spacepoint_t>::SeedFilter(
+    const SeedFilterConfig& config, std::unique_ptr<const Acts::Logger> logger,
+    IExperimentCuts<external_spacepoint_t>* expCuts /* = 0*/)
+    : m_cfg(config), m_logger(std::move(logger)), m_experimentCuts(expCuts) {
+  if (!config.isInInternalUnits) {
+    throw std::runtime_error(
+        "SeedFilterConfig not in ACTS internal units in SeedFilter");
+  }
+}
+
 // function to filter seeds based on all seeds with same bottom- and
 // middle-spacepoint.
 // return vector must contain weight of each seed
@@ -295,10 +307,14 @@ void SeedFilter<external_spacepoint_t>::filterSeeds_1SpFixed(
     seed.setVertexZ(zOrigin);
     seed.setQuality(bestSeedQuality);
 
+    ACTS_VERBOSE("Adding seed: [b=" << bottom->index() << ", m="
+                                    << medium->index() << ", t=" << top->index()
+                                    << "], quality=" << bestSeedQuality
+                                    << ", vertexZ=" << zOrigin);
     Acts::detail::pushBackOrInsertAtEnd(outputCollection, std::move(seed));
-
     ++numTotalSeeds;
   }
+  ACTS_VERBOSE("Identified " << numTotalSeeds << " seeds");
 }
 
 }  // namespace Acts

Core/include/Acts/Seeding/SeedFinder.hpp

@@ -18,6 +18,7 @@
 #include "Acts/Seeding/SeedFinderUtils.hpp"
 #include "Acts/Seeding/SpacePointGrid.hpp"
 #include "Acts/Seeding/detail/UtilityFunctions.hpp"
+#include "Acts/Utilities/Logger.hpp"
 
 #include <array>
 #include <limits>
@@ -87,15 +88,22 @@ class SeedFinder {
 
   /// The only constructor. Requires a config object.
   /// @param config the configuration for the SeedFinder
-  SeedFinder(const Acts::SeedFinderConfig<external_spacepoint_t>& config);
+  /// @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));
+  SeedFinder(SeedFinder<external_spacepoint_t, grid_t, platform_t>&&) noexcept =
+      default;
+  SeedFinder& operator=(SeedFinder<external_spacepoint_t, grid_t,
+                                   platform_t>&&) noexcept = default;
   ~SeedFinder() = default;
   /**    @name Disallow default instantiation, copy, assignment */
   //@{
   SeedFinder() = default;
   SeedFinder(const SeedFinder<external_spacepoint_t, grid_t, platform_t>&) =
       delete;
   SeedFinder<external_spacepoint_t, grid_t, platform_t>& operator=(
-      const SeedFinder<external_spacepoint_t, grid_t, platform_t>&) = default;
+      const SeedFinder<external_spacepoint_t, grid_t, platform_t>&) = delete;
   //@}
 
   /// Create all seeds from the space points in the three iterators.
@@ -172,7 +180,10 @@ class SeedFinder {
                         SeedingState& state) const;
 
  private:
+  const Logger& logger() const { return *m_logger; }
+
   Acts::SeedFinderConfig<external_spacepoint_t> m_config;
+  std::unique_ptr<const Acts::Logger> m_logger{nullptr};
 };
 
 }  // namespace Acts

Core/include/Acts/Seeding/SeedFinder.ipp

@@ -15,8 +15,9 @@ namespace Acts {
 
 template <typename external_spacepoint_t, typename grid_t, typename platform_t>
 SeedFinder<external_spacepoint_t, grid_t, platform_t>::SeedFinder(
-    const Acts::SeedFinderConfig<external_spacepoint_t>& config)
-    : m_config(config) {
+    const Acts::SeedFinderConfig<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(
         "SeedFinderConfig not in ACTS internal units in SeedFinder");
@@ -110,6 +111,12 @@ void SeedFinder<external_spacepoint_t, grid_t, platform_t>::createSeedsForGroup(
   // same z-bin
   auto [minRadiusRangeForMiddle, maxRadiusRangeForMiddle] =
       retrieveRadiusRangeForMiddle(*middleSPs.front(), rMiddleSPRange);
+  ACTS_VERBOSE("Current global bin: " << middleSPsIdx << ", z value of "
+                                      << middleSPs.front()->z());
+  ACTS_VERBOSE("Validity range (radius) for the middle space point is ["
+               << minRadiusRangeForMiddle << ", " << maxRadiusRangeForMiddle
+               << "]");
+
   for (const external_spacepoint_t* spM : middleSPs) {
     const float rM = spM->radius();
 
@@ -136,6 +143,7 @@ void SeedFinder<external_spacepoint_t, grid_t, platform_t>::createSeedsForGroup(
 
     // no top SP found -> try next spM
     if (state.compatTopSP.empty()) {
+      ACTS_VERBOSE("No compatible Tops, moving to next middle candidate");
       continue;
     }
 
@@ -157,6 +165,10 @@ void SeedFinder<external_spacepoint_t, grid_t, platform_t>::createSeedsForGroup(
       seedFilterState.rMaxSeedConf = seedConfRange.rMaxSeedConf;
       // continue if number of top SPs is smaller than minimum
       if (state.compatTopSP.size() < seedFilterState.nTopSeedConf) {
+        ACTS_VERBOSE(
+            "Number of top SPs is "
+            << state.compatTopSP.size()
+            << " and is smaller than minimum, moving to next middle candidate");
         continue;
       }
     }
@@ -170,9 +182,14 @@ void SeedFinder<external_spacepoint_t, grid_t, platform_t>::createSeedsForGroup(
 
     // no bottom SP found -> try next spM
     if (state.compatBottomSP.empty()) {
+      ACTS_VERBOSE("No compatible Bottoms, moving to next middle candidate");
       continue;
     }
 
+    ACTS_VERBOSE("Candidates: " << state.compatBottomSP.size()
+                                << " bottoms and " << state.compatTopSP.size()
+                                << " tops for middle candidate indexed "
+                                << spM->index());
     // filter candidates
     if (m_config.useDetailedDoubleMeasurementInfo) {
       filterCandidates<Acts::DetectorMeasurementInfo::eDetailed>(

Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp

@@ -11,6 +11,7 @@
 #include "Acts/Seeding/BinnedGroup.hpp"
 #include "Acts/Seeding/SeedFinderConfig.hpp"
 #include "Acts/Utilities/Grid.hpp"
+#include "Acts/Utilities/Logger.hpp"
 
 #include <numbers>
 #include <vector>
@@ -158,7 +159,8 @@ class CylindricalSpacePointGridCreator {
   template <typename external_spacepoint_t>
   static Acts::CylindricalSpacePointGrid<external_spacepoint_t> createGrid(
       const Acts::CylindricalSpacePointGridConfig& _config,
-      const Acts::CylindricalSpacePointGridOptions& _options);
+      const Acts::CylindricalSpacePointGridOptions& _options,
+      const Acts::Logger& logger = Acts::getDummyLogger());
 
   template <typename external_spacepoint_t,
             typename external_spacepoint_iterator_t>
@@ -167,7 +169,8 @@ class CylindricalSpacePointGridCreator {
       const Acts::SeedFinderOptions& options,
       Acts::CylindricalSpacePointGrid<external_spacepoint_t>& grid,
       external_spacepoint_iterator_t spBegin,
-      external_spacepoint_iterator_t spEnd);
+      external_spacepoint_iterator_t spEnd,
+      const Acts::Logger& logger = Acts::getDummyLogger());
 
   template <typename external_spacepoint_t, typename external_collection_t>
     requires std::ranges::range<external_collection_t> &&
@@ -177,7 +180,8 @@ class CylindricalSpacePointGridCreator {
       const Acts::SeedFinderConfig<external_spacepoint_t>& config,
       const Acts::SeedFinderOptions& options,
       Acts::CylindricalSpacePointGrid<external_spacepoint_t>& grid,
-      const external_collection_t& collection);
+      const external_collection_t& collection,
+      const Acts::Logger& logger = Acts::getDummyLogger());
 };
 
 }  // namespace Acts

Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp

@@ -12,7 +12,8 @@ template <typename external_spacepoint_t>
 Acts::CylindricalSpacePointGrid<external_spacepoint_t>
 Acts::CylindricalSpacePointGridCreator::createGrid(
     const Acts::CylindricalSpacePointGridConfig& config,
-    const Acts::CylindricalSpacePointGridOptions& options) {
+    const Acts::CylindricalSpacePointGridOptions& options,
+    const Acts::Logger& logger) {
   if (!config.isInInternalUnits) {
     throw std::runtime_error(
         "CylindricalSpacePointGridConfig not in ACTS internal units in "
@@ -30,6 +31,9 @@ Acts::CylindricalSpacePointGridCreator::createGrid(
   // for no magnetic field, create 100 phi-bins
   if (options.bFieldInZ == 0) {
     phiBins = 100;
+    ACTS_VERBOSE(
+        "B-Field is 0 (z-coordinate), setting the number of bins in phi to "
+        << phiBins);
   } else {
     // calculate circle intersections of helix and max detector radius
     float minHelixRadius =
@@ -39,7 +43,7 @@ Acts::CylindricalSpacePointGridCreator::createGrid(
                               // = pT[MeV] / (300 *Bz[kT])
 
     // sanity check: if yOuter takes the square root of a negative number
-    if (minHelixRadius < config.rMax / 2) {
+    if (minHelixRadius < config.rMax * 0.5) {
       throw std::domain_error(
           "The value of minHelixRadius cannot be smaller than rMax / 2. Please "
           "check the configuration of bFieldInZ and minPt");
@@ -141,6 +145,12 @@ Acts::CylindricalSpacePointGridCreator::createGrid(
 
   Axis<AxisType::Variable, AxisBoundaryType::Open> zAxis(std::move(zValues));
   Axis<AxisType::Variable, AxisBoundaryType::Open> rAxis(std::move(rValues));
+
+  ACTS_VERBOSE("Defining Grid:");
+  ACTS_VERBOSE("- Phi Axis: " << phiAxis);
+  ACTS_VERBOSE("- Z axis  : " << zAxis);
+  ACTS_VERBOSE("- R axis  : " << rAxis);
+
   return Acts::CylindricalSpacePointGrid<external_spacepoint_t>(
       std::make_tuple(std::move(phiAxis), std::move(zAxis), std::move(rAxis)));
 }
@@ -152,7 +162,7 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
     const Acts::SeedFinderOptions& options,
     Acts::CylindricalSpacePointGrid<external_spacepoint_t>& grid,
     external_spacepoint_iterator_t spBegin,
-    external_spacepoint_iterator_t spEnd) {
+    external_spacepoint_iterator_t spEnd, const Acts::Logger& logger) {
   if (!config.isInInternalUnits) {
     throw std::runtime_error(
         "SeedFinderConfig not in ACTS internal units in BinnedSPGroup");
@@ -180,9 +190,10 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
   std::vector<std::size_t> rBinsIndex;
   rBinsIndex.reserve(grid.size());
 
+  ACTS_VERBOSE("Fetching " << std::distance(spBegin, spEnd)
+                           << " space points to the grid");
   std::size_t counter = 0ul;
-  for (external_spacepoint_iterator_t it = spBegin; it != spEnd;
-       it++, ++counter) {
+  for (external_spacepoint_iterator_t it = spBegin; it != spEnd; ++it) {
     const external_spacepoint_t& sp = *it;
 
     // remove SPs according to experiment specific cuts
@@ -199,6 +210,7 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
     std::size_t globIndex = grid.globalBinFromPosition(position);
     auto& rbin = grid.at(globIndex);
     rbin.push_back(&sp);
+    ++counter;
 
     // keep track of the bins we modify so that we can later sort the SPs in
     // those bins only
@@ -213,6 +225,9 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
     auto& rbin = grid.atPosition(binIndex);
     std::ranges::sort(rbin, {}, [](const auto& rb) { return rb->radius(); });
   }
+
+  ACTS_VERBOSE(
+      "Number of space points inserted (within grid range): " << counter);
 }
 
 template <typename external_spacepoint_t, typename external_collection_t>
@@ -223,8 +238,8 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
     const Acts::SeedFinderConfig<external_spacepoint_t>& config,
     const Acts::SeedFinderOptions& options,
     Acts::CylindricalSpacePointGrid<external_spacepoint_t>& grid,
-    const external_collection_t& collection) {
+    const external_collection_t& collection, const Acts::Logger& logger) {
   Acts::CylindricalSpacePointGridCreator::fillGrid<external_spacepoint_t>(
       config, options, grid, std::ranges::begin(collection),
-      std::ranges::end(collection));
+      std::ranges::end(collection), logger);
 }

Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp

@@ -45,9 +45,8 @@ ActsExamples::SeedingAlgorithm::SeedingAlgorithm(
   // internal units
   m_cfg.seedFilterConfig = m_cfg.seedFilterConfig.toInternalUnits();
   m_cfg.seedFinderConfig.seedFilter =
-      std::make_shared<Acts::SeedFilter<SpacePointProxy_type>>(
-          m_cfg.seedFilterConfig);
-
+      std::make_unique<Acts::SeedFilter<SpacePointProxy_type>>(
+          m_cfg.seedFilterConfig, logger().cloneWithSuffix("SeedFilter"));
   m_cfg.seedFinderConfig =
       m_cfg.seedFinderConfig.toInternalUnits().calculateDerivedQuantities();
   m_cfg.seedFinderOptions =
@@ -195,13 +194,10 @@ ActsExamples::SeedingAlgorithm::SeedingAlgorithm(
   m_topBinFinder = std::make_unique<const Acts::GridBinFinder<3ul>>(
       m_cfg.numPhiNeighbors, m_cfg.zBinNeighborsTop, 0);
 
-  m_cfg.seedFinderConfig.seedFilter =
-      std::make_unique<Acts::SeedFilter<SpacePointProxy_type>>(
-          m_cfg.seedFilterConfig);
   m_seedFinder =
       Acts::SeedFinder<SpacePointProxy_type,
                        Acts::CylindricalSpacePointGrid<SpacePointProxy_type>>(
-          m_cfg.seedFinderConfig);
+          m_cfg.seedFinderConfig, logger().cloneWithSuffix("SeedFinder"));
 }
 
 ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute(
@@ -244,10 +240,11 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute(
 
   Acts::CylindricalSpacePointGrid<value_type> grid =
       Acts::CylindricalSpacePointGridCreator::createGrid<value_type>(
-          m_cfg.gridConfig, m_cfg.gridOptions);
+          m_cfg.gridConfig, m_cfg.gridOptions, logger());
 
   Acts::CylindricalSpacePointGridCreator::fillGrid<value_type>(
-      m_cfg.seedFinderConfig, m_cfg.seedFinderOptions, grid, spContainer);
+      m_cfg.seedFinderConfig, m_cfg.seedFinderOptions, grid, spContainer,
+      logger());
 
   // Compute radius Range
   // we rely on the fact the grid is storing the proxies

Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp

@@ -186,8 +186,8 @@ int main(int argc, char** argv) {
   Acts::SeedFilterConfig sfconf;
 
   Acts::ATLASCuts<value_type> atlasCuts = Acts::ATLASCuts<value_type>();
-  config.seedFilter = std::make_unique<Acts::SeedFilter<value_type>>(
-      Acts::SeedFilter<value_type>(sfconf, &atlasCuts));
+  config.seedFilter =
+      std::make_unique<Acts::SeedFilter<value_type>>(sfconf, &atlasCuts);
   Acts::SeedFinder<value_type, Acts::CylindricalSpacePointGrid<value_type>>
       a;  // test creation of unconfigured finder
   a = Acts::SeedFinder<value_type, Acts::CylindricalSpacePointGrid<value_type>>(