From 85a59e6341303a48ff01a949193f0365dd267b66 Mon Sep 17 00:00:00 2001 From: Johannes Junggeburth Date: Sat, 11 Oct 2025 16:03:12 +0200 Subject: [PATCH 1/3] Insert parameter check --- .../Seeding/CompositeSpacePointLineFitter.hpp | 25 ++++---- .../Seeding/CompositeSpacePointLineFitter.ipp | 61 ++++++++++++------- 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp b/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp index 155db87e067..fb3ed8f0b0c 100644 --- a/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp +++ b/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp @@ -86,15 +86,6 @@ class CompositeSpacePointLineFitter { using RangeArray = std::array, 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 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 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 @@ -180,7 +181,9 @@ class CompositeSpacePointLineFitter { /// nonBending, bending & time static std::vector extractFitablePars( const std::array& hitCounts); - + /// @brief Fit a line to a set of Composite space point measurements. + /// @param FitOptions: Auxiliary object carrying all necessary input + /// needed to execute the fit template Calibrator_t> FitResult fit(FitOptions&& fitOpts) const; @@ -206,7 +209,7 @@ class CompositeSpacePointLineFitter { template FitParameters fastFit(const Cont_t& measurements, const Line_t& initialGuess, const std::vector& parsToUse) const; - + /// @brief Abrivation of the fit result returned by the FastStrawLineFitter using FastFitResult = std::optional; /// @brief Executes the fast line fit in the bending direction. Returns diff --git a/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp b/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp index 0ea19d90cf9..3d5583697d9 100644 --- a/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp +++ b/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp @@ -31,6 +31,10 @@ std::array 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; @@ -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 From 9b3843133bf9a98b3f307943744d06a06c02d0b2 Mon Sep 17 00:00:00 2001 From: Johannes Junggeburth Date: Sat, 11 Oct 2025 19:01:23 +0200 Subject: [PATCH 2/3] Update doxygen --- Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp b/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp index fb3ed8f0b0c..95d7c5133dc 100644 --- a/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp +++ b/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.hpp @@ -182,8 +182,8 @@ class CompositeSpacePointLineFitter { static std::vector extractFitablePars( const std::array& hitCounts); /// @brief Fit a line to a set of Composite space point measurements. - /// @param FitOptions: Auxiliary object carrying all necessary input - /// needed to execute the fit + /// @param fitOpts: Auxiliary object carrying all necessary input + /// needed to execute the fit template Calibrator_t> FitResult fit(FitOptions&& fitOpts) const; From 65734edea62a750e199fd19dd19e688a3f972a62 Mon Sep 17 00:00:00 2001 From: Johannes Junggeburth Date: Sun, 12 Oct 2025 11:59:05 +0200 Subject: [PATCH 3/3] Return the precision fit result for straws --- Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp b/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp index 3d5583697d9..ccb48b6881c 100644 --- a/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp +++ b/Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp @@ -100,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);