From c2015b0ccbf04b7315bb4acef3de902b7eea9a4a Mon Sep 17 00:00:00 2001
From: Andreas Stefl <stefl.andreas@gmail.com>
Date: Thu, 24 Oct 2024 09:19:46 +0200
Subject: [PATCH] fix: Improve `TrackParameterHelpers.hpp` (#3766)

- unit tests
- implement `addBoundParameters`
- revisit `subtractBoundParameters`
---
 .../Acts/EventData/TrackParameterHelpers.hpp  | 19 ++++++--
 Tests/UnitTests/Core/EventData/CMakeLists.txt |  1 +
 .../EventData/TrackParameterHelpersTests.cpp  | 43 +++++++++++++++++++
 3 files changed, 59 insertions(+), 4 deletions(-)
 create mode 100644 Tests/UnitTests/Core/EventData/TrackParameterHelpersTests.cpp

diff --git a/Core/include/Acts/EventData/TrackParameterHelpers.hpp b/Core/include/Acts/EventData/TrackParameterHelpers.hpp
index cd68b5ae6a4..c9089ff9ab9 100644
--- a/Core/include/Acts/EventData/TrackParameterHelpers.hpp
+++ b/Core/include/Acts/EventData/TrackParameterHelpers.hpp
@@ -8,7 +8,6 @@
 
 #pragma once
 
-#include "Acts/Definitions/Algebra.hpp"
 #include "Acts/Definitions/TrackParametrization.hpp"
 #include "Acts/Utilities/detail/periodic.hpp"
 
@@ -26,8 +25,20 @@ inline BoundVector normalizeBoundParameters(const BoundVector& boundParams) {
   return result;
 }
 
+/// Add bound parameters and take care of angle periodicity for phi and theta.
+/// This is intended for small differences only i.e. KF updates.
+///
+/// @param lhs The left hand side bound parameters
+/// @param rhs The right hand side bound parameters
+///
+/// @return The sum of the bound parameters
+inline BoundVector addBoundParameters(const BoundVector& lhs,
+                                      const BoundVector& rhs) {
+  return normalizeBoundParameters(lhs + rhs);
+}
+
 /// Subtract bound parameters and take care of angle periodicity for phi and
-/// theta.
+/// theta. This is intended for small differences only i.e. KF updates.
 ///
 /// @param lhs The left hand side bound parameters
 /// @param rhs The right hand side bound parameters
@@ -36,8 +47,8 @@ inline BoundVector normalizeBoundParameters(const BoundVector& boundParams) {
 inline BoundVector subtractBoundParameters(const BoundVector& lhs,
                                            const BoundVector& rhs) {
   BoundVector result = lhs - rhs;
-  result[eBoundPhi] =
-      detail::difference_periodic(lhs[eBoundPhi], rhs[eBoundPhi], 2 * M_PI);
+  result[eBoundPhi] = detail::radian_sym(result[eBoundPhi]);
+  result[eBoundTheta] = detail::radian_sym(result[eBoundTheta]);
   return result;
 }
 
diff --git a/Tests/UnitTests/Core/EventData/CMakeLists.txt b/Tests/UnitTests/Core/EventData/CMakeLists.txt
index 61b5079da8e..1efda4cff29 100644
--- a/Tests/UnitTests/Core/EventData/CMakeLists.txt
+++ b/Tests/UnitTests/Core/EventData/CMakeLists.txt
@@ -16,5 +16,6 @@ add_unittest(MultiTrajectoryHelpers MultiTrajectoryHelpersTests.cpp)
 add_unittest(SubspaceHelpers SubspaceHelpersTests.cpp)
 add_unittest(SeedEdm SeedEdmTests.cpp)
 add_unittest(SpacePointContainerEdm SpacePointContainerEdmTests.cpp)
+add_unittest(TrackParameterHelpers TrackParameterHelpersTests.cpp)
 
 add_non_compile_test(MultiTrajectory TrackContainerComplianceTests.cpp)
diff --git a/Tests/UnitTests/Core/EventData/TrackParameterHelpersTests.cpp b/Tests/UnitTests/Core/EventData/TrackParameterHelpersTests.cpp
new file mode 100644
index 00000000000..0a6f3064594
--- /dev/null
+++ b/Tests/UnitTests/Core/EventData/TrackParameterHelpersTests.cpp
@@ -0,0 +1,43 @@
+// This file is part of the ACTS project.
+//
+// Copyright (C) 2016 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 https://mozilla.org/MPL/2.0/.
+
+#include <boost/test/unit_test.hpp>
+
+#include "Acts/Definitions/TrackParametrization.hpp"
+#include "Acts/EventData/TrackParameterHelpers.hpp"
+#include "Acts/Tests/CommonHelpers/FloatComparisons.hpp"
+
+BOOST_AUTO_TEST_SUITE(TrackParameterHelpers)
+
+BOOST_AUTO_TEST_CASE(normalizeBoundParameters) {
+  CHECK_CLOSE_OR_SMALL(Acts::normalizeBoundParameters({1, 2, 3, 4, 5, 6}),
+                       Acts::BoundVector(1, 2, -0.141593, 2.28319, 5, 6), 1e-3,
+                       1e-3);
+}
+
+BOOST_AUTO_TEST_CASE(addBoundParameters) {
+  CHECK_CLOSE_OR_SMALL(
+      Acts::addBoundParameters({1, 2, 3, 4, 5, 6}, {0, 0, 0, 0, 0, 0}),
+      Acts::normalizeBoundParameters({1, 2, 3, 4, 5, 6}), 1e-3, 1e-3);
+  CHECK_CLOSE_OR_SMALL(
+      Acts::addBoundParameters({1, 2, 3, 4, 5, 6}, {0, 0, 1, 1, 0, 0}),
+      Acts::normalizeBoundParameters({1, 2, 4, 5, 5, 6}), 1e-3, 1e-3);
+}
+
+BOOST_AUTO_TEST_CASE(subtractBoundParameters) {
+  CHECK_CLOSE_OR_SMALL(
+      Acts::subtractBoundParameters({1, 2, 3, 4, 5, 6}, {1, 2, 3, 4, 5, 6}),
+      Acts::BoundVector(0, 0, 0, 0, 0, 0), 1e-3, 1e-3);
+  CHECK_CLOSE_OR_SMALL(
+      Acts::addBoundParameters(
+          Acts::subtractBoundParameters({1, 2, 3, 4, 5, 6}, {0, 0, 1, 1, 0, 0}),
+          {0, 0, 1, 1, 0, 0}),
+      Acts::normalizeBoundParameters({1, 2, 3, 4, 5, 6}), 1e-3, 1e-3);
+}
+
+BOOST_AUTO_TEST_SUITE_END()