refactor: Refactor loop protection (#2535)

The loop protection should not be direction dependent anymore. The limit should be set even if it is higher. Pulled this out of #2518
acts-project · Oct 16, 2023 · b1ef696 · b1ef696
1 parent 37e1543
commit b1ef696
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 11 deletions.
diff --git a/CI/physmon/workflows/physmon_track_finding_ttbar.py b/CI/physmon/workflows/physmon_track_finding_ttbar.py
@@ -45,7 +45,7 @@
         ),
         acts.examples.Sequencer.FpeMask(
             "Examples/Algorithms/Fatras/src/FatrasSimulation.cpp",
-            (172, 173),
+            (235, 236),
             acts.FpeType.FLTOVF,
             1,
         ),

diff --git a/Core/include/Acts/Propagator/Propagator.ipp b/Core/include/Acts/Propagator/Propagator.ipp
@@ -160,7 +160,7 @@ auto Acts::Propagator<S, N>::propagate(
   // Apply the loop protection - it resets the internal path limit
   detail::setupLoopProtection(
       state, m_stepper, state.options.abortList.template get<path_aborter_t>(),
-      logger());
+      false, logger());
   // Perform the actual propagation & check its outcome
   auto result = propagate_impl<ResultType>(state, inputResult);
   if (result.ok()) {
@@ -249,7 +249,7 @@ auto Acts::Propagator<S, N>::propagate(
   // Apply the loop protection, it resets the internal path limit
   detail::setupLoopProtection(
       state, m_stepper, state.options.abortList.template get<path_aborter_t>(),
-      logger());
+      false, logger());
 
   // Perform the actual propagation
   auto result = propagate_impl<ResultType>(state, inputResult);

diff --git a/Core/include/Acts/Propagator/detail/LoopProtection.hpp b/Core/include/Acts/Propagator/detail/LoopProtection.hpp
@@ -18,11 +18,17 @@ namespace detail {
 template <typename path_aborter_t, typename propagator_state_t,
           typename stepper_t>
 void setupLoopProtection(propagator_state_t& state, const stepper_t& stepper,
-                         path_aborter_t& pathAborter, const Logger& logger) {
+                         path_aborter_t& pathAborter, bool releaseLimit,
+                         const Logger& logger) {
   if (!state.options.loopProtection) {
     return;
   }
 
+  if (releaseLimit) {
+    pathAborter.internalLimit =
+        state.options.direction * std::numeric_limits<double>::max();
+  }
+
   // Get the field at the start position
   auto fieldRes =
       stepper.getField(state.stepping, stepper.position(state.stepping));
@@ -32,7 +38,7 @@ void setupLoopProtection(propagator_state_t& state, const stepper_t& stepper,
     ACTS_WARNING("Field lookup was unsuccessful, this is very likely an error");
     return;
   }
-  Vector3 field = *fieldRes;
+  const Vector3 field = *fieldRes;
   const double B = field.norm();
   if (B == 0) {
     return;
@@ -43,13 +49,18 @@ void setupLoopProtection(propagator_state_t& state, const stepper_t& stepper,
   // Calculate the full helix path
   const double helixPath = state.options.direction * 2 * M_PI * p / B;
   // And set it as the loop limit if it overwrites the internal limit
-  double loopLimit = state.options.loopFraction * helixPath;
-  double pathLimit = pathAborter.internalLimit;
-  if (std::abs(loopLimit) < std::abs(pathLimit)) {
+  const double loopLimit = state.options.loopFraction * helixPath;
+  const double previousLimit = pathAborter.internalLimit;
+  if (std::abs(loopLimit) < std::abs(previousLimit)) {
     pathAborter.internalLimit = loopLimit;
 
     ACTS_VERBOSE("Path aborter limit set to "
-                 << loopLimit << " (full helix =  " << helixPath << ")");
+                 << loopLimit << " (full helix = " << helixPath
+                 << ", previous limit = " << previousLimit << ")");
+  } else {
+    ACTS_VERBOSE("Path aborter limit not updated to "
+                 << loopLimit << " (full helix = " << helixPath
+                 << ", previous limit = " << previousLimit << ")");
   }
 }
 

diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp
@@ -615,7 +615,7 @@ class CombinatorialKalmanFilter {
 
       detail::setupLoopProtection(
           state, stepper, result.abortList.template get<PathLimitReached>(),
-          logger());
+          true, logger());
     }
 
     /// @brief CombinatorialKalmanFilter actor operation:

diff --git a/Tests/UnitTests/Core/Propagator/LoopProtectionTests.cpp b/Tests/UnitTests/Core/Propagator/LoopProtectionTests.cpp
@@ -149,7 +149,7 @@ BOOST_DATA_TEST_CASE(
   auto initialLimit = pathLimit.internalLimit;
 
   detail::setupLoopProtection(
-      pState, pStepper, pathLimit,
+      pState, pStepper, pathLimit, false,
       *Acts::getDefaultLogger("LoopProt", Logging::INFO));
 
   auto updatedLimit =