fix: newVertex not considered in AdaptiveMultiVertexFit

Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp

@@ -24,7 +24,8 @@ auto Acts::AdaptiveMultiVertexFinder<vfitter_t, sfinder_t>::find(
   // Seed tracks
   std::vector<const InputTrack_t*> seedTracks = allTracks;
 
-  FitterState_t fitterState(*m_cfg.bField, vertexingOptions.magFieldContext);
+  FitterState_t fitterState(*m_cfg.bField, vertexingOptions.magFieldContext,
+                            m_logger->cloneWithSuffix("State"));
   SeedFinderState_t seedFinderState;
 
   std::vector<std::unique_ptr<Vertex<InputTrack_t>>> allVertices;
@@ -560,6 +561,14 @@ auto Acts::AdaptiveMultiVertexFinder<vfitter_t, sfinder_t>::deleteLastVertex(
 
   // Update fitter state with removed vertex candidate
   fitterState.removeVertexFromMultiMap(vtx);
+  // fitterState.vertexCollection contains all vertices that will be fit. When
+  // we called addVtxToFit, vtx and all vertices that share tracks with vtx were
+  // added to vertexCollection. Now, we want to refit the same set of vertices
+  // excluding vx. Therefore, we remove vtx from vertexCollection.
+  auto removeResult = fitterState.removeVertexFromCollection(vtx);
+  if (!removeResult.ok()) {
+    return removeResult.error();
+  }
 
   for (auto& entry : fitterState.tracksAtVerticesMap) {
     // Delete all linearized tracks for current (bad) vertex
@@ -569,8 +578,8 @@ auto Acts::AdaptiveMultiVertexFinder<vfitter_t, sfinder_t>::deleteLastVertex(
   }
 
   // Do the fit with removed vertex
-  auto fitResult = m_cfg.vertexFitter.addVtxToFit(
-      fitterState, vtx, m_cfg.linearizer, vertexingOptions);
+  auto fitResult =
+      m_cfg.vertexFitter.fit(fitterState, m_cfg.linearizer, vertexingOptions);
   if (!fitResult.ok()) {
     return fitResult.error();
   }

Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.hpp

@@ -18,6 +18,7 @@
 #include "Acts/Vertexing/LinearizerConcept.hpp"
 #include "Acts/Vertexing/TrackAtVertex.hpp"
 #include "Acts/Vertexing/Vertex.hpp"
+#include "Acts/Vertexing/VertexingError.hpp"
 #include "Acts/Vertexing/VertexingOptions.hpp"
 
 #include <functional>
@@ -52,9 +53,12 @@ class AdaptiveMultiVertexFitter {
   /// @brief The fitter state
   struct State {
     State(const MagneticFieldProvider& field,
-          const Acts::MagneticFieldContext& magContext)
+          const Acts::MagneticFieldContext& magContext,
+          std::unique_ptr<const Logger> logger =
+              getDefaultLogger("AdaptiveMultiVertexFitterState", Logging::INFO))
         : ipState(field.makeCache(magContext)),
-          linearizerState(field.makeCache(magContext)) {}
+          linearizerState(field.makeCache(magContext)),
+          m_logger(std::move(logger)) {}
     // Vertex collection to be fitted
     std::vector<Vertex<InputTrack_t>*> vertexCollection;
 
@@ -77,6 +81,8 @@ class AdaptiveMultiVertexFitter {
              TrackAtVertex<InputTrack_t>>
         tracksAtVerticesMap;
 
+    std::unique_ptr<const Logger> m_logger;
+
     /// @brief Default State constructor
     State() = default;
 
@@ -98,6 +104,23 @@ class AdaptiveMultiVertexFitter {
         }
       }
     }
+
+    Result<void> removeVertexFromCollection(Vertex<InputTrack_t>& vtxToRemove) {
+      auto it = std::find(vertexCollection.begin(), vertexCollection.end(),
+                          &vtxToRemove);
+      // Check if the value was found before erasing
+      if (it == vertexCollection.end()) {
+        ACTS_ERROR("vtxToRemove is not part of vertexCollection.");
+        return VertexingError::InvalidMemoryAccess;
+      }
+      // Erase the element if found
+      vertexCollection.erase(it);
+      return {};
+    }
+
+   private:
+    /// Private access to logging instance
+    const Logger& logger() const { return *m_logger; }
   };
 
   struct Config {
@@ -170,20 +193,6 @@ class AdaptiveMultiVertexFitter {
         m_extractParameters(func),
         m_logger(std::move(logger)) {}
 
-  /// @brief Performs a simultaneous fit of all vertices in `verticesToFit`
-  /// by invoking `fitImpl`.
-  ///
-  /// @param state Fitter state
-  /// @param verticesToFit Vector containing all vertices to be fitted
-  /// @param linearizer Track linearizer
-  /// @param vertexingOptions Vertexing options
-  ///
-  /// @return Result<void> object
-  Result<void> fit(
-      State& state, const std::vector<Vertex<InputTrack_t>*>& verticesToFit,
-      const Linearizer_t& linearizer,
-      const VertexingOptions<InputTrack_t>& vertexingOptions) const;
-
   /// @brief Adds a new vertex to an existing multi-vertex fit.
   /// 1. The 3D impact parameters are calculated for all tracks associated
   /// with newVertex.
@@ -204,6 +213,18 @@ class AdaptiveMultiVertexFitter {
       const Linearizer_t& linearizer,
       const VertexingOptions<InputTrack_t>& vertexingOptions) const;
 
+  /// @brief Performs a simultaneous fit of all vertices in
+  /// state.vertexCollection
+  ///
+  /// @param state Fitter state
+  /// @param linearizer Track linearizer
+  /// @param vertexingOptions Vertexing options
+  ///
+  /// @return Result<void> object
+  Result<void> fit(
+      State& state, const Linearizer_t& linearizer,
+      const VertexingOptions<InputTrack_t>& vertexingOptions) const;
+
  private:
   /// Configuration object
   const Config m_cfg;
@@ -219,18 +240,6 @@ class AdaptiveMultiVertexFitter {
   /// Private access to logging instance
   const Logger& logger() const { return *m_logger; }
 
-  /// @brief Performs a simultaneous fit of all vertices in
-  /// state.vertexCollection
-  ///
-  /// @param state Fitter state
-  /// @param linearizer Track linearizer
-  /// @param vertexingOptions Vertexing options
-  ///
-  /// @return Result<void> object
-  Result<void> fitImpl(
-      State& state, const Linearizer_t& linearizer,
-      const VertexingOptions<InputTrack_t>& vertexingOptions) const;
-
   /// @brief Tests if vertex is already in list of vertices or not
   ///
   /// @param vtx Vertex to test

Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp

@@ -13,25 +13,6 @@
 template <typename input_track_t, typename linearizer_t>
 Acts::Result<void>
 Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::fit(
-    State& state, const std::vector<Vertex<input_track_t>*>& verticesToFit,
-    const linearizer_t& linearizer,
-    const VertexingOptions<input_track_t>& vertexingOptions) const {
-  // Set all vertices to fit in the current state
-  state.vertexCollection = verticesToFit;
-
-  // Perform fit on all vertices simultaneously
-  auto fitRes = fitImpl(state, linearizer, vertexingOptions);
-
-  if (!fitRes.ok()) {
-    return fitRes.error();
-  }
-
-  return {};
-}
-
-template <typename input_track_t, typename linearizer_t>
-Acts::Result<void>
-Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::fitImpl(
     State& state, const linearizer_t& linearizer,
     const VertexingOptions<input_track_t>& vertexingOptions) const {
   // Reset annealing tool
@@ -119,7 +100,7 @@ Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::addVtxToFit(
     return VertexingError::EmptyInput;
   }
 
-  std::vector<Vertex<input_track_t>*> verticesToFit;
+  std::vector<Vertex<input_track_t>*> verticesToFit = {&newVertex};
 
   // List of vertices added in last iteration
   std::vector<Vertex<input_track_t>*> lastIterAddedVertices = {&newVertex};
@@ -164,7 +145,7 @@ Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::addVtxToFit(
   state.vertexCollection = verticesToFit;
 
   // Perform fit on all added vertices
-  auto fitRes = fitImpl(state, linearizer, vertexingOptions);
+  auto fitRes = fit(state, linearizer, vertexingOptions);
   if (!fitRes.ok()) {
     return fitRes.error();
   }

Core/include/Acts/Vertexing/VertexingError.hpp

@@ -21,6 +21,7 @@ enum class VertexingError {
   NotConverged,
   ElementNotFound,
   NoCovariance,
+  InvalidMemoryAccess,
 };
 
 std::error_code make_error_code(Acts::VertexingError e);

Core/src/Vertexing/VertexingError.cpp

@@ -34,6 +34,8 @@ class VertexingErrorCategory : public std::error_category {
         return "Unable to find element.";
       case VertexingError::NoCovariance:
         return "No covariance provided.";
+      case VertexingError::InvalidMemoryAccess:
+        return "Invalid memory access.";
       default:
         return "unknown";
     }

Tests/UnitTests/Core/Vertexing/AdaptiveMultiVertexFitterTests.cpp

@@ -442,7 +442,6 @@ BOOST_AUTO_TEST_CASE(adaptive_multi_vertex_fitter_test_athena) {
                                    covMat2, ParticleHypothesis::pion())
           .value(),
   };
-  std::vector<Vertex<BoundTrackParameters>*> vtxList;
 
   AdaptiveMultiVertexFitter<BoundTrackParameters, Linearizer>::State state(
       *bField, magFieldContext);
@@ -457,7 +456,7 @@ BOOST_AUTO_TEST_CASE(adaptive_multi_vertex_fitter_test_athena) {
   Vertex<BoundTrackParameters> vtx1(vtxPos1);
 
   // Add to vertex list
-  vtxList.push_back(&vtx1);
+  state.vertexCollection.push_back(&vtx1);
 
   // The constraint vtx for vtx1
   Vertex<BoundTrackParameters> vtx1Constr(vtxPos1);
@@ -484,7 +483,7 @@ BOOST_AUTO_TEST_CASE(adaptive_multi_vertex_fitter_test_athena) {
   Vertex<BoundTrackParameters> vtx2(vtxPos2);
 
   // Add to vertex list
-  vtxList.push_back(&vtx2);
+  state.vertexCollection.push_back(&vtx2);
 
   // The constraint vtx for vtx2
   Vertex<BoundTrackParameters> vtx2Constr(vtxPos2);
@@ -513,7 +512,7 @@ BOOST_AUTO_TEST_CASE(adaptive_multi_vertex_fitter_test_athena) {
   state.addVertexToMultiMap(vtx2);
 
   // Fit vertices
-  fitter.fit(state, vtxList, linearizer, vertexingOptions);
+  fitter.fit(state, linearizer, vertexingOptions);
 
   auto vtx1Fitted = state.vertexCollection.at(0);
   auto vtx1PosFitted = vtx1Fitted->position();