refactor: Do not insert space points if not inside grid boundaries (#…

…3698)

Changes:
- Calling `grid.isInside` to check if space point falls within the boundaries
- Check min and max values are correct (i.e. min < max)
- Additional changes to check phi is within `[-std::numbers::pi`, `std::numbers::pi`]
- Move config and options values to `double` (also because units are `double`s). Only partial since moving them all requires change in other parts of the code (will follow up in another PR)
- Use `std::numbers::pi` instead of `M_PI`

In the example, we go back to the previous computation for the `rMiddleSPRange`. Not clear why the `std::floor` was introduced in the first place, going back to that computation while we figure out why

Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp

@@ -12,6 +12,7 @@
 #include "Acts/Seeding/SeedFinderConfig.hpp"
 #include "Acts/Utilities/Grid.hpp"
 
+#include <numbers>
 #include <vector>
 
 namespace Acts {
@@ -56,9 +57,9 @@ struct CylindricalSpacePointGridConfig {
   // maximum impact parameter in mm
   float impactMax = 0 * Acts::UnitConstants::mm;
   // minimum phi value for phiAxis construction
-  float phiMin = -M_PI;
+  float phiMin = -std::numbers::pi_v<float>;
   // maximum phi value for phiAxis construction
-  float phiMax = M_PI;
+  float phiMax = std::numbers::pi_v<float>;
   // Multiplicator for the number of phi-bins. The minimum number of phi-bins
   // depends on min_pt, magnetic field: 2*M_PI/(minPT particle phi-deflection).
   // phiBinDeflectionCoverage is a multiplier for this number. If
@@ -78,6 +79,7 @@ struct CylindricalSpacePointGridConfig {
           "Repeated conversion to internal units for "
           "CylindricalSpacePointGridConfig");
     }
+
     using namespace Acts::UnitLiterals;
     CylindricalSpacePointGridConfig config = *this;
     config.isInInternalUnits = true;
@@ -88,13 +90,36 @@ struct CylindricalSpacePointGridConfig {
     config.zMin /= 1_mm;
     config.deltaRMax /= 1_mm;
 
+    if (config.phiMin < -std::numbers::pi_v<float> ||
+        config.phiMax > std::numbers::pi_v<float>) {
+      throw std::runtime_error(
+          "CylindricalSpacePointGridConfig: phiMin (" +
+          std::to_string(config.phiMin) + ") and/or phiMax (" +
+          std::to_string(config.phiMax) +
+          ") are outside "
+          "the allowed phi range, defined as "
+          "[-std::numbers::pi_v<float>, std::numbers::pi_v<float>]");
+    }
+    if (config.phiMin > config.phiMax) {
+      throw std::runtime_error(
+          "CylindricalSpacePointGridConfig: phiMin is bigger then phiMax");
+    }
+    if (config.rMin > config.rMax) {
+      throw std::runtime_error(
+          "CylindricalSpacePointGridConfig: rMin is bigger then rMax");
+    }
+    if (config.zMin > config.zMax) {
+      throw std::runtime_error(
+          "CylindricalSpacePointGridConfig: zMin is bigger than zMax");
+    }
+
     return config;
   }
 };
 
 struct CylindricalSpacePointGridOptions {
   // magnetic field
-  float bFieldInZ = 0;
+  float bFieldInZ = 0.;
   bool isInInternalUnits = false;
   CylindricalSpacePointGridOptions toInternalUnits() const {
     if (isInInternalUnits) {

Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp

@@ -167,9 +167,11 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
 
   // Space points are assumed to be ALREADY CORRECTED for beamspot position
   // phi, z and r space point selection comes naturally from the
-  // grid axis definition. No need to explicitly cut on those values.
+  // grid axis definition. Calling `isInside` will let us know if we are
+  // inside the grid range.
   // If a space point is outside the validity range of these quantities
-  // it goes in an over- or under-flow bin.
+  // it goes in an over- or under-flow bin. We want to avoid to consider those
+  // and skip some computations.
   // Additional cuts can be applied by customizing the space point selector
   // in the config object.
 
@@ -189,8 +191,12 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
     }
 
     // fill rbins into grid
-    std::size_t globIndex = grid.globalBinFromPosition(
-        Acts::Vector3{sp.phi(), sp.z(), sp.radius()});
+    Acts::Vector3 position(sp.phi(), sp.z(), sp.radius());
+    if (!grid.isInside(position)) {
+      continue;
+    }
+
+    std::size_t globIndex = grid.globalBinFromPosition(position);
     auto& rbin = grid.at(globIndex);
     rbin.push_back(&sp);
 

Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp

@@ -274,8 +274,10 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute(
 
   /// variable middle SP radial region of interest
   const Acts::Range1D<float> rMiddleSPRange(
-      minRange + m_cfg.seedFinderConfig.deltaRMiddleMinSPRange,
-      maxRange - m_cfg.seedFinderConfig.deltaRMiddleMaxSPRange);
+      std::floor(minRange / 2) * 2 +
+          m_cfg.seedFinderConfig.deltaRMiddleMinSPRange,
+      std::floor(maxRange / 2) * 2 -
+          m_cfg.seedFinderConfig.deltaRMiddleMaxSPRange);
 
   // run the seeding
   static thread_local std::vector<seed_type> seeds;

Examples/Python/python/acts/examples/itk.py

@@ -372,8 +372,8 @@ def itkSeedingAlgConfig(
     )
     zOriginWeightFactor = 1
     compatSeedWeight = 100
-    phiMin = 0
-    phiMax = 2 * math.pi
+    phiMin = -math.pi
+    phiMax = math.pi
     phiBinDeflectionCoverage = 3
     numPhiNeighbors = 1
     maxPhiBins = 200

Examples/Python/python/acts/examples/reconstruction.py

@@ -2,6 +2,7 @@
 from typing import Optional, Union, List
 from enum import Enum
 from collections import namedtuple
+import math
 
 import acts
 import acts.examples

Examples/Scripts/Python/full_chain_odd.py

@@ -3,6 +3,7 @@
 import os
 import argparse
 import pathlib
+import math
 
 import acts
 import acts.examples
@@ -412,8 +413,8 @@
                 maxSharedTracksPerMeasurement=2,
                 pTMax=1400,
                 pTMin=0.5,
-                phiMax=3.14,
-                phiMin=-3.14,
+                phiMax=math.pi,
+                phiMin=-math.pi,
                 etaMax=4,
                 etaMin=-4,
                 useAmbiguityFunction=False,