chore: Follow-up after SurfaceArray changes in #5186

Core/include/Acts/Geometry/LayerCreator.hpp

@@ -220,14 +220,6 @@ class LayerCreator {
   void associateSurfacesToLayer(Layer& layer) const;
 
  private:
-  /// Validates that all the sensitive surfaces are actually accessible through
-  /// the binning
-  ///
-  /// @param gctx Geometry context to work with
-  /// @param sArray @c SurfaceArray instance to check
-  bool checkBinning(const GeometryContext& gctx,
-                    const SurfaceArray& sArray) const;
-
   /// configuration object
   Config m_cfg;
 

Core/include/Acts/Surfaces/SurfaceArray.hpp

@@ -68,6 +68,7 @@ class SurfaceArray {
     /// Performs lookup at global bin and returns bin content as const reference
     /// @param bin Global lookup bin
     /// @return A vector of surfaces at given bin
+    [[deprecated("Use at(gridIndices, neighborDistance) instead")]]
     virtual const std::vector<const Surface*>& lookup(
         std::size_t bin) const = 0;
 
@@ -221,7 +222,9 @@ class SurfaceArray {
     /// @param bin Global lookup bin
     /// @return A vector of surfaces at given bin
     const std::vector<const Surface*>& lookup(std::size_t bin) const override {
-      return const_cast<const ISurfaceGridLookup*>(m_impl.get())->lookup(bin);
+      ACTS_PUSH_IGNORE_DEPRECATED()
+      return m_impl->lookup(bin);
+      ACTS_POP_IGNORE_DEPRECATED()
     }
 
     /// Performs a lookup at @c pos, but returns neighbors as well
@@ -440,6 +443,7 @@ class SurfaceArray {
   /// @param position the lookup position
   /// @param direction the lookup direction
   /// @return const reference to surface vector contained in bin at that position
+  [[deprecated("Use at with GeometryContext instead")]]
   const std::vector<const Surface*>& at(const Vector3& position,
                                         const Vector3& direction) const {
     ACTS_PUSH_IGNORE_DEPRECATED()
@@ -472,9 +476,11 @@ class SurfaceArray {
   /// Get all surfaces in bin given by global bin index.
   /// @param bin the global bin index
   /// @return const reference to surface vector contained in bin
+  [[deprecated("Use at(gridIndices, neighborDistance) instead")]]
   const std::vector<const Surface*>& at(std::size_t bin) const {
-    return const_cast<const ISurfaceGridLookup*>(m_gridLookup.get())
-        ->lookup(bin);
+    ACTS_PUSH_IGNORE_DEPRECATED()
+    return m_gridLookup->lookup(bin);
+    ACTS_POP_IGNORE_DEPRECATED()
   }
 
   /// Get all surfaces in bin at @p pos and its neighbors

Core/src/Geometry/LayerCreator.cpp

@@ -20,6 +20,7 @@
 #include "Acts/Surfaces/RadialBounds.hpp"
 #include "Acts/Surfaces/RectangleBounds.hpp"
 #include "Acts/Surfaces/Surface.hpp"
+#include "Acts/Utilities/Diagnostics.hpp"
 
 #include <algorithm>
 #include <iterator>
@@ -103,8 +104,6 @@ MutableLayerPtr LayerCreator::cylinderLayer(
     sArray = m_cfg.surfaceArrayCreator->surfaceArrayOnCylinder(
         gctx, std::move(surfaces), binsPhi, binsZ, protoLayer, fullTransform,
         maxNeighborDistance);
-
-    checkBinning(gctx, *sArray);
   }
 
   // create the layer and push it back
@@ -183,8 +182,6 @@ MutableLayerPtr LayerCreator::cylinderLayer(
     sArray = m_cfg.surfaceArrayCreator->surfaceArrayOnCylinder(
         gctx, std::move(surfaces), bTypePhi, bTypeZ, protoLayer, fullTransform,
         maxNeighborDistance);
-
-    checkBinning(gctx, *sArray);
   }
 
   // create the layer and push it back
@@ -253,8 +250,6 @@ MutableLayerPtr LayerCreator::discLayer(
     sArray = m_cfg.surfaceArrayCreator->surfaceArrayOnDisc(
         gctx, std::move(surfaces), binsR, binsPhi, protoLayer, fullTransform,
         maxNeighborDistance);
-
-    checkBinning(gctx, *sArray);
   }
 
   // create the share disc bounds
@@ -325,8 +320,6 @@ MutableLayerPtr LayerCreator::discLayer(
     sArray = m_cfg.surfaceArrayCreator->surfaceArrayOnDisc(
         gctx, std::move(surfaces), bTypeR, bTypePhi, protoLayer, fullTransform,
         maxNeighborDistance);
-
-    checkBinning(gctx, *sArray);
   }
 
   // create the shared disc bounds
@@ -418,8 +411,6 @@ MutableLayerPtr LayerCreator::planeLayer(
     sArray = m_cfg.surfaceArrayCreator->surfaceArrayOnPlane(
         gctx, std::move(surfaces), bins1, bins2, aDir, protoLayer,
         fullTransform, maxNeighborDistance);
-
-    checkBinning(gctx, *sArray);
   }
 
   // create the layer and push it back
@@ -450,73 +441,4 @@ void LayerCreator::associateSurfacesToLayer(Layer& layer) const {
   }
 }
 
-bool LayerCreator::checkBinning(const GeometryContext& gctx,
-                                const SurfaceArray& sArray) const {
-  // do consistency check: can we access all sensitive surfaces
-  // through the binning? If not, surfaces get lost and the binning does not
-  // work
-
-  ACTS_VERBOSE("Performing consistency check");
-
-  std::vector<const Surface*> surfaces = sArray.surfaces();
-  std::set<const Surface*> sensitiveSurfaces(surfaces.begin(), surfaces.end());
-  std::set<const Surface*> accessibleSurfaces;
-  std::size_t nEmptyBins = 0;
-  std::size_t nBinsChecked = 0;
-
-  // iterate over all bins
-  std::size_t size = sArray.size();
-  for (std::size_t b = 0; b < size; ++b) {
-    std::vector<const Surface*> binContent = sArray.at(b);
-    // we don't check under/overflow bins
-    if (!sArray.isValidBin(b)) {
-      continue;
-    }
-    for (const auto& srf : binContent) {
-      accessibleSurfaces.insert(srf);
-    }
-    if (binContent.empty()) {
-      nEmptyBins++;
-    }
-    nBinsChecked++;
-  }
-
-  std::vector<const Surface*> diff;
-  std::set_difference(sensitiveSurfaces.begin(), sensitiveSurfaces.end(),
-                      accessibleSurfaces.begin(), accessibleSurfaces.end(),
-                      std::inserter(diff, diff.begin()));
-
-  ACTS_VERBOSE(" - Checked " << nBinsChecked << " valid bins");
-
-  if (nEmptyBins > 0) {
-    ACTS_VERBOSE(" -- Not all bins point to surface. " << nEmptyBins
-                                                       << " empty");
-  } else {
-    ACTS_VERBOSE(" -- All bins point to a surface");
-  }
-
-  if (!diff.empty()) {
-    ACTS_ERROR(
-        " -- Not all sensitive surfaces are accessible through binning. "
-        "sensitive: "
-        << sensitiveSurfaces.size()
-        << "    accessible: " << accessibleSurfaces.size());
-
-    // print all inaccessibles
-    ACTS_ERROR(" -- Inaccessible surfaces: ");
-    for (const auto& srf : diff) {
-      // have to choose AxisDirection here
-      Vector3 ctr = srf->referencePosition(gctx, AxisDirection::AxisR);
-      ACTS_ERROR(" Surface(x=" << ctr.x() << ", y=" << ctr.y()
-                               << ", z=" << ctr.z() << ", r=" << perp(ctr)
-                               << ", phi=" << phi(ctr) << ")");
-    }
-
-  } else {
-    ACTS_VERBOSE(" -- All sensitive surfaces are accessible through binning.");
-  }
-
-  return diff.empty();
-}
-
 }  // namespace Acts

Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp

@@ -15,76 +15,6 @@
 #include "Acts/Navigation/NavigationStream.hpp"
 
 namespace Acts {
-namespace {
-bool checkBinning(const GeometryContext& gctx, const SurfaceArray& sArray,
-                  const Logger& logger) {
-  // do consistency check: can we access all sensitive surfaces
-  // through the binning? If not, surfaces get lost and the binning does not
-  // work
-
-  ACTS_VERBOSE("Performing consistency check");
-
-  std::vector<const Surface*> surfaces = sArray.surfaces();
-  std::set<const Surface*> sensitiveSurfaces(surfaces.begin(), surfaces.end());
-  std::set<const Surface*> accessibleSurfaces;
-  std::size_t nEmptyBins = 0;
-  std::size_t nBinsChecked = 0;
-
-  // iterate over all bins
-  std::size_t size = sArray.size();
-  for (std::size_t b = 0; b < size; ++b) {
-    std::vector<const Surface*> binContent = sArray.at(b);
-    // we don't check under/overflow bins
-    if (!sArray.isValidBin(b)) {
-      continue;
-    }
-    for (const auto& srf : binContent) {
-      accessibleSurfaces.insert(srf);
-    }
-    if (binContent.empty()) {
-      nEmptyBins++;
-    }
-    nBinsChecked++;
-  }
-
-  std::vector<const Surface*> diff;
-  std::set_difference(sensitiveSurfaces.begin(), sensitiveSurfaces.end(),
-                      accessibleSurfaces.begin(), accessibleSurfaces.end(),
-                      std::inserter(diff, diff.begin()));
-
-  ACTS_VERBOSE(" - Checked " << nBinsChecked << " valid bins");
-
-  if (nEmptyBins > 0) {
-    ACTS_VERBOSE(" -- Not all bins point to surface. " << nEmptyBins
-                                                       << " empty");
-  } else {
-    ACTS_VERBOSE(" -- All bins point to a surface");
-  }
-
-  if (!diff.empty()) {
-    ACTS_ERROR(
-        " -- Not all sensitive surfaces are accessible through binning. "
-        "sensitive: "
-        << sensitiveSurfaces.size()
-        << "    accessible: " << accessibleSurfaces.size());
-
-    // print all inaccessibles
-    ACTS_ERROR(" -- Inaccessible surfaces: ");
-    for (const auto& srf : diff) {
-      // have to choose AxisDirection here
-      Vector3 ctr = srf->referencePosition(gctx, AxisDirection::AxisR);
-      ACTS_ERROR(" Surface(x=" << ctr.x() << ", y=" << ctr.y() << ", z="
-                               << ctr.z() << ", r=" << VectorHelpers::perp(ctr)
-                               << ", phi=" << VectorHelpers::phi(ctr) << ")");
-    }
-
-  } else {
-    ACTS_VERBOSE(" -- All sensitive surfaces are accessible through binning.");
-  }
-
-  return diff.empty();
-}
-}  // namespace
 
 SurfaceArrayNavigationPolicy::SurfaceArrayNavigationPolicy(
     const GeometryContext& gctx, const TrackingVolume& volume,
@@ -188,8 +118,6 @@ SurfaceArrayNavigationPolicy::SurfaceArrayNavigationPolicy(
     ACTS_ERROR("Failed to create surface array");
     throw std::runtime_error("Failed to create surface array");
   }
-
-  checkBinning(gctx, *m_surfaceArray, logger);
 }
 
 void SurfaceArrayNavigationPolicy::initializeCandidates(

Core/src/Surfaces/SurfaceArray.cpp

@@ -17,6 +17,7 @@
 #include "Acts/Utilities/Ranges.hpp"
 #include "Acts/Utilities/detail/grid_helper.hpp"
 
+#include <limits>
 #include <map>
 #include <ranges>
 #include <utility>
@@ -250,11 +251,16 @@ struct SurfaceGridLookupImpl final : SurfaceArray::ISurfaceGridLookup {
     const GridIndex localBins = localBinsFromPosition2D(gridLocal);
 
     const Vector3 normal = m_representative->normal(gctx, *surfaceLocal);
+    // using 1e-6 to avoid division by zero, the actual value does not matter as
+    // long as it is small compared to the angles we want to distinguish
     const double neighborDistanceReal = std::min<double>(
         m_maxNeighborDistance,
-        std::max<double>(1, 1 / std::abs(normal.dot(direction))));
+        std::max<double>(1, 1 / (1e-6 + std::abs(normal.dot(direction)))));
+    // clamp value to range before converting to std::uint8_t to avoid overflow
     const std::uint8_t neighborDistance =
-        static_cast<std::uint8_t>(neighborDistanceReal);
+        static_cast<std::uint8_t>(std::clamp<double>(
+            neighborDistanceReal, std::numeric_limits<std::uint8_t>::min(),
+            std::numeric_limits<std::uint8_t>::max()));
 
     const std::size_t globalBin =
         globalBinFromLocalBins3D(localBins, neighborDistance);