refactor: Backport navigation rewrite changes (acts-project#2846)

Backporting more changes from acts-project#2625 to main. This is a followup to acts-project#2768 

Summary
- remove overstepping from steppers
- use surface tolerance for geometry lookups
- add self consistency navigation test

Core/include/Acts/Geometry/Layer.hpp

@@ -35,7 +35,7 @@ class VolumeBounds;
 class TrackingVolume;
 class ApproachDescriptor;
 class IMaterialDecorator;
-template <typename T>
+template <typename object_t>
 struct NavigationOptions;
 
 // Simple surface intersection

Core/include/Acts/Propagator/EigenStepper.hpp

@@ -162,8 +162,7 @@ class EigenStepper {
   };
 
   /// Constructor requires knowledge of the detector's magnetic field
-  EigenStepper(std::shared_ptr<const MagneticFieldProvider> bField,
-               double overstepLimit = 100 * UnitConstants::um);
+  EigenStepper(std::shared_ptr<const MagneticFieldProvider> bField);
 
   State makeState(std::reference_wrapper<const GeometryContext> gctx,
                   std::reference_wrapper<const MagneticFieldContext> mctx,
@@ -317,12 +316,6 @@ class EigenStepper {
     return state.stepSize.toString();
   }
 
-  /// Overstep limit
-  double overstepLimit(const State& /*state*/) const {
-    // A dynamic overstep limit could sit here
-    return -m_overstepLimit;
-  }
-
   /// Create and return the bound state at the current position
   ///
   /// @brief This transports (if necessary) the covariance
@@ -435,9 +428,6 @@ class EigenStepper {
  protected:
   /// Magnetic field inside of the detector
   std::shared_ptr<const MagneticFieldProvider> m_bField;
-
-  /// Overstep limit
-  double m_overstepLimit{};
 };
 
 template <typename navigator_t>

Core/include/Acts/Propagator/EigenStepper.ipp

@@ -15,8 +15,8 @@
 
 template <typename E, typename A>
 Acts::EigenStepper<E, A>::EigenStepper(
-    std::shared_ptr<const MagneticFieldProvider> bField, double overstepLimit)
-    : m_bField(std::move(bField)), m_overstepLimit(overstepLimit) {}
+    std::shared_ptr<const MagneticFieldProvider> bField)
+    : m_bField(std::move(bField)) {}
 
 template <typename E, typename A>
 auto Acts::EigenStepper<E, A>::makeState(

Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp

@@ -726,14 +726,6 @@ class MultiEigenStepperLoop
     return ss.str();
   }
 
-  /// Overstep limit
-  ///
-  /// @param state [in] The stepping state (thread-local cache)
-  double overstepLimit(const State& state) const {
-    // A dynamic overstep limit could sit here
-    return SingleStepper::overstepLimit(state.components.front().state);
-  }
-
   /// Create and return the bound state at the current position
   ///
   /// @brief This transports (if necessary) the covariance

Core/include/Acts/Propagator/Navigator.hpp

@@ -9,7 +9,6 @@
 #pragma once
 
 #include "Acts/Definitions/Units.hpp"
-#include "Acts/Geometry/BoundarySurfaceT.hpp"
 #include "Acts/Geometry/GeometryIdentifier.hpp"
 #include "Acts/Geometry/Layer.hpp"
 #include "Acts/Geometry/TrackingGeometry.hpp"
@@ -19,8 +18,6 @@
 #include "Acts/Utilities/Logger.hpp"
 #include "Acts/Utilities/StringHelpers.hpp"
 
-#include <iomanip>
-#include <iterator>
 #include <sstream>
 #include <string>
 
@@ -31,7 +28,6 @@ namespace Acts {
 /// @brief struct for the Navigation options that are forwarded to
 ///        the geometry
 ///
-/// @tparam propagator_state_t Type of the object for navigation state
 /// @tparam object_t Type of the object for navigation to check against
 template <typename object_t>
 struct NavigationOptions {
@@ -46,9 +42,9 @@ struct NavigationOptions {
   /// always look for passive
   bool resolvePassive = false;
 
-  /// object to check against: at start
+  /// Hint for start object
   const object_t* startObject = nullptr;
-  /// object to check against: at end
+  /// Hint for end object
   const object_t* endObject = nullptr;
 
   /// External surface identifier for which the boundary check is ignored
@@ -856,7 +852,13 @@ class Navigator {
               state.geoContext, stepper.position(state.stepping),
               state.options.direction * stepper.direction(state.stepping),
               navOpts, logger());
-      // The number of boundary candidates
+      std::sort(state.navigation.navBoundaries.begin(),
+                state.navigation.navBoundaries.end(),
+                [](const auto& a, const auto& b) {
+                  return SurfaceIntersection::pathLengthOrder(a.first, b.first);
+                });
+
+      // Print boundary information
       if (logger().doPrint(Logging::VERBOSE)) {
         std::ostringstream os;
         os << state.navigation.navBoundaries.size();
@@ -866,6 +868,7 @@ class Navigator {
         }
         logger().log(Logging::VERBOSE, os.str());
       }
+
       // Set the begin index
       state.navigation.navBoundaryIndex = 0;
       if (!state.navigation.navBoundaries.empty()) {
@@ -1049,18 +1052,23 @@ class Navigator {
     state.navigation.navSurfaces = currentLayer->compatibleSurfaces(
         state.geoContext, stepper.position(state.stepping),
         state.options.direction * stepper.direction(state.stepping), navOpts);
-    // the number of layer candidates
-    if (!state.navigation.navSurfaces.empty()) {
-      if (logger().doPrint(Logging::VERBOSE)) {
-        std::ostringstream os;
-        os << state.navigation.navSurfaces.size();
-        os << " surface candidates found at path(s): ";
-        for (auto& sfc : state.navigation.navSurfaces) {
-          os << sfc.pathLength() << "  ";
-        }
-        logger().log(Logging::VERBOSE, os.str());
+    std::sort(state.navigation.navSurfaces.begin(),
+              state.navigation.navSurfaces.end(),
+              SurfaceIntersection::pathLengthOrder);
+
+    // Print surface information
+    if (logger().doPrint(Logging::VERBOSE)) {
+      std::ostringstream os;
+      os << state.navigation.navSurfaces.size();
+      os << " surface candidates found at path(s): ";
+      for (auto& sfc : state.navigation.navSurfaces) {
+        os << sfc.pathLength() << "  ";
       }
+      logger().log(Logging::VERBOSE, os.str());
+    }
 
+    // Surface candidates have been found
+    if (!state.navigation.navSurfaces.empty()) {
       // set the index
       state.navigation.navSurfaceIndex = 0;
       // The stepper updates the step size ( single / multi component)
@@ -1070,6 +1078,7 @@ class Navigator {
                                   << stepper.outputStepSize(state.stepping));
       return true;
     }
+
     state.navigation.navSurfaceIndex = state.navigation.navSurfaces.size();
     ACTS_VERBOSE(volInfo(state) << "No surface candidates found.");
     return false;
@@ -1104,25 +1113,32 @@ class Navigator {
     navOpts.nearLimit = state.options.surfaceTolerance;
     navOpts.farLimit =
         stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
+
     // Request the compatible layers
     state.navigation.navLayers =
         state.navigation.currentVolume->compatibleLayers(
             state.geoContext, stepper.position(state.stepping),
             state.options.direction * stepper.direction(state.stepping),
             navOpts);
+    std::sort(state.navigation.navLayers.begin(),
+              state.navigation.navLayers.end(),
+              [](const auto& a, const auto& b) {
+                return SurfaceIntersection::pathLengthOrder(a.first, b.first);
+              });
+
+    // Print layer information
+    if (logger().doPrint(Logging::VERBOSE)) {
+      std::ostringstream os;
+      os << state.navigation.navLayers.size();
+      os << " layer candidates found at path(s): ";
+      for (auto& lc : state.navigation.navLayers) {
+        os << lc.first.pathLength() << "  ";
+      }
+      logger().log(Logging::VERBOSE, os.str());
+    }
 
     // Layer candidates have been found
     if (!state.navigation.navLayers.empty()) {
-      // Screen output where they are
-      if (logger().doPrint(Logging::VERBOSE)) {
-        std::ostringstream os;
-        os << state.navigation.navLayers.size();
-        os << " layer candidates found at path(s): ";
-        for (auto& lc : state.navigation.navLayers) {
-          os << lc.first.pathLength() << "  ";
-        }
-        logger().log(Logging::VERBOSE, os.str());
-      }
       // Set the index to the first
       state.navigation.navLayerIndex = 0;
       // Setting the step size towards first

Core/include/Acts/Propagator/StandardAborters.hpp

@@ -72,10 +72,10 @@ struct SurfaceReached {
   /// Distance limit to discard intersections "behind us"
   /// @note this is only necessary because some surfaces have more than one
   ///       intersection
-  std::optional<double> overrideNearLimit;
+  double nearLimit = -100 * UnitConstants::um;
 
   SurfaceReached() = default;
-  SurfaceReached(double nearLimit) : overrideNearLimit(nearLimit) {}
+  SurfaceReached(double nLimit) : nearLimit(nLimit) {}
 
   /// boolean operator for abort condition without using the result
   ///
@@ -101,14 +101,11 @@ struct SurfaceReached {
       return true;
     }
 
-    // not blindly using the stepper overstep limit here because it does not
-    // always work for perigee surfaces.
+    // not using the stepper overstep limit here because it does not always work
+    // for perigee surfaces
     // note: the near limit is necessary for surfaces with more than one
-    // intersections in order to discard the ones which are behind us.
-    const double nearLimit =
-        overrideNearLimit.value_or(stepper.overstepLimit(state.stepping));
-    const double farLimit =
-        state.stepping.stepSize.value(ConstrainedStep::aborter);
+    // intersection in order to discard the ones which are behind us
+    const double farLimit = std::numeric_limits<double>::max();
     const double tolerance = state.options.surfaceTolerance;
 
     const auto sIntersection = surface->intersect(

Core/include/Acts/Propagator/StepperConcept.hpp

@@ -44,7 +44,6 @@ METHOD_TRAIT(absolute_momentum_t, absoluteMomentum);
 METHOD_TRAIT(momentum_t, momentum);
 METHOD_TRAIT(charge_t, charge);
 METHOD_TRAIT(time_t, time);
-METHOD_TRAIT(overstep_t, overstepLimit);
 METHOD_TRAIT(bound_state_method_t, boundState);
 METHOD_TRAIT(curvilinear_state_method_t, curvilinearState);
 METHOD_TRAIT(update_t, update);
@@ -114,8 +113,6 @@ constexpr bool MultiStepperStateConcept= require<
         static_assert(charge_exists, "charge method not found");
         constexpr static bool time_exists = has_method<const S, double, time_t, const state&>;
         static_assert(time_exists, "time method not found");
-        constexpr static bool overstep_exists = has_method<const S, double, overstep_t, const state&>;
-        static_assert(overstep_exists, "overstepLimit method not found");
         constexpr static bool bound_state_method_exists= has_method<const S, Result<typename S::BoundState>, bound_state_method_t, state&, const Surface&, bool, const FreeToBoundCorrection&>;
         static_assert(bound_state_method_exists, "boundState method not found");
         constexpr static bool curvilinear_state_method_exists = has_method<const S, typename S::CurvilinearState, curvilinear_state_method_t, state&, bool>;

Core/include/Acts/Propagator/StraightLineStepper.hpp

@@ -218,11 +218,6 @@ class StraightLineStepper {
   /// @param state [in] The stepping state (thread-local cache)
   double time(const State& state) const { return state.pars[eFreeTime]; }
 
-  /// Overstep limit
-  double overstepLimit(const State& /*state*/) const {
-    return -m_overstepLimit;
-  }
-
   /// Update surface status
   ///
   /// This method intersects the provided surface and update the navigation
@@ -451,9 +446,6 @@ class StraightLineStepper {
     // return h
     return h;
   }
-
- private:
-  double m_overstepLimit = s_onSurfaceTolerance;
 };
 
 template <typename navigator_t>

Core/include/Acts/Propagator/detail/SteppingHelper.hpp

@@ -17,6 +17,8 @@
 #include "Acts/Utilities/Intersection.hpp"
 #include "Acts/Utilities/Logger.hpp"
 
+#include <limits>
+
 namespace Acts::detail {
 
 /// Update surface status - Single component
@@ -50,8 +52,7 @@ Acts::Intersection3D::Status updateSingleSurfaceStatus(
     return Intersection3D::Status::onSurface;
   }
 
-  // Path and overstep limit checking
-  const double nearLimit = stepper.overstepLimit(state);
+  const double nearLimit = std::numeric_limits<double>::lowest();
   const double farLimit = state.stepSize.value(ConstrainedStep::aborter);
 
   if (sIntersection && detail::checkIntersection(sIntersection.intersection(),

Core/include/Acts/Utilities/Intersection.hpp

@@ -217,6 +217,10 @@ class ObjectMultiIntersection {
     return {m_intersections[index], m_object, index};
   }
 
+  constexpr const MultiIntersection3D& intersections() const {
+    return m_intersections;
+  }
+
   constexpr std::size_t size() const { return m_intersections.size(); }
 
   constexpr const object_t* object() const { return m_object; }
@@ -274,7 +278,7 @@ bool checkIntersection(const intersection_t& intersection, double nearLimit,
                << nearLimit << ", " << farLimit << ", " << distance);
 
   const bool coCriterion = distance > nearLimit;
-  const bool cpCriterion = std::abs(distance) < std::abs(farLimit) + tolerance;
+  const bool cpCriterion = distance < farLimit + tolerance;
 
   const bool accept = coCriterion && cpCriterion;
 
@@ -288,9 +292,9 @@ bool checkIntersection(const intersection_t& intersection, double nearLimit,
     }
     if (!cpCriterion) {
       ACTS_VERBOSE("- intersection path length "
-                   << std::abs(distance) << " is over the far limit "
-                   << (std::abs(farLimit) + tolerance)
-                   << " (including tolerance of " << tolerance << ")");
+                   << distance << " is over the far limit "
+                   << (farLimit + tolerance) << " (including tolerance of "
+                   << tolerance << ")");
     }
   }