refactor!: Generalize navigation options for external surfaces (#5243)

Generalizes external surfaces from `Navigator::Options` to `NavigatorPlainOptions` which is the base for all our navigators. This way all navigators fulfill the interface necessary to deal with external surfaces. The `DirectNavigator` now also uses this central external surface list.

Also
- adapt `Navigator` to handle the new option vs the previous 2 lists
- rename `Navigator::State::reset` to `resetForRenavigation`
- rename `insertExternalSurface` to `appendExternalSurface` to hit that order is important
- improve `appendExternalSurface` doc
- downstream changes

Author: andiwand

Core/include/Acts/Propagator/DirectNavigator.hpp

@@ -25,7 +25,6 @@
 
 #include <limits>
 #include <memory>
-#include <vector>
 
 namespace Acts {
 
@@ -39,9 +38,6 @@ namespace Acts {
 ///
 class DirectNavigator {
  public:
-  /// The sequentially crossed surfaces
-  using SurfaceSequence = std::vector<const Surface*>;
-
   /// @brief The nested configuration struct
   struct Config {};
 
@@ -52,9 +48,6 @@ class DirectNavigator {
     explicit Options(const GeometryContext& gctx)
         : NavigatorPlainOptions(gctx) {}
 
-    /// The Surface sequence
-    SurfaceSequence surfaces;
-
     /// The surface tolerance
     double surfaceTolerance = s_onSurfaceTolerance;
 
@@ -93,7 +86,7 @@ class DirectNavigator {
     /// Index of the next surface to try
     /// @note -1 means before the first surface in the sequence and size()
     ///       means after the last surface in the sequence
-    int surfaceIndex = -1;
+    std::int32_t surfaceIndex = -1;
 
     /// Navigation state - external interface: the current surface
     const Surface* currentSurface = nullptr;
@@ -107,7 +100,7 @@ class DirectNavigator {
     /// Get the current navigation surface
     /// @return Reference to the surface at the current surface index
     const Surface& navSurface() const {
-      return *options.surfaces.at(surfaceIndex);
+      return *options.externalSurfaces.at(surfaceIndex);
     }
 
     /// Move to the next surface in the sequence
@@ -124,7 +117,8 @@ class DirectNavigator {
     /// @return True if no more surfaces remain in the propagation direction
     bool endOfSurfaces() const {
       if (direction == Direction::Forward()) {
-        return surfaceIndex >= static_cast<int>(options.surfaces.size());
+        return surfaceIndex >=
+               static_cast<int>(options.externalSurfaces.size());
       }
       return surfaceIndex < 0;
     }
@@ -133,7 +127,7 @@ class DirectNavigator {
     /// @return Number of surfaces left to process in the propagation direction
     int remainingSurfaces() const {
       if (direction == Direction::Forward()) {
-        return options.surfaces.size() - surfaceIndex;
+        return options.externalSurfaces.size() - surfaceIndex;
       }
       return surfaceIndex + 1;
     }
@@ -144,7 +138,7 @@ class DirectNavigator {
     void resetSurfaceIndex() {
       surfaceIndex = direction == Direction::Forward()
                          ? -1
-                         : static_cast<int>(options.surfaces.size());
+                         : static_cast<int>(options.externalSurfaces.size());
     }
   };
 
@@ -232,7 +226,7 @@ class DirectNavigator {
     static_cast<void>(direction);
 
     ACTS_VERBOSE("Initialize. Surface sequence for navigation:");
-    for (const Surface* surface : state.options.surfaces) {
+    for (const Surface* surface : state.options.externalSurfaces) {
       ACTS_VERBOSE(surface->geometryId()
                    << " - "
                    << surface->center(state.options.geoContext).transpose());
@@ -251,13 +245,14 @@ class DirectNavigator {
     }
 
     // Find initial index.
-    auto found =
-        std::ranges::find(state.options.surfaces, state.options.startSurface);
+    auto found = std::ranges::find(state.options.externalSurfaces,
+                                   state.options.startSurface);
 
-    if (found != state.options.surfaces.end()) {
+    if (found != state.options.externalSurfaces.end()) {
       // The index should be the index before the start surface, depending on
       // the direction
-      state.surfaceIndex = std::distance(state.options.surfaces.begin(), found);
+      state.surfaceIndex =
+          std::distance(state.options.externalSurfaces.begin(), found);
       state.surfaceIndex += state.direction == Direction::Backward() ? 1 : -1;
     } else {
       ACTS_DEBUG(
@@ -300,7 +295,7 @@ class DirectNavigator {
       ACTS_VERBOSE("Next surface candidate is "
                    << state.navSurface().geometryId() << ". "
                    << state.remainingSurfaces() << " out of "
-                   << state.options.surfaces.size()
+                   << state.options.externalSurfaces.size()
                    << " surfaces remain to try.");
 
       // Establish & update the surface status

Core/include/Acts/Propagator/Navigator.hpp

@@ -8,7 +8,6 @@
 
 #pragma once
 
-#include "Acts/Definitions/Tolerance.hpp"
 #include "Acts/Geometry/GeometryIdentifier.hpp"
 #include "Acts/Geometry/Layer.hpp"
 #include "Acts/Geometry/TrackingGeometry.hpp"
@@ -94,9 +93,6 @@ class Navigator final {
   using NavigationCandidates =
       boost::container::small_vector<NavigationTarget, 10>;
 
-  /// Type alias for external surfaces container
-  using ExternalSurfaces = std::vector<GeometryIdentifier>;
-
   /// Type alias for geometry version enumeration
   using GeometryVersion = TrackingGeometry::GeometryVersion;
 
@@ -128,21 +124,6 @@ class Navigator final {
     explicit Options(const GeometryContext& gctx)
         : NavigatorPlainOptions(gctx) {}
 
-    /// Externally provided surfaces - these are tried to be hit
-    ExternalSurfaces externalSurfaces = {};
-    /// Surfaces that are not part of the tracking geometry
-    std::vector<const Surface*> freeSurfaces = {};
-
-    /// Insert an external surface to be considered during navigation
-    /// @param surface: The surface to add to the list
-    void insertExternalSurface(const Surface& surface) {
-      if (surface.geometryId() != GeometryIdentifier{}) {
-        externalSurfaces.push_back(surface.geometryId());
-      } else {
-        freeSurfaces.push_back(&surface);
-      }
-    }
-
     /// Set the plain navigation options
     /// @param options The plain navigator options to set
     void setPlainOptions(const NavigatorPlainOptions& options) {
@@ -243,6 +224,9 @@ class Navigator final {
     /// Stream for navigation debugging and monitoring
     NavigationStream stream;
 
+    /// Surfaces that are not part of the tracking geometry
+    std::vector<const Surface*> freeSurfaces;
+
     /// Reset navigation state after switching layers
     void resetAfterLayerSwitch() {
       navSurfaces.clear();
@@ -266,19 +250,22 @@ class Navigator final {
     }
 
     /// Completely reset navigation state to initial conditions
-    void reset() {
+    void resetForRenavigation() {
       resetAfterVolumeSwitch();
 
       currentVolume = nullptr;
       currentSurface = nullptr;
 
       navigationBreak = false;
       navigationStage = Stage::initial;
+
       // Set the surface reached switches back to false
       std::ranges::for_each(freeCandidates,
                             [](std::pair<const Surface*, bool>& freeSurface) {
                               freeSurface.second = false;
                             });
+
+      stream.reset();
     }
   };
 

Core/include/Acts/Propagator/NavigatorOptions.hpp

@@ -48,16 +48,29 @@ struct NavigatorPlainOptions {
   /// stream given the current volume and the track coordinates. If the
   /// delegate is set, it is called in each candidate resolution step
   /// for each surface that has not been marked as reached yet.
-  /// @param gctx: Current geometry context carrying the alignment information
-  /// @param currentVol: The current tracking volume in which the propagator resides
-  /// @param pos: Position of the track in global coordinates
-  /// @param dir: Direction vector of the track
-  /// @param surface: Free surface candidate to test
-  using FreeSurfaceSelctor_t = Delegate<bool(
+  /// @param gctx Current geometry context carrying the alignment information
+  /// @param currentVol The current tracking volume in which the propagator resides
+  /// @param pos Position of the track in global coordinates
+  /// @param dir Direction vector of the track
+  /// @param surface Free surface candidate to test
+  using FreeSurfaceSelctor = Delegate<bool(
       const GeometryContext& gctx, const TrackingVolume& currentVol,
       const Vector3& pos, const Vector3& dir, const Surface& candidate)>;
+
   /// Delegate for selecting free surfaces during navigation
-  FreeSurfaceSelctor_t freeSurfaceSelector{};
+  FreeSurfaceSelctor freeSurfaceSelector;
+
+  /// Surfaces that are not part of the tracking geometry
+  std::vector<const Surface*> externalSurfaces;
+
+  /// Append an external surface to be considered during navigation. The surface
+  /// will be intersected without bounds check to force the propagation to
+  /// resolve it. Note the surface must outlive the propagation and surfaces
+  /// have to be added in the order they should be considered during navigation.
+  /// @param surface The surface to add to the list
+  void appendExternalSurface(const Surface& surface) {
+    externalSurfaces.push_back(&surface);
+  }
 };
 
 }  // namespace Acts

Core/include/Acts/TrackFitting/GaussianSumFitter.hpp

@@ -116,7 +116,7 @@ struct GaussianSumFitter {
 
       propOptions.setPlainOptions(opts.propagatorPlainOptions);
 
-      propOptions.navigation.surfaces = sSequence;
+      propOptions.navigation.externalSurfaces = sSequence;
       propOptions.actorList.template get<GsfActor>()
           .m_cfg.bethe_heitler_approx = m_betheHeitlerApproximation.get();
 
@@ -132,7 +132,7 @@ struct GaussianSumFitter {
 
       propOptions.setPlainOptions(opts.propagatorPlainOptions);
 
-      propOptions.navigation.surfaces = sSequence;
+      propOptions.navigation.externalSurfaces = sSequence;
       propOptions.actorList.template get<GsfActor>()
           .m_cfg.bethe_heitler_approx = m_betheHeitlerApproximation.get();
 
@@ -170,7 +170,7 @@ struct GaussianSumFitter {
 
       if (options.useExternalSurfaces) {
         for (auto it = begin; it != end; ++it) {
-          propOptions.navigation.insertExternalSurface(
+          propOptions.navigation.appendExternalSurface(
               *options.extensions.surfaceAccessor(*it));
         }
       }
@@ -192,7 +192,7 @@ struct GaussianSumFitter {
 
       if (options.useExternalSurfaces) {
         for (auto it = begin; it != end; ++it) {
-          propOptions.navigation.insertExternalSurface(
+          propOptions.navigation.appendExternalSurface(
               *options.extensions.surfaceAccessor(*it));
         }
       }

Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp

@@ -1264,7 +1264,7 @@ class Gx2Fitter {
       // Add the measurement surface as external surface to the navigator.
       // We will try to hit those surface by ignoring boundary checks.
       for (const auto& [surface, _] : inputMeasurements) {
-        propagatorOptions.navigation.insertExternalSurface(*surface);
+        propagatorOptions.navigation.appendExternalSurface(*surface);
       }
 
       auto& gx2fActor = propagatorOptions.actorList.template get<GX2FActor>();
@@ -1430,7 +1430,7 @@ class Gx2Fitter {
       // Add the measurement surface as external surface to the navigator.
       // We will try to hit those surface by ignoring boundary checks.
       for (const auto& [surface, _] : inputMeasurements) {
-        propagatorOptions.navigation.insertExternalSurface(*surface);
+        propagatorOptions.navigation.appendExternalSurface(*surface);
       }
 
       auto& gx2fActor = propagatorOptions.actorList.template get<GX2FActor>();

Core/include/Acts/TrackFitting/KalmanFitter.hpp

@@ -745,13 +745,13 @@ class KalmanFitter {
       // Add the measurement surface as external surface to navigator.
       // We will try to hit those surface by ignoring boundary checks.
       for (const auto& [surface, _] : inputMeasurements) {
-        propagatorOptions.navigation.insertExternalSurface(*surface);
+        propagatorOptions.navigation.appendExternalSurface(*surface);
       }
     } else {
       assert(sSequence != nullptr &&
              "DirectNavigator requires a surface sequence for KalmanFitter");
       // Set the surface sequence
-      propagatorOptions.navigation.surfaces = *sSequence;
+      propagatorOptions.navigation.externalSurfaces = *sSequence;
     }
 
     // Catch the actor and set the measurements

Core/src/Navigation/Navigator.cpp

@@ -10,6 +10,7 @@
 
 #include "Acts/Definitions/Units.hpp"
 #include "Acts/Geometry/BoundarySurfaceT.hpp"
+#include "Acts/Geometry/GeometryIdentifier.hpp"
 #include "Acts/Geometry/Portal.hpp"
 #include "Acts/Propagator/NavigatorError.hpp"
 #include "Acts/Surfaces/Surface.hpp"
@@ -89,7 +90,7 @@ Result<void> Navigator::initialize(State& state, const Vector3& position,
   ACTS_VERBOSE(volInfo(state) << "Geometry version is: "
                               << printGeometryVersion(m_geometryVersion));
 
-  state.reset();
+  state.resetForRenavigation();
 
   if (m_geometryVersion == GeometryVersion::Gen3) {
     // Empirical pre-allocation of candidates for the next navigation
@@ -98,9 +99,11 @@ Result<void> Navigator::initialize(State& state, const Vector3& position,
     state.stream.candidates().reserve(50);
 
     state.freeCandidates.clear();
-    state.freeCandidates.reserve(state.options.freeSurfaces.size());
-    for (const Surface* candidate : state.options.freeSurfaces) {
-      state.freeCandidates.emplace_back(candidate, false);
+    state.freeCandidates.reserve(state.options.externalSurfaces.size());
+    for (const Surface* candidate : state.options.externalSurfaces) {
+      if (candidate->geometryId() == GeometryIdentifier{}) {
+        state.freeCandidates.emplace_back(candidate, false);
+      }
     }
   }
 
@@ -215,7 +218,7 @@ NavigationTarget Navigator::nextTarget(State& state, const Vector3& position,
     return nextTarget;
   }
 
-  state.reset();
+  state.resetForRenavigation();
   ++state.statistics.nRenavigations;
 
   // We might have punched through a boundary and entered another volume
@@ -573,20 +576,16 @@ void Navigator::resolveCandidates(State& state, const Vector3& position,
 
   ACTS_VERBOSE(volInfo(state) << "Found " << state.stream.candidates().size()
                               << " navigation candidates.");
-  if (!state.options.externalSurfaces.empty()) {
-    for (const GeometryIdentifier& geoId : state.options.externalSurfaces) {
-      // Don't add any surface which is not in the same volume (volume bits)
-      // or sub volume (extra bits)
-      if (geoId.volume() != state.currentVolume->geometryId().volume() ||
-          geoId.extra() != state.currentVolume->geometryId().extra()) {
-        continue;
-      }
-      const Surface* surface = m_cfg.trackingGeometry->findSurface(geoId);
-      assert(surface != nullptr);
-      ACTS_VERBOSE(volInfo(state) << "Try to navigate to " << surface->type()
-                                  << " surface " << geoId);
-      appendOnly.addSurfaceCandidate(*surface, BoundaryTolerance::Infinite());
-    };
+  for (const Surface* surface : state.options.externalSurfaces) {
+    const GeometryIdentifier geoId = surface->geometryId();
+    // Don't add any surface which is not in the same volume (volume bits)
+    // or sub volume (extra bits)
+    if (geoId.withSensitive(0) != state.currentVolume->geometryId()) {
+      continue;
+    }
+    ACTS_VERBOSE(volInfo(state) << "Try to navigate to " << surface->type()
+                                << " surface " << geoId);
+    appendOnly.addSurfaceCandidate(*surface, BoundaryTolerance::Infinite());
   }
   bool pruneFreeCand{false};
   if (!state.freeCandidates.empty()) {
@@ -681,12 +680,11 @@ void Navigator::resolveSurfaces(State& state, const Vector3& position,
   navOpts.nearLimit = state.options.nearLimit;
   navOpts.farLimit = state.options.farLimit;
 
-  if (!state.options.externalSurfaces.empty()) {
-    const auto layerId = layerSurface->geometryId().layer();
-    for (const GeometryIdentifier& id : state.options.externalSurfaces) {
-      if (id.layer() == layerId) {
-        navOpts.externalSurfaces.push_back(id);
-      }
+  const auto layerId = layerSurface->geometryId().layer();
+  for (const Surface* surface : state.options.externalSurfaces) {
+    const GeometryIdentifier geoId = surface->geometryId();
+    if (geoId.layer() == layerId) {
+      navOpts.externalSurfaces.push_back(geoId);
     }
   }
 

Tests/UnitTests/Core/Propagator/DirectNavigatorTests.cpp

@@ -147,7 +147,7 @@ void runTest(const rpropagator_t& rprop, const dpropagator_t& dprop, double pT,
         typename dpropagator_t::template Options<DirectActorList>;
     DirectOptions dOptions(tgContext, mfContext);
     // Set the surface sequence
-    dOptions.navigation.surfaces = surfaceSequence;
+    dOptions.navigation.externalSurfaces = surfaceSequence;
     // Surface collector configuration
     auto& dCollector = dOptions.actorList.template get<SurfaceCollector<>>();
     dCollector.selector.selectSensitive = true;
@@ -239,7 +239,7 @@ void runSimpleTest(const std::vector<const Surface*>& surfaces,
                           DirectNavigator>::template Options<DirectActorList>;
   DirectOptions pOptions(tgContext, mfContext);
   pOptions.direction = direction;
-  pOptions.navigation.surfaces = surfaces;
+  pOptions.navigation.externalSurfaces = surfaces;
   pOptions.navigation.startSurface = startSurface;
   auto& dCollector = pOptions.actorList.template get<SurfaceCollector<>>();
   dCollector.selector.selectSensitive = true;