From ccfd018de9351d14577bdbd1f746fdffe77b7236 Mon Sep 17 00:00:00 2001
From: Moritz Kiehn <msmk@cern.ch>
Date: Fri, 10 Mar 2017 08:16:52 +0100
Subject: [PATCH] Surfaces: remove FastArcTan

---
 Core/include/ACTS/Surfaces/BoundaryCheck.hpp  | 25 -------------------
 Core/include/ACTS/Surfaces/CylinderBounds.hpp |  7 +++---
 Core/include/ACTS/Surfaces/DiamondBounds.hpp  |  7 +++---
 .../ACTS/Surfaces/DiscTrapezoidalBounds.hpp   |  5 ++--
 Core/include/ACTS/Surfaces/LineBounds.hpp     |  7 +++---
 Core/include/ACTS/Surfaces/RadialBounds.hpp   | 15 ++++++-----
 .../include/ACTS/Surfaces/RectangleBounds.hpp |  5 ++--
 .../include/ACTS/Surfaces/TrapezoidBounds.hpp |  2 +-
 Core/include/ACTS/Surfaces/TriangleBounds.hpp |  5 ++--
 Tests/Surfaces/BoundaryCheckTests.cpp         | 14 -----------
 10 files changed, 23 insertions(+), 69 deletions(-)

diff --git a/Core/include/ACTS/Surfaces/BoundaryCheck.hpp b/Core/include/ACTS/Surfaces/BoundaryCheck.hpp
index 9dca2be13..fb2621b92 100644
--- a/Core/include/ACTS/Surfaces/BoundaryCheck.hpp
+++ b/Core/include/ACTS/Surfaces/BoundaryCheck.hpp
@@ -125,9 +125,6 @@ public:
   bool
   TestKDOPKDOP(std::vector<KDOP>& a, std::vector<KDOP>& b) const;
 
-  double
-  FastArcTan(double x) const;
-
   sincosCache
   FastSinCos(double x) const;
 
@@ -137,28 +134,6 @@ private:
   static double s_cos67;
 };
 
-/// should have maximum (average) error of 0.0015 (0.00089) radians or 0.0859
-/// (0.0509) degrees, fine for us and much faster (>4 times)
-inline double
-BoundaryCheck::FastArcTan(double x) const
-{
-  double y;
-  bool   complement = false;  // true if arg was >1
-  bool   sign       = false;  // true if arg was < 0
-  if (x < 0.) {
-    x    = -x;
-    sign = true;  // arctan(-x)=-arctan(x)
-  }
-  if (x > 1.) {
-    x          = 1. / x;  // keep arg between 0 and 1
-    complement = true;
-  }
-  y = M_PI_4 * x - x * (std::abs(x) - 1) * (0.2447 + 0.0663 * std::abs(x));
-  if (complement) y = M_PI_2 - y;  // correct for 1/x if we did that
-  if (sign) y       = -y;          // correct for negative arg
-  return y;
-}
-
 /// should have maximum (average) error of 0.001 (0.0005) radians or 0.0573
 /// (0.029) degrees, fine for us and much faster (>8 times)
 inline sincosCache
diff --git a/Core/include/ACTS/Surfaces/CylinderBounds.hpp b/Core/include/ACTS/Surfaces/CylinderBounds.hpp
index db2f8e166..7136528b2 100644
--- a/Core/include/ACTS/Surfaces/CylinderBounds.hpp
+++ b/Core/include/ACTS/Surfaces/CylinderBounds.hpp
@@ -215,10 +215,9 @@ CylinderBounds::inside(const Vector2D& lpos, const BoundaryCheck& bcheck) const
   float theta
       = ((*bcheck.lCovariance)(1, 0) != 0
          && ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)) != 0)
-      ? .5
-          * bcheck.FastArcTan(
-                2 * (*bcheck.lCovariance)(1, 0)
-                / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
+      ? .5 * std::atan(
+                 2 * (*bcheck.lCovariance)(1, 0)
+                 / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
       : 0.;
   sincosCache scResult = bcheck.FastSinCos(theta);
   double dphi    = scResult.sinC * scResult.sinC * (*bcheck.lCovariance)(0, 0);
diff --git a/Core/include/ACTS/Surfaces/DiamondBounds.hpp b/Core/include/ACTS/Surfaces/DiamondBounds.hpp
index 06e971c3a..c4feed579 100644
--- a/Core/include/ACTS/Surfaces/DiamondBounds.hpp
+++ b/Core/include/ACTS/Surfaces/DiamondBounds.hpp
@@ -275,10 +275,9 @@ DiamondBounds::inside(const Vector2D& lpos, const BoundaryCheck& bcheck) const
   float                 theta
       = ((*bcheck.lCovariance)(1, 0) != 0
          && ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)) != 0)
-      ? .5
-          * bcheck.FastArcTan(
-                2 * (*bcheck.lCovariance)(1, 0)
-                / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
+      ? .5 * std::atan(
+                 2 * (*bcheck.lCovariance)(1, 0)
+                 / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
       : 0.;
   auto rotMatrix = Eigen::Rotation2D<double>(theta).toRotationMatrix();
   ActsMatrixD<2, 2> normal;
diff --git a/Core/include/ACTS/Surfaces/DiscTrapezoidalBounds.hpp b/Core/include/ACTS/Surfaces/DiscTrapezoidalBounds.hpp
index 3b9e30280..520c9ea70 100644
--- a/Core/include/ACTS/Surfaces/DiscTrapezoidalBounds.hpp
+++ b/Core/include/ACTS/Surfaces/DiscTrapezoidalBounds.hpp
@@ -402,9 +402,8 @@ DiscTrapezoidalBounds::inside(const Vector2D&      lpos,
   double y0    = 0;
   float  theta = (lCovarianceCar(1, 0) != 0
                  && (lCovarianceCar(1, 1) - lCovarianceCar(0, 0)) != 0)
-      ? .5
-          * bcheck.FastArcTan(2 * lCovarianceCar(1, 0)
-                              / (lCovarianceCar(1, 1) - lCovarianceCar(0, 0)))
+      ? .5 * std::atan(2 * lCovarianceCar(1, 0)
+                       / (lCovarianceCar(1, 1) - lCovarianceCar(0, 0)))
       : 0.;
   auto     rotMatrix = Eigen::Rotation2D<double>(theta).toRotationMatrix();
   Vector2D tmp       = rotMatrix * (-lposCar);
diff --git a/Core/include/ACTS/Surfaces/LineBounds.hpp b/Core/include/ACTS/Surfaces/LineBounds.hpp
index 6e06c1feb..010ee69d8 100644
--- a/Core/include/ACTS/Surfaces/LineBounds.hpp
+++ b/Core/include/ACTS/Surfaces/LineBounds.hpp
@@ -163,10 +163,9 @@ LineBounds::inside(const Vector2D& lpos, const BoundaryCheck& bcheck) const
   float theta
       = ((*bcheck.lCovariance)(1, 0) != 0
          && ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)) != 0)
-      ? .5
-          * bcheck.FastArcTan(
-                2 * (*bcheck.lCovariance)(1, 0)
-                / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
+      ? .5 * std::atan(
+                 2 * (*bcheck.lCovariance)(1, 0)
+                 / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
       : 0.;
   sincosCache scResult = bcheck.FastSinCos(theta);
   double dphi    = scResult.sinC * scResult.sinC * (*bcheck.lCovariance)(0, 0);
diff --git a/Core/include/ACTS/Surfaces/RadialBounds.hpp b/Core/include/ACTS/Surfaces/RadialBounds.hpp
index 736614e97..ecfc1dd18 100644
--- a/Core/include/ACTS/Surfaces/RadialBounds.hpp
+++ b/Core/include/ACTS/Surfaces/RadialBounds.hpp
@@ -358,15 +358,14 @@ RadialBounds::inside(const Vector2D& lpos, const BoundaryCheck& bcheck) const
   double y0    = 0;
   float  theta = (lCovarianceCar(1, 0) != 0
                  && (lCovarianceCar(1, 1) - lCovarianceCar(0, 0)) != 0)
-      ? .5
-          * bcheck.FastArcTan(2 * lCovarianceCar(1, 0)
-                              / (lCovarianceCar(1, 1) - lCovarianceCar(0, 0)))
+      ? .5 * std::atan(2 * lCovarianceCar(1, 0)
+                       / (lCovarianceCar(1, 1) - lCovarianceCar(0, 0)))
       : 0.;
-  auto rotMatrix = Eigen::Rotation2D<double>(theta).toRotationMatrix();
-  Vector2D tmp = rotMatrix * (-lposCar);
-  double   x1  = tmp(0, 0);
-  double   y1  = tmp(1, 0);
-  double   r   = m_valueStore[RadialBounds::bv_rMax];
+  auto     rotMatrix = Eigen::Rotation2D<double>(theta).toRotationMatrix();
+  Vector2D tmp       = rotMatrix * (-lposCar);
+  double   x1        = tmp(0, 0);
+  double   y1        = tmp(1, 0);
+  double   r         = m_valueStore[RadialBounds::bv_rMax];
   // check if ellipse and circle overlap and return result
   return test.collide(x0, y0, w, h, x1, y1, r);
 }
diff --git a/Core/include/ACTS/Surfaces/RectangleBounds.hpp b/Core/include/ACTS/Surfaces/RectangleBounds.hpp
index 63a41a893..f0fa736bc 100644
--- a/Core/include/ACTS/Surfaces/RectangleBounds.hpp
+++ b/Core/include/ACTS/Surfaces/RectangleBounds.hpp
@@ -175,9 +175,8 @@ RectangleBounds::inside(const Vector2D& lpos, const BoundaryCheck& bcheck) const
       = ((*bcheck.lCovariance)(1, 0) != 0
          && ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)) != 0)
       ? .5
-          * bcheck.FastArcTan(
-                2 * (*bcheck.lCovariance)(1, 0)
-                / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
+          * std::atan(2 * (*bcheck.lCovariance)(1, 0)
+                            / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
       : 0.;
   auto rotMatrix = Eigen::Rotation2D<double>(theta).toRotationMatrix();
   // ellipse is always at (0,0), surface is moved to ellipse position and then
diff --git a/Core/include/ACTS/Surfaces/TrapezoidBounds.hpp b/Core/include/ACTS/Surfaces/TrapezoidBounds.hpp
index 0f5370bb0..9e2355ff2 100644
--- a/Core/include/ACTS/Surfaces/TrapezoidBounds.hpp
+++ b/Core/include/ACTS/Surfaces/TrapezoidBounds.hpp
@@ -326,7 +326,7 @@ TrapezoidBounds::inside(const Vector2D& lpos, const BoundaryCheck& bcheck) const
       = ((*bcheck.lCovariance)(1, 0) != 0
          && ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)) != 0)
       ? .5
-          * bcheck.FastArcTan(
+          * std::atan(
                 2 * (*bcheck.lCovariance)(1, 0)
                 / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
       : 0.;
diff --git a/Core/include/ACTS/Surfaces/TriangleBounds.hpp b/Core/include/ACTS/Surfaces/TriangleBounds.hpp
index 04503f0c9..2d7a571e4 100644
--- a/Core/include/ACTS/Surfaces/TriangleBounds.hpp
+++ b/Core/include/ACTS/Surfaces/TriangleBounds.hpp
@@ -217,9 +217,8 @@ TriangleBounds::inside(const Vector2D& lpos, const BoundaryCheck& bcheck) const
       = ((*bcheck.lCovariance)(1, 0) != 0
          && ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)) != 0)
       ? .5
-          * bcheck.FastArcTan(
-                2 * (*bcheck.lCovariance)(1, 0)
-                / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
+          * std::atan(2 * (*bcheck.lCovariance)(1, 0)
+                            / ((*bcheck.lCovariance)(1, 1) - (*bcheck.lCovariance)(0, 0)))
       : 0.;
   auto rotMatrix = Eigen::Rotation2D<double>(theta).toRotationMatrix();
   ActsMatrixD<2, 2> normal;
diff --git a/Tests/Surfaces/BoundaryCheckTests.cpp b/Tests/Surfaces/BoundaryCheckTests.cpp
index 4c81427fa..bff7129e2 100644
--- a/Tests/Surfaces/BoundaryCheckTests.cpp
+++ b/Tests/Surfaces/BoundaryCheckTests.cpp
@@ -141,20 +141,6 @@ namespace Test {
     BOOST_CHECK_SMALL(fastCos - cosValue,
                       0.001);  // claimed max difference in code
   }
-  /// Unit test for FastArcTan
-  BOOST_DATA_TEST_CASE(BoundaryCheckFastArcTanAccuracy,
-                       bdata::xrange(0., 200., 10.))
-  {
-    BoundaryCheck b(true);
-    // NOTE: Naming violation
-    double       fastArctan = b.FastArcTan(sample);
-    const double atanValue  = std::atan(sample);
-    // NOTE: Use CHECK_SMALL on the difference to check within absolute
-    // tolerance
-    // because BOOST_TEST(a==b, tolerance) checks the relative tolerance.
-    BOOST_CHECK_SMALL(fastArctan - atanValue,
-                      0.0015);  // claimed max difference in code
-  }
 
   /// Unit test for conversion of smooth ellipse to 4 + 4*n point polygon
   BOOST_AUTO_TEST_CASE(BoundaryCheckEllipseToPoly)
-- 
GitLab