From 8a8fe8718ea9581be73f9e3b716251420fd8c4ff Mon Sep 17 00:00:00 2001
From: Bastian Schlag <bastian.schlag@cern.ch>
Date: Thu, 27 Feb 2020 18:02:46 +0100
Subject: [PATCH] use m_extractParameters function in AMVFitter again and use
 const ref for linearizeTrack

---
 .../Vertexing/AdaptiveMultiVertexFinder.ipp   | 19 ++++------
 .../Vertexing/AdaptiveMultiVertexFitter.ipp   |  6 +--
 .../Vertexing/FullBilloirVertexFitter.ipp     |  2 +-
 .../Acts/Vertexing/HelicalTrackLinearizer.hpp |  2 +-
 .../Acts/Vertexing/HelicalTrackLinearizer.ipp | 13 +++----
 .../Acts/Vertexing/IterativeVertexFinder.ipp  |  2 +-
 .../Acts/Vertexing/LinearizerConcept.hpp      |  2 +-
 .../AdaptiveMultiVertexFitterTests.cpp        |  4 +-
 Tests/UnitTests/Core/Vertexing/CMakeLists.txt |  2 +-
 .../KalmanVertexTrackUpdaterTests.cpp         |  2 +-
 .../Vertexing/KalmanVertexUpdaterTests.cpp    |  2 +-
 .../Vertexing/LinearizedTrackFactoryTests.cpp | 38 +------------------
 .../Core/Vertexing/VertexSmootherTests.cpp    |  2 +-
 13 files changed, 27 insertions(+), 69 deletions(-)

diff --git a/Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp b/Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp
index 9d883b324..a742bd332 100644
--- a/Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp
+++ b/Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp
@@ -18,18 +18,13 @@ auto Acts::AdaptiveMultiVertexFinder<vfitter_t, sfinder_t>::find(
     return VertexingError::EmptyInput;
   }
 
+  std::vector<const InputTrack_t*> seedTracks;
+  for(const auto& trk : allTracks){
+    seedTracks.push_back(&trk);
+  }
+
   // Original tracks
-  const std::vector<InputTrack_t>& origTracks = allTracks;
-  // Tracks for seeding
-  // Note: Remains to be investigated if another container (e.g. std::list)
-  // or also std::vector<InputTrack_t*> is a faster option since erasures
-  // of tracks is quite expensive with std::vector.
-  // std::vector<InputTrack_t*> would however also come with an overhead
-  // since m_cfg.vertexFitter.fit and m_cfg.seedFinder.find take
-  // vector<InputTrack_t> and hence a lot of copying would be required.
-  // Maybe use std::vector<InputTrack_t*> and adapt fit accordingly to
-  // also take pointers to tracks instead of the track object.
-  std::vector<InputTrack_t> seedTracks = allTracks;
+  const std::vector<const InputTrack_t*> origTracks = seedTracks;
 
   // Construct the vertex fitter options from vertex finder options
   VertexFitterOptions<InputTrack_t> vFitterOptions(
@@ -49,7 +44,7 @@ auto Acts::AdaptiveMultiVertexFinder<vfitter_t, sfinder_t>::find(
     // Tracks that are used for searching compatible tracks
     // near a vertex candidate
     // TODO: This involves a lot of copying. Change the way of accessing tracks
-    std::vector<InputTrack_t> myTracks;
+    std::vector<const InputTrack_t*> myTracks;
     if (m_cfg.realMultiVertex == true) {
       myTracks = origTracks;
     } else {
diff --git a/Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp b/Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp
index 4731b0fde..6f6cc71c5 100644
--- a/Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp
+++ b/Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp
@@ -217,7 +217,7 @@ Acts::Result<void> Acts::
   // Loop over all tracks at current vertex
   for (const auto& trk : currentVtxInfo.trackLinks) {
     auto res =
-        m_cfg.ipEst.getParamsAtClosestApproach(geoContext, *trk, seedPos);
+        m_cfg.ipEst.getParamsAtClosestApproach(geoContext, m_extractParameters(*trk), seedPos);
     if (!res.ok()) {
       return res.error();
     }
@@ -245,7 +245,7 @@ Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::
     if (currentVtxInfo.ip3dParams.find(trk) ==
         currentVtxInfo.ip3dParams.end()) {
       auto res = m_cfg.ipEst.getParamsAtClosestApproach(
-          geoContext, *trk, VectorHelpers::position(currentVtxInfo.linPoint));
+          geoContext, m_extractParameters(*trk), VectorHelpers::position(currentVtxInfo.linPoint));
       if (!res.ok()) {
         return res.error();
       }
@@ -293,7 +293,7 @@ Acts::Result<void> Acts::AdaptiveMultiVertexFitter<
                 BoundSymMatrix::Zero() ||
             state.vtxInfoMap[vtx].relinearize) {
           auto result =
-              linearizer.linearizeTrack(trk, state.vtxInfoMap[vtx].oldPosition);
+              linearizer.linearizeTrack(m_extractParameters(*trk), state.vtxInfoMap[vtx].oldPosition);
           if (!result.ok()) {
             return result.error();
           }
diff --git a/Core/include/Acts/Vertexing/FullBilloirVertexFitter.ipp b/Core/include/Acts/Vertexing/FullBilloirVertexFitter.ipp
index 3d62db028..f7574c9d6 100644
--- a/Core/include/Acts/Vertexing/FullBilloirVertexFitter.ipp
+++ b/Core/include/Acts/Vertexing/FullBilloirVertexFitter.ipp
@@ -114,7 +114,7 @@ Acts::FullBilloirVertexFitter<input_track_t, linearizer_t>::fit(
         trackMomenta.push_back(Vector3D(phi, theta, qop));
       }
 
-      auto result = linearizer.linearizeTrack(&trackParams, linPoint);
+      auto result = linearizer.linearizeTrack(trackParams, linPoint);
       if (result.ok()) {
         const auto linTrack = *result;
         double d0 = linTrack.parametersAtPCA[ParID_t::eLOC_D0];
diff --git a/Core/include/Acts/Vertexing/HelicalTrackLinearizer.hpp b/Core/include/Acts/Vertexing/HelicalTrackLinearizer.hpp
index d1c7d74c0..d2b6bedc2 100644
--- a/Core/include/Acts/Vertexing/HelicalTrackLinearizer.hpp
+++ b/Core/include/Acts/Vertexing/HelicalTrackLinearizer.hpp
@@ -107,7 +107,7 @@ class HelicalTrackLinearizer {
   ///
   /// @return Linearized track
   Result<LinearizedTrack> linearizeTrack(
-      const BoundParameters* params, const SpacePointVector& linPoint) const;
+      const BoundParameters& params, const SpacePointVector& linPoint) const;
 
  private:
   /// Configuration object
diff --git a/Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp b/Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp
index afd5a1ec0..c18d683e0 100644
--- a/Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp
+++ b/Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp
@@ -11,10 +11,7 @@
 template <typename propagator_t, typename propagator_options_t>
 Acts::Result<Acts::LinearizedTrack> Acts::
     HelicalTrackLinearizer<propagator_t, propagator_options_t>::linearizeTrack(
-        const BoundParameters* params, const SpacePointVector& linPoint) const {
-  if (params == nullptr) {
-    return LinearizedTrack();
-  }
+        const BoundParameters& params, const SpacePointVector& linPoint) const {
 
   Vector3D linPointPos = VectorHelpers::position(linPoint);
 
@@ -24,7 +21,7 @@ Acts::Result<Acts::LinearizedTrack> Acts::
   const BoundParameters* endParams = nullptr;
   // Do the propagation to linPointPos
   auto result =
-      m_cfg.propagator->propagate(*params, *perigeeSurface, m_cfg.pOptions);
+      m_cfg.propagator->propagate(params, *perigeeSurface, m_cfg.pOptions);
   if (result.ok()) {
     endParams = (*result).endParameters.get();
 
@@ -39,9 +36,9 @@ Acts::Result<Acts::LinearizedTrack> Acts::
 
   if (endParams->covariance()->determinant() == 0) {
     // Use the original parameters
-    paramsAtPCA = params->parameters();
-    VectorHelpers::position(positionAtPCA) = params->position();
-    parCovarianceAtPCA = *(params->covariance());
+    paramsAtPCA = params.parameters();
+    VectorHelpers::position(positionAtPCA) = params.position();
+    parCovarianceAtPCA = *(params.covariance());
   }
 
   // phiV and functions
diff --git a/Core/include/Acts/Vertexing/IterativeVertexFinder.ipp b/Core/include/Acts/Vertexing/IterativeVertexFinder.ipp
index fa4fad596..bf6946a8e 100644
--- a/Core/include/Acts/Vertexing/IterativeVertexFinder.ipp
+++ b/Core/include/Acts/Vertexing/IterativeVertexFinder.ipp
@@ -211,7 +211,7 @@ Acts::Result<double>
 Acts::IterativeVertexFinder<vfitter_t, sfinder_t>::getCompatibility(
     const BoundParameters& params, const Vertex<InputTrack_t>& vertex) const {
   // Linearize track
-  auto result = m_cfg.linearizer.linearizeTrack(&params, vertex.fullPosition());
+  auto result = m_cfg.linearizer.linearizeTrack(params, vertex.fullPosition());
   if (!result.ok()) {
     return result.error();
   }
diff --git a/Core/include/Acts/Vertexing/LinearizerConcept.hpp b/Core/include/Acts/Vertexing/LinearizerConcept.hpp
index 656ca5aa8..bfa7eeadf 100644
--- a/Core/include/Acts/Vertexing/LinearizerConcept.hpp
+++ b/Core/include/Acts/Vertexing/LinearizerConcept.hpp
@@ -30,7 +30,7 @@ namespace concept {
       struct LinearizerConcept {
 
          constexpr static bool linTrack_exists = has_method<const S, Result<LinearizedTrack>,
-         linTrack_t, const BoundParameters*,
+         linTrack_t, const BoundParameters&,
                      const SpacePointVector&>;
   
         static_assert(linTrack_exists, "linearizeTrack method not found");
diff --git a/Tests/UnitTests/Core/Vertexing/AdaptiveMultiVertexFitterTests.cpp b/Tests/UnitTests/Core/Vertexing/AdaptiveMultiVertexFitterTests.cpp
index bdff6e69d..7bb06769c 100644
--- a/Tests/UnitTests/Core/Vertexing/AdaptiveMultiVertexFitterTests.cpp
+++ b/Tests/UnitTests/Core/Vertexing/AdaptiveMultiVertexFitterTests.cpp
@@ -61,7 +61,7 @@ std::uniform_int_distribution<> nTracksDist(3, 10);
 /// @brief Unit test for AdaptiveMultiVertexFitter
 ///
 BOOST_AUTO_TEST_CASE(adaptive_multi_vertex_fitter_test) {
-  bool debugMode = true;
+  bool debugMode = false;
 
   // Set up RNG
   int mySeed = 31415;
@@ -310,7 +310,7 @@ BOOST_AUTO_TEST_CASE(adaptive_multi_vertex_fitter_test) {
 /// test values are used here
 BOOST_AUTO_TEST_CASE(adaptive_multi_vertex_fitter_test_athena) {
   // Set debug mode
-  bool debugMode = true;
+  bool debugMode = false;
   // Set up constant B-Field
   ConstantBField bField(Vector3D(0., 0., 2_T));
 
diff --git a/Tests/UnitTests/Core/Vertexing/CMakeLists.txt b/Tests/UnitTests/Core/Vertexing/CMakeLists.txt
index 37c700abd..cb9e130a7 100644
--- a/Tests/UnitTests/Core/Vertexing/CMakeLists.txt
+++ b/Tests/UnitTests/Core/Vertexing/CMakeLists.txt
@@ -5,7 +5,7 @@ add_unittest(KalmanVertexTrackUpdaterTests KalmanVertexTrackUpdaterTests.cpp)
 add_unittest(KalmanVertexUpdaterTests KalmanVertexUpdaterTests.cpp)
 add_unittest(LinearizedTrackFactoryTests LinearizedTrackFactoryTests.cpp)
 add_unittest(AdaptiveMultiVertexFitterTests AdaptiveMultiVertexFitterTests.cpp)
-# add_unittest(AdaptiveMultiVertexFinderTests AdaptiveMultiVertexFinderTests.cpp)
+#add_unittest(AdaptiveMultiVertexFinderTests AdaptiveMultiVertexFinderTests.cpp)
 add_unittest(TrackToVertexIPEstimatorTests TrackToVertexIPEstimatorTests.cpp)
 # add_unittest(VertexSmootherTests VertexSmootherTests.cpp)
 add_unittest(ZScanVertexFinderTests ZScanVertexFinderTests.cpp)
diff --git a/Tests/UnitTests/Core/Vertexing/KalmanVertexTrackUpdaterTests.cpp b/Tests/UnitTests/Core/Vertexing/KalmanVertexTrackUpdaterTests.cpp
index 53b91704d..7b69991ee 100644
--- a/Tests/UnitTests/Core/Vertexing/KalmanVertexTrackUpdaterTests.cpp
+++ b/Tests/UnitTests/Core/Vertexing/KalmanVertexTrackUpdaterTests.cpp
@@ -134,7 +134,7 @@ BOOST_AUTO_TEST_CASE(Kalman_Vertex_TrackUpdater) {
 
     // Linearized state of the track
     LinearizedTrack linTrack =
-        linearizer.linearizeTrack(&params, SpacePointVector::Zero()).value();
+        linearizer.linearizeTrack(params, SpacePointVector::Zero()).value();
 
     // Create TrackAtVertex
     TrackAtVertex<BoundParameters> trkAtVtx(0., params, params);
diff --git a/Tests/UnitTests/Core/Vertexing/KalmanVertexUpdaterTests.cpp b/Tests/UnitTests/Core/Vertexing/KalmanVertexUpdaterTests.cpp
index e8697189a..8708c4224 100644
--- a/Tests/UnitTests/Core/Vertexing/KalmanVertexUpdaterTests.cpp
+++ b/Tests/UnitTests/Core/Vertexing/KalmanVertexUpdaterTests.cpp
@@ -131,7 +131,7 @@ BOOST_AUTO_TEST_CASE(Kalman_Vertex_Updater) {
 
     // Linearized state of the track
     LinearizedTrack linTrack =
-        linearizer.linearizeTrack(&params, SpacePointVector::Zero()).value();
+        linearizer.linearizeTrack(params, SpacePointVector::Zero()).value();
 
     // Create TrackAtVertex
     TrackAtVertex<BoundParameters> trkAtVtx(0., params, params);
diff --git a/Tests/UnitTests/Core/Vertexing/LinearizedTrackFactoryTests.cpp b/Tests/UnitTests/Core/Vertexing/LinearizedTrackFactoryTests.cpp
index 4f59579a6..595e3b95b 100644
--- a/Tests/UnitTests/Core/Vertexing/LinearizedTrackFactoryTests.cpp
+++ b/Tests/UnitTests/Core/Vertexing/LinearizedTrackFactoryTests.cpp
@@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(linearized_track_factory_test) {
 
   for (const BoundParameters& parameters : tracks) {
     LinearizedTrack linTrack =
-        linFactory.linearizeTrack(&parameters, SpacePointVector::Zero())
+        linFactory.linearizeTrack(parameters, SpacePointVector::Zero())
             .value();
 
     BOOST_CHECK_NE(linTrack.parametersAtPCA, vecBoundZero);
@@ -148,40 +148,6 @@ BOOST_AUTO_TEST_CASE(linearized_track_factory_test) {
   }
 }
 
-BOOST_AUTO_TEST_CASE(linearized_track_factory_empty_test) {
-  ConstantBField bField(0.0, 0.0, 1_T);
-
-  // Set up Eigenstepper
-  EigenStepper<ConstantBField> stepper(bField);
-
-  // Set up propagator with void navigator
-  auto propagator =
-      std::make_shared<Propagator<EigenStepper<ConstantBField>>>(stepper);
-
-  PropagatorOptions<> pOptions(tgContext, mfContext);
-
-  Linearizer::Config ltConfig(bField, propagator, pOptions);
-  Linearizer linFactory(ltConfig);
-
-  BoundVector vecBoundZero = BoundVector::Zero();
-  BoundSymMatrix matBoundZero = BoundSymMatrix::Zero();
-  SpacePointVector vecSPZero = SpacePointVector::Zero();
-  SpacePointToBoundMatrix matBound2SPZero = SpacePointToBoundMatrix::Zero();
-  ActsMatrixD<BoundParsDim, 3> matBound2MomZero =
-      ActsMatrixD<BoundParsDim, 3>::Zero();
-
-  LinearizedTrack linTrack =
-      linFactory.linearizeTrack(nullptr, SpacePointVector(1., 2., 3., 4.))
-          .value();
-
-  BOOST_CHECK_EQUAL(linTrack.parametersAtPCA, vecBoundZero);
-  BOOST_CHECK_EQUAL(linTrack.covarianceAtPCA, matBoundZero);
-  BOOST_CHECK_EQUAL(linTrack.linearizationPoint, vecSPZero);
-  BOOST_CHECK_EQUAL(linTrack.positionJacobian, matBound2SPZero);
-  BOOST_CHECK_EQUAL(linTrack.momentumJacobian, matBound2MomZero);
-  BOOST_CHECK_EQUAL(linTrack.constantTerm, vecBoundZero);
-}
-
 ///
 /// @brief Unit test for HelicalTrackLinearizer
 ///
@@ -260,7 +226,7 @@ BOOST_AUTO_TEST_CASE(linearized_track_factory_straightline_test) {
 
   for (const BoundParameters& parameters : tracks) {
     LinearizedTrack linTrack =
-        linFactory.linearizeTrack(&parameters, SpacePointVector::Zero())
+        linFactory.linearizeTrack(parameters, SpacePointVector::Zero())
             .value();
 
     BOOST_CHECK_NE(linTrack.parametersAtPCA, vecBoundZero);
diff --git a/Tests/UnitTests/Core/Vertexing/VertexSmootherTests.cpp b/Tests/UnitTests/Core/Vertexing/VertexSmootherTests.cpp
index 9552cfc21..fc3e23639 100644
--- a/Tests/UnitTests/Core/Vertexing/VertexSmootherTests.cpp
+++ b/Tests/UnitTests/Core/Vertexing/VertexSmootherTests.cpp
@@ -159,7 +159,7 @@ BOOST_AUTO_TEST_CASE(sequential_vertex_smoother_test) {
     BoundParameters fittedParams = trackAtVtx.fittedParams;
 
     LinearizedTrack linTrack =
-        linearizer.linearizeTrack(&fittedParams, vertexPosition).value();
+        linearizer.linearizeTrack(fittedParams, vertexPosition).value();
     trackAtVtx.linearizedState = linTrack;
     tracksWithLinState.push_back(trackAtVtx);
   }
-- 
GitLab