refactor: Explicit checkPathLength and isValid intersection check (…

…#3416)

- Rename `checkIntersection` to `checkPathLength`
  - Remove templating
- Add `isValid` to intersection objects
  - Will remove the boolean conversion in the future

Core/include/Acts/Navigation/NavigationStateUpdaters.hpp

@@ -56,7 +56,7 @@ inline void updateCandidates(const GeometryContext& gctx,
         gctx, position, direction, c.boundaryTolerance, s_onSurfaceTolerance);
     for (auto& si : sIntersection.split()) {
       c.objectIntersection = si;
-      if (c.objectIntersection &&
+      if (c.objectIntersection.isValid() &&
           c.objectIntersection.pathLength() > overstepTolerance) {
         nextSurfaceCandidates.emplace_back(c);
       }

Core/include/Acts/Propagator/DirectNavigator.hpp

@@ -277,8 +277,8 @@ class DirectNavigator {
                                            boundaryTolerance, tolerance);
 
     for (auto& intersection : intersections.split()) {
-      if (detail::checkIntersection(intersection, nearLimit, farLimit,
-                                    logger())) {
+      if (detail::checkPathLength(intersection.pathLength(), nearLimit,
+                                  farLimit, logger())) {
         return intersection;
       }
     }

Core/include/Acts/Propagator/Navigator.hpp

@@ -989,7 +989,7 @@ class Navigator {
                   state.options.direction * stepper.direction(state.stepping),
                   BoundaryTolerance::Infinite(), state.options.surfaceTolerance)
               .closest();
-      if (targetIntersection) {
+      if (targetIntersection.isValid()) {
         ACTS_VERBOSE(volInfo(state)
                      << "Target estimate position ("
                      << targetIntersection.position().x() << ", "

Core/include/Acts/Propagator/StandardAborters.hpp

@@ -130,9 +130,9 @@ struct SurfaceReached {
     bool intersectionFound = false;
 
     for (const auto& intersection : sIntersection.split()) {
-      if (intersection &&
-          detail::checkIntersection(intersection.intersection(), nearLimit,
-                                    farLimit, logger)) {
+      if (intersection.isValid() &&
+          detail::checkPathLength(intersection.pathLength(), nearLimit,
+                                  farLimit, logger)) {
         stepper.updateStepSize(state.stepping, intersection.pathLength(),
                                ConstrainedStep::aborter, false);
         ACTS_VERBOSE(

Core/include/Acts/Propagator/TryAllNavigator.hpp

@@ -343,8 +343,9 @@ class TryAllNavigator : public TryAllNavigatorBase {
                               state.options.surfaceTolerance);
       for (const auto& intersection : intersections.first.split()) {
         // exclude invalid intersections
-        if (!intersection ||
-            !detail::checkIntersection(intersection, nearLimit, farLimit)) {
+        if (!intersection.isValid() ||
+            !detail::checkPathLength(intersection.pathLength(), nearLimit,
+                                     farLimit)) {
           continue;
         }
         // store candidate
@@ -746,8 +747,9 @@ class TryAllOverstepNavigator : public TryAllNavigatorBase {
             state.geoContext, end, direction, state.options.surfaceTolerance);
         for (const auto& intersection : intersections.first.split()) {
           // exclude invalid intersections
-          if (!intersection ||
-              !detail::checkIntersection(intersection, nearLimit, farLimit)) {
+          if (!intersection.isValid() ||
+              !detail::checkPathLength(intersection.pathLength(), nearLimit,
+                                       farLimit)) {
             continue;
           }
           // exclude last candidate

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

@@ -56,8 +56,9 @@ Acts::Intersection3D::Status updateSingleSurfaceStatus(
   const double nearLimit = std::numeric_limits<double>::lowest();
   const double farLimit = state.stepSize.value(ConstrainedStep::aborter);
 
-  if (sIntersection && detail::checkIntersection(sIntersection.intersection(),
-                                                 nearLimit, farLimit, logger)) {
+  if (sIntersection.isValid() &&
+      detail::checkPathLength(sIntersection.pathLength(), nearLimit, farLimit,
+                              logger)) {
     ACTS_VERBOSE("Surface is reachable");
     stepper.updateStepSize(state, sIntersection.pathLength(),
                            ConstrainedStep::actor);

Core/include/Acts/Utilities/Intersection.hpp

@@ -1,6 +1,6 @@
 // This file is part of the Acts project.
 //
-// Copyright (C) 2016-2023 CERN for the benefit of the Acts project
+// Copyright (C) 2016-2024 CERN for the benefit of the Acts project
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -60,10 +60,15 @@ class Intersection {
       : m_position(position), m_pathLength(pathLength), m_status(status) {}
 
   /// Returns whether the intersection was successful or not
-  constexpr explicit operator bool() const {
-    return m_status != Status::missed;
+  /// @deprecated
+  [[deprecated("Use isValid() instead")]] constexpr explicit operator bool()
+      const {
+    return isValid();
   }
 
+  /// Returns whether the intersection was successful or not
+  constexpr bool isValid() const { return m_status != Status::missed; }
+
   constexpr const Position& position() const { return m_position; }
 
   constexpr ActsScalar pathLength() const { return m_pathLength; }
@@ -141,10 +146,15 @@ class ObjectIntersection {
       : m_intersection(intersection), m_object(object), m_index(index) {}
 
   /// Returns whether the intersection was successful or not
-  constexpr explicit operator bool() const {
-    return m_intersection.operator bool();
+  /// @deprecated
+  [[deprecated("Use isValid() instead")]] constexpr explicit operator bool()
+      const {
+    return isValid();
   }
 
+  /// Returns whether the intersection was successful or not
+  constexpr bool isValid() const { return m_intersection.isValid(); }
+
   constexpr const Intersection3D& intersection() const {
     return m_intersection;
   }
@@ -256,50 +266,16 @@ class ObjectMultiIntersection {
 
 namespace detail {
 
-/// This function checks if an intersection is valid for the specified
-/// path-limit and overstep-limit
-///
-/// @tparam intersection_t Type of the intersection object
+/// This function checks if an intersection path length is valid for the
+/// specified near-limit and far-limit
 ///
-/// @param intersection The intersection to check
-/// @param nearLimit The minimum distance for an intersection to be considered
-/// @param farLimit The maximum distance for an intersection to be considered
+/// @param pathLength The path length of the intersection
+/// @param nearLimit The minimum path length for an intersection to be considered
+/// @param farLimit The maximum path length for an intersection to be considered
 /// @param logger A optionally supplied logger which prints out a lot of infos
 ///               at VERBOSE level
-template <typename intersection_t>
-bool checkIntersection(const intersection_t& intersection, double nearLimit,
-                       double farLimit,
-                       const Logger& logger = getDummyLogger()) {
-  const double distance = intersection.pathLength();
-  // TODO why?
-  const double tolerance = s_onSurfaceTolerance;
-
-  ACTS_VERBOSE(" -> near limit, far limit, distance: "
-               << nearLimit << ", " << farLimit << ", " << distance);
-
-  const bool coCriterion = distance > nearLimit;
-  const bool cpCriterion = distance < farLimit + tolerance;
-
-  const bool accept = coCriterion && cpCriterion;
-
-  if (accept) {
-    ACTS_VERBOSE("Intersection is WITHIN limit");
-  } else {
-    ACTS_VERBOSE("Intersection is OUTSIDE limit because: ");
-    if (!coCriterion) {
-      ACTS_VERBOSE("- intersection path length "
-                   << distance << " <= near limit " << nearLimit);
-    }
-    if (!cpCriterion) {
-      ACTS_VERBOSE("- intersection path length "
-                   << distance << " is over the far limit "
-                   << (farLimit + tolerance) << " (including tolerance of "
-                   << tolerance << ")");
-    }
-  }
-
-  return accept;
-}
+bool checkPathLength(double pathLength, double nearLimit, double farLimit,
+                     const Logger& logger = getDummyLogger());
 
 }  // namespace detail
 

Core/include/Acts/Utilities/TrackHelpers.hpp

@@ -195,7 +195,7 @@ findTrackStateForExtrapolation(
       }
 
       SurfaceIntersection intersection = intersect(*first);
-      if (!intersection) {
+      if (!intersection.isValid()) {
         ACTS_ERROR("no intersection found");
         return Result<std::pair<TrackStateProxy, double>>::failure(
             TrackExtrapolationError::ReferenceSurfaceUnreachable);
@@ -215,7 +215,7 @@ findTrackStateForExtrapolation(
       }
 
       SurfaceIntersection intersection = intersect(*last);
-      if (!intersection) {
+      if (!intersection.isValid()) {
         ACTS_ERROR("no intersection found");
         return Result<std::pair<TrackStateProxy, double>>::failure(
             TrackExtrapolationError::ReferenceSurfaceUnreachable);
@@ -246,13 +246,13 @@ findTrackStateForExtrapolation(
       double absDistanceFirst = std::abs(intersectionFirst.pathLength());
       double absDistanceLast = std::abs(intersectionLast.pathLength());
 
-      if (intersectionFirst && absDistanceFirst <= absDistanceLast) {
+      if (intersectionFirst.isValid() && absDistanceFirst <= absDistanceLast) {
         ACTS_VERBOSE("using first track state with intersection at "
                      << intersectionFirst.pathLength());
         return std::make_pair(*first, intersectionFirst.pathLength());
       }
 
-      if (intersectionLast && absDistanceLast <= absDistanceFirst) {
+      if (intersectionLast.isValid() && absDistanceLast <= absDistanceFirst) {
         ACTS_VERBOSE("using last track state with intersection at "
                      << intersectionLast.pathLength());
         return std::make_pair(*last, intersectionLast.pathLength());

Core/src/Geometry/GenericApproachDescriptor.cpp

@@ -35,8 +35,9 @@ Acts::SurfaceIntersection Acts::GenericApproachDescriptor::approachSurface(
     auto sfIntersection =
         sf->intersect(gctx, position, direction, boundaryTolerance);
     for (const auto& intersection : sfIntersection.split()) {
-      if (intersection &&
-          detail::checkIntersection(intersection, nearLimit, farLimit)) {
+      if (intersection.isValid() &&
+          detail::checkPathLength(intersection.pathLength(), nearLimit,
+                                  farLimit)) {
         sIntersections.push_back(intersection);
       }
     }

Core/src/Geometry/Layer.cpp

@@ -14,6 +14,7 @@
 #include "Acts/Propagator/Navigator.hpp"
 #include "Acts/Surfaces/BoundaryTolerance.hpp"
 #include "Acts/Surfaces/Surface.hpp"
+#include "Acts/Utilities/Intersection.hpp"
 
 #include <algorithm>
 #include <functional>
@@ -138,7 +139,7 @@ Acts::Layer::compatibleSurfaces(
     // indicates wrong direction or faulty setup
     // -> do not return compatible surfaces since they may lead you on a wrong
     // navigation path
-    if (endInter) {
+    if (endInter.isValid()) {
       farLimit = endInter.pathLength();
     } else {
       return sIntersections;
@@ -196,8 +197,8 @@ Acts::Layer::compatibleSurfaces(
     // the surface intersection
     SurfaceIntersection sfi =
         sf.intersect(gctx, position, direction, boundaryTolerance).closest();
-    if (sfi &&
-        detail::checkIntersection(sfi.intersection(), nearLimit, farLimit) &&
+    if (sfi.isValid() &&
+        detail::checkPathLength(sfi.pathLength(), nearLimit, farLimit) &&
         isUnique(sfi)) {
       sIntersections.push_back(sfi);
     }
@@ -267,8 +268,8 @@ Acts::SurfaceIntersection Acts::Layer::surfaceOnApproach(
   auto findValidIntersection =
       [&](const SurfaceMultiIntersection& sfmi) -> SurfaceIntersection {
     for (const auto& sfi : sfmi.split()) {
-      if (sfi &&
-          detail::checkIntersection(sfi.intersection(), nearLimit, farLimit)) {
+      if (sfi.isValid() &&
+          detail::checkPathLength(sfi.pathLength(), nearLimit, farLimit)) {
         return sfi;
       }
     }

Core/src/Geometry/TrackingVolume.cpp

@@ -19,6 +19,7 @@
 #include "Acts/Surfaces/RegularSurface.hpp"
 #include "Acts/Surfaces/Surface.hpp"
 #include "Acts/Utilities/BinningType.hpp"
+#include "Acts/Utilities/Intersection.hpp"
 
 #include <algorithm>
 #include <memory>
@@ -440,7 +441,7 @@ TrackingVolume::compatibleBoundaries(const GeometryContext& gctx,
       [&](SurfaceMultiIntersection& candidates,
           const BoundarySurface* boundary) -> BoundaryIntersection {
     for (const auto& intersection : candidates.split()) {
-      if (!intersection) {
+      if (!intersection.isValid()) {
         continue;
       }
 
@@ -464,8 +465,8 @@ TrackingVolume::compatibleBoundaries(const GeometryContext& gctx,
 
       ACTS_VERBOSE("Check intersection with surface "
                    << boundary->surfaceRepresentation().geometryId());
-      if (detail::checkIntersection(intersection.intersection(), nearLimit,
-                                    farLimit, logger)) {
+      if (detail::checkPathLength(intersection.pathLength(), nearLimit,
+                                  farLimit, logger)) {
         return BoundaryIntersection(intersection, boundary);
       }
     }
@@ -495,7 +496,7 @@ TrackingVolume::compatibleBoundaries(const GeometryContext& gctx,
                                           options.boundaryTolerance);
       // Intersect and continue
       auto intersection = checkIntersection(candidates, boundary.get());
-      if (intersection.first) {
+      if (intersection.first.isValid()) {
         ACTS_VERBOSE(" - Proceed with surface");
         intersections.push_back(intersection);
       } else {
@@ -550,7 +551,7 @@ TrackingVolume::compatibleLayers(
       auto atIntersection =
           tLayer->surfaceOnApproach(gctx, position, direction, options);
       // Intersection is ok - take it (move to surface on approach)
-      if (atIntersection) {
+      if (atIntersection.isValid()) {
         // create a layer intersection
         lIntersections.push_back(LayerIntersection(atIntersection, tLayer));
       }

Core/src/Utilities/CMakeLists.txt

@@ -7,4 +7,5 @@ target_sources(
     SpacePointUtility.cpp
     TrackHelpers.cpp
     BinningType.cpp
+    Intersection.cpp
 )

Core/src/Utilities/Intersection.cpp

@@ -0,0 +1,45 @@
+// This file is part of the Acts project.
+//
+// Copyright (C) 2024 CERN for the benefit of the Acts project
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include "Acts/Utilities/Intersection.hpp"
+
+namespace Acts {
+
+bool detail::checkPathLength(double pathLength, double nearLimit,
+                             double farLimit, const Logger& logger) {
+  // TODO why?
+  const double tolerance = s_onSurfaceTolerance;
+
+  ACTS_VERBOSE(" -> near limit, far limit, distance: "
+               << nearLimit << ", " << farLimit << ", " << pathLength);
+
+  const bool coCriterion = pathLength > nearLimit;
+  const bool cpCriterion = pathLength < farLimit + tolerance;
+
+  const bool accept = coCriterion && cpCriterion;
+
+  if (accept) {
+    ACTS_VERBOSE("Intersection is WITHIN limit");
+  } else {
+    ACTS_VERBOSE("Intersection is OUTSIDE limit because: ");
+    if (!coCriterion) {
+      ACTS_VERBOSE("- intersection path length "
+                   << pathLength << " <= near limit " << nearLimit);
+    }
+    if (!cpCriterion) {
+      ACTS_VERBOSE("- intersection path length "
+                   << pathLength << " is over the far limit "
+                   << (farLimit + tolerance) << " (including tolerance of "
+                   << tolerance << ")");
+    }
+  }
+
+  return accept;
+}
+
+}  // namespace Acts

Fatras/src/Digitization/PlanarSurfaceMask.cpp

@@ -38,7 +38,7 @@ namespace {
 /// @param sLength The segment length, maximal allowed length
 void checkIntersection(std::vector<Acts::Intersection2D>& intersections,
                        const Acts::Intersection2D& candidate, double sLength) {
-  if (candidate && candidate.pathLength() > 0 &&
+  if (candidate.isValid() && candidate.pathLength() > 0 &&
       candidate.pathLength() < sLength) {
     intersections.push_back(candidate);
   }
@@ -282,7 +282,7 @@ ActsFatras::PlanarSurfaceMask::annulusMask(const Acts::AnnulusBounds& aBounds,
         Acts::VectorHelpers::phi(vertices[phii[iarc * 2]] - moduleOrigin),
         Acts::VectorHelpers::phi(vertices[phii[iarc * 2 + 1]] - moduleOrigin),
         segment[0] - moduleOrigin, sDir);
-    if (intersection) {
+    if (intersection.isValid()) {
       checkIntersection(intersections,
                         Acts::Intersection2D(
                             intersection.position() + moduleOrigin,

Tests/UnitTests/Core/Surfaces/DiscSurfaceTests.cpp

@@ -210,7 +210,7 @@ BOOST_AUTO_TEST_CASE(DiscSurfaceProperties) {
                             .closest();
   Intersection3D expectedIntersect{Vector3{1.2, 0., 0.}, 10.,
                                    Intersection3D::Status::reachable};
-  BOOST_CHECK(bool(sfIntersection));
+  BOOST_CHECK(sfIntersection.isValid());
   CHECK_CLOSE_ABS(sfIntersection.position(), expectedIntersect.position(),
                   1e-9);
   CHECK_CLOSE_ABS(sfIntersection.pathLength(), expectedIntersect.pathLength(),