refactor!: error handling in the vertexing algorithms

Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.hpp

@@ -58,11 +58,11 @@ class AdaptiveMultiVertexFinder {
     /// @param ipEst ImpactPointEstimator
     /// @param lin Track linearizer
     /// @param bIn Input magnetic field
-    Config(vfitter_t fitter, const sfinder_t& sfinder,
+    Config(vfitter_t fitter, sfinder_t sfinder,
            const ImpactPointEstimator<InputTrack_t, Propagator_t>& ipEst,
            Linearizer_t lin, std::shared_ptr<const MagneticFieldProvider> bIn)
         : vertexFitter(std::move(fitter)),
-          seedFinder(sfinder),
+          seedFinder(std::move(sfinder)),
           ipEstimator(ipEst),
           linearizer(std::move(lin)),
           bField{std::move(bIn)} {}

Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp

@@ -56,7 +56,7 @@ auto Acts::AdaptiveMultiVertexFinder<vfitter_t, sfinder_t>::find(
     Vertex<InputTrack_t>& vtxCandidate = *allVertices.back();
     allVerticesPtr.push_back(&vtxCandidate);
 
-    ACTS_DEBUG("Position of current vertex candidate after seeding: "
+    ACTS_DEBUG("Position of vertex candidate after seeding: "
                << vtxCandidate.fullPosition().transpose());
     if (vtxCandidate.position().z() ==
         vertexingOptions.constraint.position().z()) {
@@ -79,7 +79,8 @@ auto Acts::AdaptiveMultiVertexFinder<vfitter_t, sfinder_t>::find(
       return prepResult.error();
     }
     if (!(*prepResult)) {
-      ACTS_DEBUG("Could not prepare for fit anymore. Break.");
+      ACTS_DEBUG(
+          "Could not prepare for fit. Discarding the vertex candindate.");
       allVertices.pop_back();
       allVerticesPtr.pop_back();
       break;
@@ -93,7 +94,7 @@ auto Acts::AdaptiveMultiVertexFinder<vfitter_t, sfinder_t>::find(
     if (!fitResult.ok()) {
       return fitResult.error();
     }
-    ACTS_DEBUG("New position of current vertex candidate after fit: "
+    ACTS_DEBUG("Position of vertex candidate after the fit: "
                << vtxCandidate.fullPosition().transpose());
     // Check if vertex is good vertex
     auto [nCompatibleTracks, isGoodVertex] =

Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.hpp

@@ -297,6 +297,13 @@ class AdaptiveMultiVertexFitter {
   ///
   /// @param state Fitter state
   void doVertexSmoothing(State& state) const;
+
+  /// @brief Logs vertices in state.vertexCollection and associated tracks
+  ///
+  /// @param state Fitter state
+  /// @param geoContext Geometry context
+  void logDebugData(const State& state,
+                    const GeometryContext& geoContext) const;
 };
 
 }  // namespace Acts

Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp

@@ -51,7 +51,15 @@ Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::fit(
         vtxInfo.relinearize = true;
         // Recalculate the track impact parameters at the current vertex
         // position
-        prepareVertexForFit(state, vtx, vertexingOptions);
+        auto prepareVertexResult =
+            prepareVertexForFit(state, vtx, vertexingOptions);
+        if (!prepareVertexResult.ok()) {
+          // Print vertices and associated tracks if logger is in debug mode
+          if (logger().doPrint(Logging::DEBUG)) {
+            logDebugData(state, vertexingOptions.geoContext);
+          }
+          return prepareVertexResult.error();
+        }
       }
 
       // Check if we use the constraint during the vertex fit
@@ -68,11 +76,27 @@ Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::fit(
 
       // Set vertexCompatibility for all TrackAtVertex objects
       // at the current vertex
-      setAllVertexCompatibilities(state, vtx, vertexingOptions);
+      auto setCompatibilitiesResult =
+          setAllVertexCompatibilities(state, vtx, vertexingOptions);
+      if (!setCompatibilitiesResult.ok()) {
+        // Print vertices and associated tracks if logger is in debug mode
+        if (logger().doPrint(Logging::DEBUG)) {
+          logDebugData(state, vertexingOptions.geoContext);
+        }
+        return setCompatibilitiesResult.error();
+      }
     }  // End loop over vertex collection
 
     // Recalculate all track weights and update vertices
-    setWeightsAndUpdate(state, linearizer, vertexingOptions);
+    auto setWeightsResult =
+        setWeightsAndUpdate(state, linearizer, vertexingOptions);
+    if (!setWeightsResult.ok()) {
+      // Print vertices and associated tracks if logger is in debug mode
+      if (logger().doPrint(Logging::DEBUG)) {
+        logDebugData(state, vertexingOptions.geoContext);
+      }
+      return setWeightsResult.error();
+    }
 
     // Cool the system down, i.e., reduce the temperature parameter. At lower
     // temperatures, outlying tracks are downweighted more.
@@ -100,17 +124,14 @@ Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::addVtxToFit(
     const linearizer_t& linearizer,
     const VertexingOptions<input_track_t>& vertexingOptions) const {
   if (state.vtxInfoMap[&newVertex].trackLinks.empty()) {
+    ACTS_ERROR(
+        "newVertex does not have any associated tracks (i.e., its trackLinks "
+        "are empty).")
     return VertexingError::EmptyInput;
   }
 
   std::vector<Vertex<input_track_t>*> verticesToFit = {&newVertex};
 
-  // Save the 3D impact parameters of all tracks associated with newVertex.
-  auto res = prepareVertexForFit(state, &newVertex, vertexingOptions);
-  if (!res.ok()) {
-    return res.error();
-  }
-
   // List of vertices added in last iteration
   std::vector<Vertex<input_track_t>*> lastIterAddedVertices = {&newVertex};
   // List of vertices added in current iteration
@@ -153,6 +174,16 @@ Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::addVtxToFit(
 
   state.vertexCollection = verticesToFit;
 
+  // Save the 3D impact parameters of all tracks associated with newVertex.
+  auto res = prepareVertexForFit(state, &newVertex, vertexingOptions);
+  if (!res.ok()) {
+    // Print vertices and associated tracks if logger is in debug mode
+    if (logger().doPrint(Logging::DEBUG)) {
+      logDebugData(state, vertexingOptions.geoContext);
+    }
+    return res.error();
+  }
+
   // Perform fit on all added vertices
   auto fitRes = fit(state, linearizer, vertexingOptions);
   if (!fitRes.ok()) {
@@ -343,3 +374,30 @@ void Acts::AdaptiveMultiVertexFitter<
     }
   }
 }
+
+template <typename input_track_t, typename linearizer_t>
+void Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::logDebugData(
+    const State& state, const Acts::GeometryContext& geoContext) const {
+  ACTS_DEBUG("Encountered an error when fitting the following "
+             << state.vertexCollection.size() << " vertices:");
+  for (std::size_t vtxInd = 0; vtxInd < state.vertexCollection.size();
+       ++vtxInd) {
+    auto vtx = state.vertexCollection[vtxInd];
+    ACTS_DEBUG("Position of " << vtxInd << ". vertex seed:\n"
+                              << state.vtxInfoMap.at(vtx).seedPosition);
+    ACTS_DEBUG("Position of said vertex after the last fitting step:\n"
+               << state.vtxInfoMap.at(vtx).oldPosition);
+    ACTS_DEBUG("Associated tracks:");
+    const auto& trks = state.vtxInfoMap.at(vtx).trackLinks;
+    for (std::size_t trkInd = 0; trkInd < trks.size(); ++trkInd) {
+      const auto& trkAtVtx =
+          state.tracksAtVerticesMap.at(std::make_pair(trks[trkInd], vtx));
+      const auto& trkParams = m_extractParameters(*(trkAtVtx.originalParams));
+      ACTS_DEBUG(trkInd << ". track parameters:\n" << trkParams.parameters());
+      ACTS_DEBUG(trkInd << ". track covariance matrix:\n"
+                        << trkParams.covariance().value());
+      ACTS_DEBUG("Origin of corresponding reference surface:\n"
+                 << trkParams.referenceSurface().center(geoContext));
+    }
+  }
+}

Core/include/Acts/Vertexing/FullBilloirVertexFitter.hpp

@@ -10,6 +10,7 @@
 
 #include "Acts/Propagator/EigenStepper.hpp"
 #include "Acts/Propagator/Propagator.hpp"
+#include "Acts/Utilities/Logger.hpp"
 #include "Acts/Utilities/Result.hpp"
 #include "Acts/Vertexing/HelicalTrackLinearizer.hpp"
 #include "Acts/Vertexing/LinearizerConcept.hpp"
@@ -70,8 +71,13 @@ class FullBilloirVertexFitter {
   template <
       typename T = input_track_t,
       std::enable_if_t<std::is_same<T, BoundTrackParameters>::value, int> = 0>
-  FullBilloirVertexFitter(const Config& cfg)
-      : m_cfg(cfg), extractParameters([](T params) { return params; }) {}
+  FullBilloirVertexFitter(const Config& cfg,
+                          std::unique_ptr<const Logger> logger =
+                              getDefaultLogger("FullBilloirVertexFitter",
+                                               Logging::INFO))
+      : m_cfg(cfg),
+        extractParameters([](T params) { return params; }),
+        m_logger(std::move(logger)) {}
 
   /// @brief Constructor for user-defined input_track_t type =!
   /// BoundTrackParameters
@@ -81,8 +87,10 @@ class FullBilloirVertexFitter {
   /// object
   FullBilloirVertexFitter(
       const Config& cfg,
-      std::function<BoundTrackParameters(input_track_t)> func)
-      : m_cfg(cfg), extractParameters(func) {}
+      std::function<BoundTrackParameters(input_track_t)> func,
+      std::unique_ptr<const Logger> logger =
+          getDefaultLogger("FullBilloirVertexFitter", Logging::INFO))
+      : m_cfg(cfg), extractParameters(func), m_logger(std::move(logger)) {}
 
   /// @brief Fit method, fitting vertex for provided tracks with constraint
   ///
@@ -107,6 +115,12 @@ class FullBilloirVertexFitter {
   /// overwritten to return BoundTrackParameters for other input_track_t
   /// objects.
   std::function<BoundTrackParameters(input_track_t)> extractParameters;
+
+  /// Logging instance
+  std::unique_ptr<const Logger> m_logger;
+
+  /// Private access to logging instance
+  const Logger& logger() const { return *m_logger; }
 };
 
 }  // namespace Acts

Core/include/Acts/Vertexing/FullBilloirVertexFitter.ipp

@@ -298,6 +298,7 @@ Acts::FullBilloirVertexFitter<input_track_t, linearizer_t>::fit(
     }
 
     if (!std::isnormal(newChi2)) {
+      ACTS_ERROR("Encountered non-normal chi2 value during the fit.");
       return VertexingError::NumericFailure;
     }
 

Core/include/Acts/Vertexing/ImpactPointEstimator.hpp

@@ -14,6 +14,7 @@
 #include "Acts/MagneticField/MagneticFieldProvider.hpp"
 #include "Acts/MagneticField/NullBField.hpp"
 #include "Acts/Propagator/Propagator.hpp"
+#include "Acts/Utilities/Logger.hpp"
 #include "Acts/Utilities/Result.hpp"
 #include "Acts/Vertexing/TrackAtVertex.hpp"
 #include "Acts/Vertexing/Vertex.hpp"
@@ -86,7 +87,10 @@ class ImpactPointEstimator {
   /// @brief Constructor
   ///
   /// @param cfg Configuration object
-  ImpactPointEstimator(const Config& cfg) : m_cfg(cfg) {}
+  ImpactPointEstimator(const Config& cfg,
+                       std::unique_ptr<const Logger> logger = getDefaultLogger(
+                           "ImpactPointEstimator", Logging::INFO))
+      : m_cfg(cfg), m_logger(std::move(logger)) {}
 
   /// @brief Calculates 3D distance between a track and a vertex
   ///
@@ -209,6 +213,12 @@ class ImpactPointEstimator {
   /// Configuration object
   const Config m_cfg;
 
+  /// Logging instance
+  std::shared_ptr<const Logger> m_logger;
+
+  /// Private access to logging instance
+  const Logger& logger() const { return *m_logger; }
+
   /// @brief Performs a Newton approximation to retrieve a point
   /// of closest approach in 3D to a reference position
   ///

Core/include/Acts/Vertexing/ImpactPointEstimator.ipp

@@ -98,6 +98,10 @@ Acts::ImpactPointEstimator<input_track_t, propagator_t, propagator_options_t>::
   if (result.ok()) {
     return *result->endParameters;
   } else {
+    ACTS_ERROR("Error during propagation in estimate3DImpactParameters.");
+    ACTS_DEBUG(
+        "The plane surface to which we tried to propagate has its origin at\n"
+        << vtxPos);
     return result.error();
   }
 }
@@ -205,6 +209,8 @@ Acts::Result<double> Acts::ImpactPointEstimator<
                                   rho * cotTheta * cotTheta);
 
     if (secDerivative < 0.) {
+      ACTS_ERROR(
+          "Encountered negative second derivative during Newton optimization.");
       return VertexingError::NumericFailure;
     }
 
@@ -222,7 +228,7 @@ Acts::Result<double> Acts::ImpactPointEstimator<
   }  // end while loop
 
   if (!hasConverged) {
-    // max iterations reached but did not converge
+    ACTS_ERROR("Newton optimization did not converge.");
     return VertexingError::NotConverged;
   }
   return phi;
@@ -250,6 +256,8 @@ Acts::ImpactPointEstimator<input_track_t, propagator_t, propagator_options_t>::
   // Note that we assume a constant B field here!
   auto fieldRes = m_cfg.bField->getField(refPoint, state.fieldCache);
   if (!fieldRes.ok()) {
+    ACTS_ERROR("In getDistanceAndMomentum, the B field at\n"
+               << refPoint << "\ncould not be retrieved.");
     return fieldRes.error();
   }
   double bZ = (*fieldRes)[eZ];
@@ -389,6 +397,10 @@ Acts::ImpactPointEstimator<input_track_t, propagator_t, propagator_options_t>::
   auto result = m_cfg.propagator->propagate(track, *perigeeSurface, pOptions);
 
   if (!result.ok()) {
+    ACTS_ERROR("Error during propagation in getImpactParameters.");
+    ACTS_DEBUG(
+        "The Perigee surface to which we tried to propagate has its origin at\n"
+        << vtx.position());
     return result.error();
   }
 

Core/include/Acts/Vertexing/IterativeVertexFinder.hpp

@@ -75,9 +75,9 @@ class IterativeVertexFinder {
     /// @param lin Track linearizer
     /// @param sfinder The seed finder
     /// @param est ImpactPointEstimator
-    Config(const vfitter_t& fitter, Linearizer_t lin, sfinder_t sfinder,
+    Config(vfitter_t fitter, Linearizer_t lin, sfinder_t sfinder,
            const IPEstimator& est)
-        : vertexFitter(fitter),
+        : vertexFitter(std::move(fitter)),
           linearizer(std::move(lin)),
           seedFinder(std::move(sfinder)),
           ipEst(est) {}

Core/include/Acts/Vertexing/IterativeVertexFinder.ipp

@@ -164,15 +164,15 @@ auto Acts::IterativeVertexFinder<vfitter_t, sfinder_t>::getVertexSeed(
   auto res = m_cfg.seedFinder.find(seedTracks, vertexingOptions, finderState);
 
   if (!res.ok()) {
-    ACTS_DEBUG("Seeding error: internal. Number of input tracks: "
+    ACTS_ERROR("Internal seeding error. Number of input tracks: "
                << seedTracks.size());
     return VertexingError::SeedingError;
   }
 
   const auto& vertexCollection = *res;
 
   if (vertexCollection.empty()) {
-    ACTS_DEBUG("Seeding error: no seeds. Number of input tracks: "
+    ACTS_ERROR("Empty seed collection was returned. Number of input tracks: "
                << seedTracks.size());
     return VertexingError::SeedingError;
   }
@@ -183,11 +183,9 @@ auto Acts::IterativeVertexFinder<vfitter_t, sfinder_t>::getVertexSeed(
   // the seed vertexCollection
   Vertex<InputTrack_t> seedVertex = vertexCollection.back();
 
-  ACTS_DEBUG("Considering seed at position: ("
-             << seedVertex.fullPosition()[eX] << ", "
-             << seedVertex.fullPosition()[eY] << ", "
-             << seedVertex.fullPosition()[eZ] << ", " << seedVertex.time()
-             << "). Number of input tracks: " << seedTracks.size());
+  ACTS_DEBUG("Use " << seedTracks.size() << " tracks for vertex seed finding.")
+  ACTS_DEBUG(
+      "Found seed at position: " << seedVertex.fullPosition().transpose());
 
   return seedVertex;
 }

Examples/Algorithms/Vertexing/src/AdaptiveMultiVertexFinderAlgorithm.cpp

@@ -87,11 +87,13 @@ ActsExamples::AdaptiveMultiVertexFinderAlgorithm::executeAfterSeederChoice(
 
   // Set up ImpactPointEstimator
   IPEstimator::Config ipEstimatorCfg(m_cfg.bField, propagator);
-  IPEstimator ipEstimator(ipEstimatorCfg);
+  IPEstimator ipEstimator(ipEstimatorCfg,
+                          logger().cloneWithSuffix("ImpactPointEstimator"));
 
   // Set up the helical track linearizer
   Linearizer::Config ltConfig(m_cfg.bField, propagator);
-  Linearizer linearizer(ltConfig, logger().cloneWithSuffix("HelLin"));
+  Linearizer linearizer(ltConfig,
+                        logger().cloneWithSuffix("HelicalTrackLinearizer"));
 
   // Set up deterministic annealing with user-defined temperatures
   Acts::AnnealingUtility::Config annealingConfig;
@@ -103,17 +105,18 @@ ActsExamples::AdaptiveMultiVertexFinderAlgorithm::executeAfterSeederChoice(
   fitterCfg.annealingTool = annealingUtility;
   fitterCfg.minWeight = 0.001;
   fitterCfg.doSmoothing = true;
-  Fitter fitter(std::move(fitterCfg), logger().cloneWithSuffix("AMVFitter"));
+  Fitter fitter(std::move(fitterCfg),
+                logger().cloneWithSuffix("AdaptiveMultiVertexFitter"));
 
-  typename Finder::Config finderConfig(std::move(fitter), seedFinder,
+  typename Finder::Config finderConfig(std::move(fitter), std::move(seedFinder),
                                        ipEstimator, std::move(linearizer),
                                        m_cfg.bField);
   finderConfig.looseConstrValue = 1e2;
   finderConfig.tracksMaxZinterval = 1. * Acts::UnitConstants::mm;
   finderConfig.maxIterations = 200;
 
   // Instantiate the finder
-  Finder finder(std::move(finderConfig), logger().cloneWithSuffix("AMVFinder"));
+  Finder finder(std::move(finderConfig), logger().clone());
 
   // retrieve input tracks and convert into the expected format