refactor: Remove direction from NavigationOptions and drop construc…

…tor (#2345)

I propose to drop the navigation direction from `NavigationOptions` as the direction vector parameter in the lookup functions is sufficient to provide the same functionality.

Additionally I removed the constructor of `NavigationOptions` which duplicated default values and was less explicit. I also removed the magic overstepping limit which will be overwritten by the one from the stepper anyways.

Core/include/Acts/Navigation/DetectorNavigator.hpp

@@ -84,7 +84,7 @@ class DetectorNavigator {
 
   void resetState(State& state, const GeometryContext& /*geoContext*/,
                   const Vector3& /*pos*/, const Vector3& /*dir*/,
-                  Direction /*navDir*/, const Surface* /*ssurface*/,
+                  const Surface* /*ssurface*/,
                   const Surface* /*tsurface*/) const {
     // Reset everything first
     state = State();

Core/include/Acts/Propagator/DirectNavigator.hpp

@@ -129,8 +129,7 @@ class DirectNavigator {
   /// @param tsurface is the target surface
   void resetState(State& state, const GeometryContext& /*geoContext*/,
                   const Vector3& /*pos*/, const Vector3& /*dir*/,
-                  Direction /*navDir*/, const Surface* ssurface,
-                  const Surface* tsurface) const {
+                  const Surface* ssurface, const Surface* tsurface) const {
     // Reset everything except the navSurfaces
     auto navSurfaces = state.navSurfaces;
     state = State();

Core/include/Acts/Propagator/Navigator.hpp

@@ -35,9 +35,6 @@ namespace Acts {
 /// @tparam object_t Type of the object for navigation to check against
 template <typename object_t>
 struct NavigationOptions {
-  /// The navigation direction
-  Direction navDir = Direction::Forward;
-
   /// The boundary check directive
   BoundaryCheck boundaryCheck = true;
 
@@ -63,35 +60,10 @@ struct NavigationOptions {
   double pathLimit = std::numeric_limits<double>::max();
 
   /// The overstep tolerance for this navigation step
-  /// @note must be negative as it describes overstepping
-  /// @todo could be dynamic in the future (pT dependent)
-  double overstepLimit = -1 * UnitConstants::um;
+  double overstepLimit = 0;
 
   /// Force intersection with boundaries
   bool forceIntersectBoundaries = false;
-
-  /// Constructor
-  ///
-  /// @param ndir Navigation direction prescription
-  /// @param bcheck Boundary check for the navigation action
-  /// @param resolves Boolean whether to resolve sensitives
-  /// @param resolvem Boolean whether to resolve material
-  /// @param resolvep Boolean whether to resolve passives
-  /// @param sobject Start object to check against
-  /// @param eobject End object to check against
-  NavigationOptions(Direction ndir, BoundaryCheck bcheck, bool resolves = true,
-                    bool resolvem = true, bool resolvep = false,
-                    const object_t* sobject = nullptr,
-                    const object_t* eobject = nullptr)
-      : navDir(ndir),
-        boundaryCheck(std::move(bcheck)),
-        resolveSensitive(resolves),
-        resolveMaterial(resolvem),
-        resolvePassive(resolvep),
-        startObject(sobject),
-        endObject(eobject),
-        pathLimit(ndir * std::numeric_limits<double>::max()),
-        overstepLimit(-1 * UnitConstants::um) {}
 };
 
 /// Navigator class
@@ -244,12 +216,11 @@ class Navigator {
   /// @param state is the state
   /// @param geoContext is the geometry context
   /// @param pos is the global position
-  /// @param dir is the momentum direction
-  /// @param navDir is the navigation direction
+  /// @param dir is the direction of navigation
   /// @param ssurface is the new starting surface
   /// @param tsurface is the target surface
   void resetState(State& state, const GeometryContext& geoContext,
-                  const Vector3& pos, const Vector3& dir, Direction navDir,
+                  const Vector3& pos, const Vector3& dir,
                   const Surface* ssurface, const Surface* tsurface) const {
     // Reset everything first
     state = State();
@@ -267,8 +238,10 @@ class Navigator {
     state.targetSurface = tsurface;
 
     // Get the compatible layers (including the current layer)
-    NavigationOptions<Layer> navOpts(navDir, true, true, true, true, nullptr,
-                                     nullptr);
+    NavigationOptions<Layer> navOpts;
+    navOpts.resolveSensitive = true;
+    navOpts.resolveMaterial = true;
+    navOpts.resolvePassive = true;
     state.navLayers =
         state.currentVolume->compatibleLayers(geoContext, pos, dir, navOpts);
 
@@ -786,10 +759,11 @@ class Navigator {
       // check if current volume has BVH, or layers
       if (state.navigation.currentVolume->hasBoundingVolumeHierarchy()) {
         // has hierarchy, use that, skip layer resolution
-        NavigationOptions<Surface> navOpts(
-            state.options.direction, true, m_cfg.resolveSensitive,
-            m_cfg.resolveMaterial, m_cfg.resolvePassive, nullptr,
-            state.navigation.targetSurface);
+        NavigationOptions<Surface> navOpts;
+        navOpts.resolveSensitive = m_cfg.resolveSensitive;
+        navOpts.resolveMaterial = m_cfg.resolveMaterial;
+        navOpts.resolvePassive = m_cfg.resolvePassive;
+        navOpts.endObject = state.navigation.targetSurface;
         navOpts.overstepLimit = stepper.overstepLimit(state.stepping);
         double opening_angle = 0;
 
@@ -826,7 +800,8 @@ class Navigator {
         auto protoNavSurfaces =
             state.navigation.currentVolume->compatibleSurfacesFromHierarchy(
                 state.geoContext, stepper.position(state.stepping),
-                stepper.direction(state.stepping), opening_angle, navOpts);
+                state.options.direction * stepper.direction(state.stepping),
+                opening_angle, navOpts);
         if (!protoNavSurfaces.empty()) {
           // did we find any surfaces?
 
@@ -967,15 +942,15 @@ class Navigator {
     // Helper function to find boundaries
     auto findBoundaries = [&]() -> bool {
       // The navigation options
-      NavigationOptions<Surface> navOpts(state.options.direction, true);
+      NavigationOptions<Surface> navOpts;
+      // Exclude the current surface in case it's a boundary
+      navOpts.startObject = state.navigation.currentSurface;
       navOpts.pathLimit =
           stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
       navOpts.overstepLimit = stepper.overstepLimit(state.stepping);
       navOpts.forceIntersectBoundaries =
           state.navigation.forceIntersectBoundaries;
 
-      // Exclude the current surface in case it's a boundary
-      navOpts.startObject = state.navigation.currentSurface;
       ACTS_VERBOSE(volInfo(state)
                    << "Try to find boundaries, we are at: "
                    << stepper.position(state.stepping).transpose() << ", dir: "
@@ -985,7 +960,8 @@ class Navigator {
       state.navigation.navBoundaries =
           state.navigation.currentVolume->compatibleBoundaries(
               state.geoContext, stepper.position(state.stepping),
-              stepper.direction(state.stepping), navOpts, logger());
+              state.options.direction * stepper.direction(state.stepping),
+              navOpts, logger());
       // The number of boundary candidates
       if (logger().doPrint(Logging::VERBOSE)) {
         std::ostringstream os;
@@ -1151,10 +1127,12 @@ class Navigator {
     bool onStart = (navLayer == state.navigation.startLayer);
     auto startSurface = onStart ? state.navigation.startSurface : layerSurface;
     // Use navigation parameters and NavigationOptions
-    NavigationOptions<Surface> navOpts(
-        state.options.direction, true, m_cfg.resolveSensitive,
-        m_cfg.resolveMaterial, m_cfg.resolvePassive, startSurface,
-        state.navigation.targetSurface);
+    NavigationOptions<Surface> navOpts;
+    navOpts.resolveSensitive = m_cfg.resolveSensitive;
+    navOpts.resolveMaterial = m_cfg.resolveMaterial;
+    navOpts.resolvePassive = m_cfg.resolvePassive;
+    navOpts.startObject = startSurface;
+    navOpts.endObject = state.navigation.targetSurface;
 
     std::vector<GeometryIdentifier> externalSurfaces;
     if (!state.navigation.externalSurfaces.empty()) {
@@ -1179,7 +1157,7 @@ class Navigator {
     // get the surfaces
     state.navigation.navSurfaces = navLayer->compatibleSurfaces(
         state.geoContext, stepper.position(state.stepping),
-        stepper.direction(state.stepping), navOpts);
+        state.options.direction * stepper.direction(state.stepping), navOpts);
     // the number of layer candidates
     if (!state.navigation.navSurfaces.empty()) {
       if (logger().doPrint(Logging::VERBOSE)) {
@@ -1232,10 +1210,12 @@ class Navigator {
             : nullptr;
     // Create the navigation options
     // - and get the compatible layers, start layer will be excluded
-    NavigationOptions<Layer> navOpts(
-        state.options.direction, m_cfg.boundaryCheckLayerResolving,
-        m_cfg.resolveSensitive, m_cfg.resolveMaterial, m_cfg.resolvePassive,
-        startLayer, nullptr);
+    NavigationOptions<Layer> navOpts;
+    navOpts.boundaryCheck = m_cfg.boundaryCheckLayerResolving;
+    navOpts.resolveSensitive = m_cfg.resolveSensitive;
+    navOpts.resolveMaterial = m_cfg.resolveMaterial;
+    navOpts.resolvePassive = m_cfg.resolvePassive;
+    navOpts.startObject = startLayer;
     // Set also the target surface
     navOpts.targetSurface = state.navigation.targetSurface;
     navOpts.pathLimit =
@@ -1245,7 +1225,8 @@ class Navigator {
     state.navigation.navLayers =
         state.navigation.currentVolume->compatibleLayers(
             state.geoContext, stepper.position(state.stepping),
-            stepper.direction(state.stepping), navOpts);
+            state.options.direction * stepper.direction(state.stepping),
+            navOpts);
 
     // Layer candidates have been found
     if (!state.navigation.navLayers.empty()) {

Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp

@@ -591,7 +591,7 @@ class CombinatorialKalmanFilter {
       // after smoothing
       navigator.resetState(
           state.navigation, state.geoContext, stepper.position(state.stepping),
-          stepper.direction(state.stepping), state.options.direction,
+          state.options.direction * stepper.direction(state.stepping),
           &currentState.referenceSurface(), nullptr);
 
       // No Kalman filtering for the starting surface, but still need

Core/include/Acts/TrackFitting/KalmanFitter.hpp

@@ -537,7 +537,7 @@ class KalmanFitter {
       // Reset navigation state
       navigator.resetState(
           state.navigation, state.geoContext, stepper.position(state.stepping),
-          stepper.direction(state.stepping), state.options.direction,
+          state.options.direction * stepper.direction(state.stepping),
           &st.referenceSurface(), targetSurface);
 
       // Update material effects for last measurement state in reversed

Core/src/Geometry/Layer.cpp

@@ -131,7 +131,7 @@ Acts::Layer::compatibleSurfaces(
     // intersect the end surface
     // - it is the final one don't use the boundary check at all
     SurfaceIntersection endInter = options.endObject->intersect(
-        gctx, position, options.navDir * direction, BoundaryCheck(true));
+        gctx, position, direction, BoundaryCheck(true));
     // non-valid intersection with the end surface provided at this layer
     // indicates wrong direction or faulty setup
     // -> do not return compatible surfaces since they may lead you on a wrong
@@ -148,7 +148,7 @@ Acts::Layer::compatibleSurfaces(
     // -> this avoids punch through for cylinders
     double pCorrection =
         surfaceRepresentation().pathCorrection(gctx, position, direction);
-    pathLimit = 1.5 * thickness() * pCorrection * options.navDir;
+    pathLimit = 1.5 * thickness() * pCorrection;
   }
 
   // lemma 0 : accept the surface
@@ -185,12 +185,11 @@ Acts::Layer::compatibleSurfaces(
     }
     // the surface intersection
     SurfaceIntersection sfi =
-        sf.intersect(gctx, position, options.navDir * direction, boundaryCheck);
+        sf.intersect(gctx, position, direction, boundaryCheck);
     // check if intersection is valid and pathLimit has not been exceeded
     if (sfi && detail::checkIntersection(sfi.intersection, pathLimit,
                                          overstepLimit, s_onSurfaceTolerance)) {
       // Now put the right sign on it
-      sfi.intersection.pathLength *= options.navDir;
       sIntersections.push_back(sfi);
     }
   };
@@ -250,11 +249,7 @@ Acts::Layer::compatibleSurfaces(
   sIntersections.resize(std::distance(sIntersections.begin(), it));
 
   // sort according to the path length
-  if (options.navDir == Direction::Forward) {
-    std::sort(sIntersections.begin(), sIntersections.end());
-  } else {
-    std::sort(sIntersections.begin(), sIntersections.end(), std::greater<>());
-  }
+  std::sort(sIntersections.begin(), sIntersections.end());
 
   return sIntersections;
 }
@@ -272,9 +267,6 @@ Acts::SurfaceIntersection Acts::Layer::surfaceOnApproach(
                    (m_ssSensitiveSurfaces > 1 || m_ssApproachSurfaces > 1 ||
                     (surfaceRepresentation().surfaceMaterial() != nullptr));
 
-  // The signed direction: solution (except overstepping) is positive
-  auto sDirection = options.navDir * direction;
-
   // The Limits: current path & overstepping
   double pLimit = options.pathLimit;
   double oLimit = options.overstepLimit;
@@ -305,13 +297,13 @@ Acts::SurfaceIntersection Acts::Layer::surfaceOnApproach(
   // Approach descriptor present and resolving is necessary
   if (m_approachDescriptor && (resolvePS || resolveMS)) {
     SurfaceIntersection aSurface = m_approachDescriptor->approachSurface(
-        gctx, position, sDirection, options.boundaryCheck);
+        gctx, position, direction, options.boundaryCheck);
     return checkIntersection(aSurface);
   }
 
   // Intersect and check the representing surface
   const Surface& rSurface = surfaceRepresentation();
   auto sIntersection =
-      rSurface.intersect(gctx, position, sDirection, options.boundaryCheck);
+      rSurface.intersect(gctx, position, direction, options.boundaryCheck);
   return checkIntersection(sIntersection);
 }

Core/src/Geometry/TrackingVolume.cpp

@@ -472,9 +472,6 @@ Acts::TrackingVolume::compatibleBoundaries(
   auto excludeObject = options.startObject;
   boost::container::small_vector<Acts::BoundaryIntersection, 4> bIntersections;
 
-  // The signed direction: solution (except overstepping) is positive
-  auto sDirection = options.navDir * direction;
-
   // The Limits: current, path & overstepping
   double pLimit = options.pathLimit;
   double oLimit = options.overstepLimit;
@@ -550,7 +547,7 @@ Acts::TrackingVolume::compatibleBoundaries(
 
       // Exclude the boundary where you are on
       if (excludeObject != &bSurfaceRep) {
-        auto bCandidate = bSurfaceRep.intersect(gctx, position, sDirection,
+        auto bCandidate = bSurfaceRep.intersect(gctx, position, direction,
                                                 options.boundaryCheck);
         // Intersect and continue
         auto bIntersection = checkIntersection(bCandidate, bsIter.get());
@@ -635,10 +632,9 @@ Acts::TrackingVolume::compatibleLayers(
         }
       }
       // move to next one or break because you reached the end layer
-      tLayer =
-          (tLayer == options.endObject)
-              ? nullptr
-              : tLayer->nextLayer(gctx, position, options.navDir * direction);
+      tLayer = (tLayer == options.endObject)
+                   ? nullptr
+                   : tLayer->nextLayer(gctx, position, direction);
     }
     std::sort(lIntersections.begin(), lIntersections.end());
   }
@@ -693,16 +689,13 @@ Acts::TrackingVolume::compatibleSurfacesFromHierarchy(
     return sIntersections;
   }
 
-  // The signed direction
-  Vector3 sdir = options.navDir * direction;
-
   std::vector<const Volume*> hits;
   if (angle == 0) {
     // use ray
-    Ray3D obj(position, sdir);
+    Ray3D obj(position, direction);
     hits = intersectSearchHierarchy(std::move(obj), m_bvhTop);
   } else {
-    Acts::Frustum<ActsScalar, 3, 4> obj(position, sdir, angle);
+    Acts::Frustum<ActsScalar, 3, 4> obj(position, direction, angle);
     hits = intersectSearchHierarchy(std::move(obj), m_bvhTop);
   }
 
@@ -713,7 +706,7 @@ Acts::TrackingVolume::compatibleSurfacesFromHierarchy(
         boundarySurfaces = avol->boundarySurfaces();
     for (const auto& bs : boundarySurfaces) {
       const Surface& srf = bs->surfaceRepresentation();
-      auto sfi = srf.intersect(gctx, position, sdir, false);
+      auto sfi = srf.intersect(gctx, position, direction, false);
       if (sfi and sfi.intersection.pathLength > oLimit and
           sfi.intersection.pathLength <= pLimit) {
         sIntersections.push_back(std::move(sfi));
@@ -722,11 +715,7 @@ Acts::TrackingVolume::compatibleSurfacesFromHierarchy(
   }
 
   // Sort according to the path length
-  if (options.navDir == Direction::Forward) {
-    std::sort(sIntersections.begin(), sIntersections.end());
-  } else {
-    std::sort(sIntersections.begin(), sIntersections.end(), std::greater<>());
-  }
+  std::sort(sIntersections.begin(), sIntersections.end());
 
   return sIntersections;
 }

Examples/Python/tests/root_file_hashes.txt

@@ -25,19 +25,19 @@ test_material_recording__geant4_material_tracks.root: e411152d370775463c22b19a35
 test_truth_tracking_kalman[generic-0.0]__trackstates_fitter.root: 7e933e0219728ad8c139c2c8d0a7b09d133108a2c3053b0e1972189e34534592
 test_truth_tracking_kalman[generic-0.0]__tracksummary_fitter.root: e41639378960b2ecbcc31ad432e29eb56b6d900e642fdc53879d0708f8ab9155
 test_truth_tracking_kalman[generic-0.0]__performance_track_finder.root: 7fc6f717723c9eddcbf44820b384b373cee6f04b72f79902f938f35e3ff9b470
-test_truth_tracking_kalman[generic-1000.0]__trackstates_fitter.root: 6c0313a50148aa71e20d817642cd2dffc85e467915298f00d75bef33ad83cf3c
+test_truth_tracking_kalman[generic-1000.0]__trackstates_fitter.root: 2298c2ae839e8208bc640e7208d2dc406f3316042c50f23ff0a65e4652a0bc2c
 test_truth_tracking_kalman[generic-1000.0]__tracksummary_fitter.root: fd522a5636614a4b987802bbb8c9ff9b5eb0d9b0e876a3066e66354eaa297398
 test_truth_tracking_kalman[generic-1000.0]__performance_track_finder.root: 7fc6f717723c9eddcbf44820b384b373cee6f04b72f79902f938f35e3ff9b470
 test_truth_tracking_kalman[odd-0.0]__trackstates_fitter.root: 4093d18a84cd01ddec712adadcf8f271e7dd1fa473a734f44b80963477fc4f54
 test_truth_tracking_kalman[odd-0.0]__tracksummary_fitter.root: d33319894494feb8c0d6f0f90a1d1e59caf9de32eda480ab73d0e40dfcdd1322
 test_truth_tracking_kalman[odd-0.0]__performance_track_finder.root: 39aec6316cceb90e314e16b02947faa691c18f57c3a851a25e547a8fc05a4593
-test_truth_tracking_kalman[odd-1000.0]__trackstates_fitter.root: 0fcf2020a10ddd7030cd1b58ef7affd60121b83eabb92e9547818e93bab9c777
+test_truth_tracking_kalman[odd-1000.0]__trackstates_fitter.root: 11f863960ba24d32832a620e3a4447fd8efceda09b620bdaef35a5fcc54a5676
 test_truth_tracking_kalman[odd-1000.0]__tracksummary_fitter.root: 99fe9c3be034549816c73f7f3d2190df7efb0adf6aed16cceb86aa4a9ac271f0
 test_truth_tracking_kalman[odd-1000.0]__performance_track_finder.root: 39aec6316cceb90e314e16b02947faa691c18f57c3a851a25e547a8fc05a4593
 test_truth_tracking_gsf[generic]__trackstates_gsf.root: 787294f42fadbd14827ae47133ba90d657999fa815df3fa01e3ddc3c0709d880
 test_truth_tracking_gsf[generic]__tracksummary_gsf.root: abfa600668eda81fa0542df2d9ced6d550d744f1f8f3f4c789d137f7748363e2
-test_truth_tracking_gsf[odd]__trackstates_gsf.root: 0c6b46ee55e992c0846abea69d69205cbec358aa48330396ce83955bb2153177
-test_truth_tracking_gsf[odd]__tracksummary_gsf.root: 2b6f6b95af4bd36d0a1084a6fb79e0d09dac622f81c8fd1a0c5550c4cc458b66
+test_truth_tracking_gsf[odd]__trackstates_gsf.root: 8ea8bdeda5ba2d4fbedd5b25b1d9f01a47980f9ae5398ad157b40ec5fa070cda
+test_truth_tracking_gsf[odd]__tracksummary_gsf.root: 690b72c9ed7a6779d6233b8b68d05e9b860165efd258ad80581485a4e01d9996
 test_particle_gun__particles.root: 8549ba6e20338004ab8ba299fc65e1ee5071985b46df8f77f887cb6fef56a8ec
 test_material_mapping__material-map_tracks.root: 4e1c866038f0c06b099aa74fd01c3d875f07b89f54898f90debd9b558d8e4025
 test_material_mapping__propagation-material.root: 646b8e2bbacec40d0bc4132236f9ab3f03b088e656e6e9b80c47ae03eaf6eab5