fix: CompositeSpacePointLineFitter - Add forgotten parameter boundary check (acts-project#4707)

- Implement the parameter boundary checks post update step
- Fix fast fit for straw measurements
- Minor doxygen improvements

Author: junggjo9

Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp

@@ -86,15 +86,6 @@ class CompositeSpacePointLineFitter {
     using RangeArray = std::array<std::array<double, 2>, s_nPars>;
     RangeArray ranges{};
   };
-
-  /// @brief Class constructor
-  /// @param cfg Reference to the fitter configuration object
-  /// @param logger
-  explicit CompositeSpacePointLineFitter(
-      const Config& cfg,
-      std::unique_ptr<const Logger> logger = getDefaultLogger(
-          "CompositeSpacePointLineFitter", Logging::Level::INFO));
-
   /// @brief Auxiliary object to store the fitted parameters, covariance,
   ///        the chi2 / nDoF & the number of required iterations
   struct FitParameters {
@@ -162,6 +153,16 @@ class CompositeSpacePointLineFitter {
     /// @brief Standard constructor
     FitOptions() = default;
   };
+
+  /// @brief Class constructor
+  /// @param cfg Reference to the fitter configuration object
+  /// @param logger Logger object used for debug print out
+  explicit CompositeSpacePointLineFitter(
+      const Config& cfg,
+      std::unique_ptr<const Logger> logger = getDefaultLogger(
+          "CompositeSpacePointLineFitter", Logging::Level::INFO));
+  /// @brief Returns the instantiated configuration object
+  const Config& config() const { return m_cfg; }
   /// @brief Counts how many measurements measure loc0, loc1 & time
   /// @param measurements: Collection of composite space points of interest
   template <CompositeSpacePointContainer Cont_t>
@@ -180,7 +181,9 @@ class CompositeSpacePointLineFitter {
   ///                    nonBending, bending & time
   static std::vector<FitParIndex> extractFitablePars(
       const std::array<std::size_t, 3>& hitCounts);
-
+  /// @brief Fit a line to a set of Composite space point measurements.
+  /// @param fitOpts: Auxiliary object carrying all necessary input
+  ///                 needed to execute the fit
   template <CompositeSpacePointContainer Cont_t,
             CompositeSpacePointCalibrator<Cont_t, Cont_t> Calibrator_t>
   FitResult<Cont_t> fit(FitOptions<Cont_t, Calibrator_t>&& fitOpts) const;
@@ -206,7 +209,7 @@ class CompositeSpacePointLineFitter {
   template <CompositeSpacePointContainer Cont_t>
   FitParameters fastFit(const Cont_t& measurements, const Line_t& initialGuess,
                         const std::vector<FitParIndex>& parsToUse) const;
-
+  /// @brief Abrivation of the fit result returned by the FastStrawLineFitter
   using FastFitResult = std::optional<detail::FastStrawLineFitter::FitResult>;
 
   /// @brief Executes the fast line fit in the bending direction. Returns

Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp

@@ -31,6 +31,10 @@ std::array<std::size_t, 3> CompositeSpacePointLineFitter::countDoF(
   std::size_t nValid{0};
   for (const auto& sp : measurements) {
     if (selector.connected() && !selector(*sp)) {
+      ACTS_VERBOSE(__func__ << "() " << __LINE__
+                            << " -  Skip invalid measurement @"
+                            << toString(sp->localPosition()));
+
       continue;
     }
     ++nValid;
@@ -96,9 +100,9 @@ CompositeSpacePointLineFitter::fastPrecFit(
         precResult->nIter += swappedPrecResult->nIter;
         ACTS_DEBUG(__func__ << "() " << __LINE__ << " - Fit did not improve "
                             << (*swappedPrecResult));
-        return precResult;
       }
     }
+    return precResult;
   }
   using ResidualIdx = detail::CompSpacePointAuxiliaries::ResidualIdx;
   return m_fastFitter.fit(measurements, ResidualIdx::bending);
@@ -300,7 +304,7 @@ CompositeSpacePointLineFitter::fit(
       double driftV{0.};
       double driftA{0.};
       // Calculate the residual & derivatives
-      if (resCfg.parsToUse.back() == FitParIndex::t0) {
+      if (pullCalculator.config().parsToUse.back() == FitParIndex::t0) {
         pullCalculator.updateFullResidual(line, t0, *spacePoint, driftV,
                                           driftA);
       } else {
@@ -313,37 +317,51 @@ CompositeSpacePointLineFitter::fit(
     result.chi2 = cache.chi2;
     // Now update the parameters
     UpdateStep update{UpdateStep::goodStep};
-    switch (resCfg.parsToUse.size()) {
+    switch (pullCalculator.config().parsToUse.size()) {
       // 2D fit (intercept + inclination angle)
       case 2: {
-        update = updateParameters<2>(resCfg.parsToUse.front(), cache,
-                                     result.parameters);
+        update = updateParameters<2>(pullCalculator.config().parsToUse.front(),
+                                     cache, result.parameters);
         break;
       }
       // 2D fit + time
       case 3: {
-        update = updateParameters<3>(resCfg.parsToUse.front(), cache,
-                                     result.parameters);
+        update = updateParameters<3>(pullCalculator.config().parsToUse.front(),
+                                     cache, result.parameters);
         break;
       }
       // 3D spatial fit (x0, y0, theta, phi)
       case 4: {
-        update = updateParameters<4>(resCfg.parsToUse.front(), cache,
-                                     result.parameters);
+        update = updateParameters<4>(pullCalculator.config().parsToUse.front(),
+                                     cache, result.parameters);
         break;
       }
       // full fit
       case 5: {
-        update = updateParameters<5>(resCfg.parsToUse.front(), cache,
-                                     result.parameters);
+        update = updateParameters<5>(pullCalculator.config().parsToUse.front(),
+                                     cache, result.parameters);
         break;
       }
       default:
         ACTS_WARNING(__func__ << "() " << __LINE__
                               << ": Invalid parameter size "
-                              << resCfg.parsToUse.size());
+                              << pullCalculator.config().parsToUse.size());
         return result;
     }
+    /// Check whether the fit parameters are within range
+    for (const FitParIndex par : pullCalculator.config().parsToUse) {
+      const auto p = toUnderlying(par);
+      if (m_cfg.ranges[p][0] < m_cfg.ranges[p][1] &&
+          (result.parameters[p] < m_cfg.ranges[p][0] ||
+           result.parameters[p] > m_cfg.ranges[p][1])) {
+        ACTS_VERBOSE(__func__ << "() " << __LINE__ << ": The parameter "
+                              << pullCalculator.parName(par) << " "
+                              << result.parameters[p] << " is out range ["
+                              << m_cfg.ranges[p][0] << ";" << m_cfg.ranges[p][1]
+                              << "]");
+        update = UpdateStep::outOfBounds;
+      }
+    }
     switch (update) {
       using enum UpdateStep;
       case converged: {
@@ -361,7 +379,8 @@ CompositeSpacePointLineFitter::fit(
   // Parameters converged
   if (result.converged) {
     const auto doF = countDoF(result.measurements, fitOpts.selector);
-    result.nDoF = (doF[0] + doF[1] + doF[2]) - resCfg.parsToUse.size();
+    result.nDoF =
+        (doF[0] + doF[1] + doF[2]) - pullCalculator.config().parsToUse.size();
     line.updateParameters(result.parameters);
     const double t0 = result.parameters[toUnderlying(FitParIndex::t0)];
     // Recalibrate the measurements before returning
@@ -370,29 +389,29 @@ CompositeSpacePointLineFitter::fit(
         result.measurements);
 
     // Assign the Hessian
-    switch (resCfg.parsToUse.size()) {
+    switch (pullCalculator.config().parsToUse.size()) {
       // 2D fit (intercept + inclination angle)
       case 2: {
-        fillCovariance<2>(resCfg.parsToUse.front(), cache.hessian,
-                          result.covariance);
+        fillCovariance<2>(pullCalculator.config().parsToUse.front(),
+                          cache.hessian, result.covariance);
         break;
       }
       // 2D fit + time
       case 3: {
-        fillCovariance<3>(resCfg.parsToUse.front(), cache.hessian,
-                          result.covariance);
+        fillCovariance<3>(pullCalculator.config().parsToUse.front(),
+                          cache.hessian, result.covariance);
         break;
       }
       // 3D spatial fit (x0, y0, theta, phi)
       case 4: {
-        fillCovariance<4>(resCfg.parsToUse.front(), cache.hessian,
-                          result.covariance);
+        fillCovariance<4>(pullCalculator.config().parsToUse.front(),
+                          cache.hessian, result.covariance);
         break;
       }
       // full fit
       case 5: {
-        fillCovariance<5>(resCfg.parsToUse.front(), cache.hessian,
-                          result.covariance);
+        fillCovariance<5>(pullCalculator.config().parsToUse.front(),
+                          cache.hessian, result.covariance);
         break;
       }
       // No need to warn here -> captured by the fit iterations