refactor: StrawLineSeeder - Use forwarding in constructor & minor clean (#4884)

junggjo9 · web-flow · commit c8f9806e1901 · 2026-01-05T07:20:51.000Z
PLEASE DESCRIBE YOUR CHANGES.
THIS MESSAGE ENDS UP AS THE COMMIT MESSAGE.
DO NOT USE @-MENTIONS HERE!
diff --git a/Core/include/Acts/Seeding/CompositeSpacePointLineSeeder.hpp b/Core/include/Acts/Seeding/CompositeSpacePointLineSeeder.hpp
@@ -43,8 +43,12 @@ concept CompositeSpacePointSeedFiller =
       /// @param t0: Offse in the time of arrival of the particle
       /// @param testSp: Reference to the straw space point to test
       {
-        filler.candidatePull(cctx, pos, dir, t0, testSp)
+        filler.candidateChi2(cctx, pos, dir, t0, testSp)
       } -> std::same_as<double>;
+      /// @brief Returns the radius of the straw tube in which the space point
+      ///        is recorded
+      /// @param testSp: Reference to the straw space point
+      { filler.strawRadius(testSp) } -> std::same_as<double>;
       /// @brief Creates a new empty container to construct a new segment seed
       /// @param cctx: Calibration context in case that the container shall be
       ///              part of a predefined memory block
@@ -81,8 +85,7 @@ concept CompositeSpacePointSeedFiller =
 /// the following attributes
 ///
 ///      using SpVec_t =  Standard container satisfiyng the
-/// CompositeSpacePointContainer concept
-///
+///                       CompositeSpacePointContainer concept
 ///
 ///      const std::vector<SpVec_t>& strawHits();
 ///      const std::vector<SpVec_t>& stripHits();
@@ -101,6 +104,8 @@ concept CompositeSpacePointSorter =
       } -> std::same_as<const std::vector<SpacePointCont_t>&>;
     };
 
+/// @brief Concept of the interface for the auxiliary class such that the
+///        CompositeSpacePointLineSeeder can construct segment seeds
 template <typename SeedAuxiliary_t, typename UnCalibCont_t,
           typename CalibCont_t>
 concept CompSpacePointSeederDelegate =
@@ -131,21 +136,36 @@ concept CompSpacePointSeederDelegate =
 ///        compatible strip measurements from each strip layer are added.
 class CompositeSpacePointLineSeeder {
  public:
+  /// @brief Use the assignment of the parameter indices from the CompSpacePointAuxiliaries
+  using ParIdx = detail::CompSpacePointAuxiliaries::FitParIndex;
+  /// @brief Use the assignment of the parameter indices from the CompSpacePointAuxiliaries
+  using CovIdx = detail::CompSpacePointAuxiliaries::ResidualIdx;
+  /// @brief Use the vector from the CompSpacePointAuxiliaires
+  using Vector = detail::CompSpacePointAuxiliaries::Vector;
+  /// @brief Vector containing the 5 straight segment line parameters
+  using SeedParam_t = std::array<double, toUnderlying(ParIdx::nPars)>;
+  /// @brief Abrivation of the straight line. The first element is the
+  ///        reference position and the second element is the direction
+  using Line_t = std::pair<Vector, Vector>;
+
   /// @brief Configuration of the cuts to sort out generated
   ///        seeds with poor quality.
   struct Config {
     /// @brief Cut on the theta angle
-    std::array<double, 2> thetaRange{0, 0};
+    std::array<double, 2> thetaRange{0., 0.};
     /// @brief Cut on the intercept range
-    std::array<double, 2> interceptRange{0, 0};
-
+    std::array<double, 2> interceptRange{0., 0.};
     /// @brief Upper cut on the hit chi2 w.r.t. seed in order to be associated to the seed
     double hitPullCut{5.};
     /// @brief How many drift circles may be on a layer to be used for seeding
     std::size_t busyLayerLimit{2};
     /// @brief Layers may contain measurements with bad hits and hence the
     bool busyLimitCountGood{true};
-
+    /// @brief Try at the first time the external seed parameters as candidate
+    bool startWithPattern{false};
+    /// @brief Use explicitly the line distance and the driftRadius to calculate
+    ///        the pull from the seed line to the space point.
+    bool useSimpleStrawPull{true};
     /// @brief How many drift circle hits needs the seed to contain in order to be valid
     std::size_t nStrawHitCut{3};
     /// @brief Hit cut based on the fraction of collected tube layers.
@@ -167,15 +187,6 @@ class CompositeSpacePointLineSeeder {
           "CompositeSpacePointLineSeeder", Logging::Level::INFO));
   /// @brief Return the configuration object of the seeder
   const Config& config() const { return m_cfg; }
-  /// @brief Use the assignment of the parameter indices from the CompSpacePointAuxiliaries
-  using ParIdx = detail::CompSpacePointAuxiliaries::FitParIndex;
-  /// @brief Use the vector from the CompSpacePointAuxiliaires
-  using Vector = detail::CompSpacePointAuxiliaries::Vector;
-  /// @brief Vector containing the 5 straight segment line parameters
-  using SeedParam_t = std::array<double, toUnderlying(ParIdx::nPars)>;
-  /// @brief Abrivation of the straight line. The first element is the
-  ///        reference position and the second element is the direction
-  using Line_t = std::pair<Vector, Vector>;
 
   /// @brief Enumeration to pick one of the four tangent lines to
   ///       the straw circle pair.
@@ -325,14 +336,16 @@ class CompositeSpacePointLineSeeder {
             detail::CompSpacePointSeederDelegate<UncalibCont_t, CalibCont_t>
                 Delegate_t>
   struct SeedingState : public Delegate_t {
-    /// @brief Use all constructors from the base class for this one as well
-    using Delegate_t::Delegate_t;
-    /// @brief radius of the straw tubes used to reject hits outside the tube
-    double strawRadius{0.};
-    /// @brief Try at the first time the external seed parameters as candidate
-    bool startWithPattern{false};
-    /// @brief Estimated parameters from pattern
-    SeedParam_t patternParams{};
+    /// @brief Declare the public constructor by explicitly forwarding the
+    ///        constructor arguments to the (protected) base class constructor
+    /// @param initialPars: Initial parameters from an external pattern seed
+    ///                     (Needed to combine the precision parameters with a
+    ///                     non-precision estimate)
+    /// @param args: Arguments to be forwarded to the base class constructor
+    template <typename... args_t>
+    explicit SeedingState(const SeedParam_t& initialPars, args_t&&... args)
+        : Delegate_t{std::forward<args_t>(args)...},
+          m_initialPars{initialPars} {}
     /// @brief Stringstream output operator
     friend std::ostream& operator<<(std::ostream& ostr,
                                     const SeedingState& opts) {
@@ -341,6 +354,8 @@ class CompositeSpacePointLineSeeder {
     }
     /// @brief Return the number of generated seeds
     std::size_t nGenSeeds() const { return m_seenSolutions.size(); }
+    /// @brief Returns the pattern parameters
+    const SeedParam_t& initialParameters() const { return m_initialPars; }
     /// @brief Grant the embedding class access to the private members
     friend CompositeSpacePointLineSeeder;
 
@@ -361,8 +376,13 @@ class CompositeSpacePointLineSeeder {
     std::size_t m_nStrawCut{0ul};
     /// @brief Flag toggling whether the upper of the lower layer shall be moved
     bool m_moveUpLayer{true};
+    /// @brief Flag toggling whether the pattern parameters shall be returned as
+    ///        first seed
+    bool m_patternSeedProduced{false};
     /// @brief Prints the seed solution to the screen
     void print(std::ostream& ostr) const;
+    /// @brief Estimated parameters from pattern
+    SeedParam_t m_initialPars{};
   };
   /// @brief Main interface method provided by the SeederClass. The user instantiates
   ///        a SeedingState object containing all the straw hit candidates from
diff --git a/Core/include/Acts/Seeding/CompositeSpacePointLineSeeder.ipp b/Core/include/Acts/Seeding/CompositeSpacePointLineSeeder.ipp
@@ -24,7 +24,6 @@ CompositeSpacePointLineSeeder::TwoCircleTangentPars
 CompositeSpacePointLineSeeder::constructTangentLine(const Sp_t& topHit,
                                                     const Sp_t& bottomHit,
                                                     const TangentAmbi ambi) {
-  using ResidualIdx = detail::CompSpacePointAuxiliaries::ResidualIdx;
   using namespace Acts::UnitLiterals;
   using namespace Acts::detail;
   TwoCircleTangentPars result{};
@@ -46,9 +45,9 @@ CompositeSpacePointLineSeeder::constructTangentLine(const Sp_t& topHit,
   const double dZ = D.dot(eZ);
 
   const double thetaTubes = std::atan2(dY, dZ);
-  const double distTubes = Acts::fastHypot(dY, dZ);
+  const double distTubes = fastHypot(dY, dZ);
   assert(distTubes > 1._mm);
-  constexpr auto covIdx = Acts::toUnderlying(ResidualIdx::bending);
+  constexpr auto covIdx = toUnderlying(CovIdx::bending);
   const double combDriftUncert{topHit.covariance()[covIdx] +
                                bottomHit.covariance()[covIdx]};
   const double R =
@@ -170,8 +169,8 @@ void CompositeSpacePointLineSeeder::SeedingState<
        << toString(encodeAmbiguity(s_signCombo[m_signComboIndex][0],
                                    s_signCombo[m_signComboIndex][1]))
        << "\n";
-  ostr << " start with pattern " << startWithPattern << " nGenSeeds "
-       << nGenSeeds() << " nStrawCut " << m_nStrawCut << "\n";
+  ostr << "Number of seeds " << nGenSeeds() << " nStrawCut " << m_nStrawCut
+       << "\n";
   if (nGenSeeds() > 0ul) {
     for (const auto& seen : m_seenSolutions) {
       ostr << "################################################" << std::endl;
@@ -288,17 +287,18 @@ CompositeSpacePointLineSeeder::nextSeed(
     state.m_nStrawCut = m_cfg.nStrawHitCut;
     /// Check whether the seeding can start with the external pattern
     /// parameters
-    if (state.startWithPattern &&
-        std::ranges::any_of(strawLayers, [&](const UncalibCont_t& layerHits) {
-          return countHits(layerHits, selector) > m_cfg.busyLayerLimit;
-        })) {
-      state.startWithPattern = false;
+    if (!state.m_patternSeedProduced &&
+        (!m_cfg.startWithPattern ||
+         std::ranges::any_of(strawLayers, [&](const UncalibCont_t& layerHits) {
+           return countHits(layerHits, selector) > m_cfg.busyLayerLimit;
+         }))) {
+      state.m_patternSeedProduced = true;
     }
-    if (state.startWithPattern) {
-      SegmentSeed<CalibCont_t> patternSeed{state.patternParams,
+    if (!state.m_patternSeedProduced) {
+      SegmentSeed<CalibCont_t> patternSeed{state.initialParameters(),
                                            state.newContainer(cctx)};
 
-      const auto [pos, dir] = makeLine(state.patternParams);
+      const auto [pos, dir] = makeLine(state.initialParameters());
       const double t0 = patternSeed.parameters[toUnderlying(ParIdx::t0)];
 
       auto append = [&](const StrawLayers_t<UncalibCont_t>& hitLayers) {
@@ -310,7 +310,7 @@ CompositeSpacePointLineSeeder::nextSeed(
       };
       append(strawLayers);
       append(state.stripHits());
-      state.startWithPattern = false;
+      state.m_patternSeedProduced = true;
       return patternSeed;
     }
     ACTS_DEBUG(__func__ << "() " << __LINE__ << " - Instantiate layers. ");
@@ -450,10 +450,6 @@ CompositeSpacePointLineSeeder::buildSeed(
 
   ACTS_DEBUG(__func__ << "() " << __LINE__ << " - Try to draw new seed from \n"
                       << state << ".");
-  if (state.strawRadius <= Acts::s_epsilon) {
-    throw std::invalid_argument(
-        "buildSeed() - Please configure the state's strawRadius");
-  }
   const auto& upperHit =
       *strawLayers.at(state.m_upperLayer.value()).at(state.m_upperHitIndex);
   const auto& lowerHit =
@@ -484,7 +480,7 @@ CompositeSpacePointLineSeeder::buildSeed(
     return std::nullopt;
   }
   /// Continue to construct a new solution
-  const double t0 = state.patternParams[toUnderlying(ParIdx::t0)];
+  const double t0 = state.initialParameters()[toUnderlying(ParIdx::t0)];
   SeedSolution<UncalibCont_t, Delegate_t> newSolution{seedPars, state};
 
   ACTS_DEBUG(__func__ << "() " << __LINE__
@@ -493,6 +489,9 @@ CompositeSpacePointLineSeeder::buildSeed(
   const Line_t tangentSeed{seedPars.y0 * Vector3::UnitY(),
                            makeDirection(lowerHit, seedPars.theta)};
   const auto& [seedPos, seedDir] = tangentSeed;
+  const double maxPullSq{Acts::square(m_cfg.hitPullCut)};
+  constexpr auto covIdx = Acts::toUnderlying(CovIdx::bending);
+
   for (const auto& [layerNr, hitsInLayer] :
        Acts::enumerate(state.strawHits())) {
     bool hadGoodHit{false};
@@ -504,17 +503,28 @@ CompositeSpacePointLineSeeder::buildSeed(
       // the hits are ordered in the layer so we assume that once we found good
       // hits we are moving away from the seed line so we can abort the hit
       // association
-      if (distance < state.strawRadius &&
-          state.candidatePull(cctx, seedPos, seedDir, t0, *testMe) <
-              Acts::pow(m_cfg.hitPullCut, 2u)) {
-        ACTS_VERBOSE(__func__ << "() " << __LINE__ << " - layer,hit = ("
-                              << layerNr << "," << hitNr
-                              << ") -> spacePoint: " << Acts::toString(*testMe)
-                              << " is close enough: " << distance);
-        hadGoodHit = true;
-        newSolution.append(layerNr, hitNr);
-        newSolution.nStrawHits += selector(*testMe);
-        continue;
+      const double rMax = state.strawRadius(*testMe);
+      assert(rMax > Acts::s_epsilon);
+      assert(testMe->covariance()[covIdx] > Acts::s_epsilon);
+      if (distance < rMax) {
+        const double pullSq =
+            m_cfg.useSimpleStrawPull
+                ? Acts::square(distance - Acts::abs(testMe->driftRadius())) /
+                      testMe->covariance()[covIdx]
+                : state.candidateChi2(cctx, seedPos, seedDir, t0, *testMe);
+
+        if (pullSq < maxPullSq) {
+          ACTS_VERBOSE(__func__
+                       << "() " << __LINE__ << " - layer,hit = (" << layerNr
+                       << "," << hitNr
+                       << ") -> spacePoint: " << Acts::toString(*testMe)
+                       << " is close enough: " << distance
+                       << ", pull: " << std::sqrt(pullSq));
+          hadGoodHit = true;
+          newSolution.append(layerNr, hitNr);
+          newSolution.nStrawHits += selector(*testMe);
+          continue;
+        }
       }
       if (hadGoodHit) {
         break;
@@ -539,7 +549,7 @@ CompositeSpacePointLineSeeder::consructSegmentSeed(
   ACTS_DEBUG(__func__ << "() " << __LINE__ << " Construct new seed from \n"
                       << newSolution);
   SegmentSeed<CalibCont_t> finalSeed{
-      combineWithPattern(tangentSeed, state.patternParams),
+      combineWithPattern(tangentSeed, state.initialParameters()),
       state.newContainer(cctx)};
   const auto [seedPos, seedDir] = makeLine(finalSeed.parameters);
   /// Append the collected straws to the seed
@@ -561,7 +571,7 @@ CompositeSpacePointLineSeeder::consructSegmentSeed(
     /// Find the hit with the lowest pull
     for (const auto& [hitIdx, testMe] : Acts::enumerate(stripLayerHits)) {
       const double chi2 =
-          state.candidatePull(cctx, seedPos, seedDir, t0, *testMe);
+          state.candidateChi2(cctx, seedPos, seedDir, t0, *testMe);
       if (testMe->measuresLoc0() && chi2 < bestChi2Loc0) {
         bestChi2Loc0 = chi2;
         bestIdxLoc0 = hitIdx;
diff --git a/Tests/UnitTests/Core/Seeding/StrawHitGeneratorHelper.hpp b/Tests/UnitTests/Core/Seeding/StrawHitGeneratorHelper.hpp
@@ -391,7 +391,10 @@ static_assert(
 
 /// @brief Split the composite space point container into straw and strip measurements
 class SpSorter {
- public:
+ protected:
+  /// @brief Protected constructor to instantiate the SpSorter.
+  /// @param hits: List of space points to sort per layer
+  /// @param calibrator: Space point calibrator to recalibrate the hits
   SpSorter(const Container_t& hits, const SpCalibrator* calibrator)
       : m_calibrator{calibrator} {
     for (const auto& spPtr : hits) {
@@ -402,9 +405,14 @@ class SpSorter {
       pushMe[spPtr->layer()].push_back(spPtr);
     }
   }
+
+ public:
+  /// @brief Return the sorted straw hits
   const std::vector<Container_t>& strawHits() const { return m_straws; }
+  /// @brief Returns the sorted strip hits
   const std::vector<Container_t>& stripHits() const { return m_strips; }
-
+  /// @brief Returns whether the candidate space point is a good hit or not
+  /// @param testPoint: Hit to test
   bool goodCandidate(const FitTestSpacePoint& testPoint) const {
     return testPoint.isGood();
   }
@@ -417,7 +425,7 @@ class SpSorter {
   /// @param dir: Direction of the candidate seed line
   /// @param t0: Offse in the time of arrival of the particle
   /// @param testSp: Reference to the straw space point to test
-  double candidatePull(const CalibrationContext& /* cctx*/, const Vector3& pos,
+  double candidateChi2(const CalibrationContext& /* cctx*/, const Vector3& pos,
                        const Vector3& dir, const double /*t0*/,
                        const FitTestSpacePoint& testSp) const {
     return CompSpacePointAuxiliaries::chi2Term(pos, dir, testSp);
@@ -440,6 +448,9 @@ class SpSorter {
                    const std::size_t upperLayer) const {
     return lowerLayer >= upperLayer;
   }
+  double strawRadius(const FitTestSpacePoint& /*testSp*/) const {
+    return 15._mm;
+  }
 
  private:
   const SpCalibrator* m_calibrator{nullptr};
diff --git a/Tests/UnitTests/Core/Seeding/StrawLineSeederTest.cpp b/Tests/UnitTests/Core/Seeding/StrawLineSeederTest.cpp
@@ -67,13 +67,13 @@ void testSeeder(RandomEngine& engine, TFile& outFile) {
     using SeedState_t =
         CompositeSpacePointLineSeeder::SeedingState<Container_t, Container_t,
                                                     SpSorter>;
-    SeedState_t seedOpts{testTubes, calibrator.get()};
-    seedOpts.strawRadius = 15._mm;
-    ACTS_DEBUG(seedOpts);
+    SeedState_t seedState{startParameters(line, testTubes), testTubes,
+                          calibrator.get()};
+    ACTS_DEBUG(seedState);
     nSeeds = 0;
     CalibrationContext cctx{};
-    while (auto seed = seeder.nextSeed(cctx, seedOpts)) {
-      ACTS_DEBUG("Seed finder loop " << seedOpts);
+    while (auto seed = seeder.nextSeed(cctx, seedState)) {
+      ACTS_DEBUG("Seed finder loop " << seedState);
       if (seed == std::nullopt) {
         break;
       }
@@ -192,7 +192,7 @@ BOOST_AUTO_TEST_CASE(SeedTangents) {
             Seeder::constructTangentLine(topTube, bottomTube, trueAmbi);
         /// Construct line parameters
         ACTS_DEBUG(__func__
-                   << "() " << __LINE__ << " - Line tan theta: " << lineTanBeta
+                   << "() " << __LINE__ << " - Line tan beta: " << lineTanBeta
                    << ", reconstructed tan theta: " << std::tan(seedPars.theta)
                    << ", line y0: " << lineY0
                    << ", reconstructed y0: " << seedPars.y0);
