refactor!: std::unique_ptr to std::optional in Acts::PropagatorResult (…

…#1622)

This changes the type that stores the parameters and covariance matrix
in the `Acts::PropagatorResult`.

I'm not sure why we have `std::unique_ptr` here, maybe this is from
pre-C++17 where `std::optional` was not available? However, both from
usibility (not copy-able) and from performance perspective (heap
allocation) this does not make sense to me, but maybe I forget
something...

Core/include/Acts/Propagator/Propagator.hpp

@@ -29,7 +29,7 @@
 
 #include <cmath>
 #include <functional>
-#include <memory>
+#include <optional>
 #include <type_traits>
 
 #include <boost/algorithm/string.hpp>
@@ -46,11 +46,11 @@ struct PropagatorResult : private detail::Extendable<result_list...> {
   /// Accessor to additional propagation quantities
   using detail::Extendable<result_list...>::get;
 
-  /// Final track parameters - initialized to null pointer
-  std::unique_ptr<parameters_t> endParameters = nullptr;
+  /// Final track parameters
+  std::optional<parameters_t> endParameters = std::nullopt;
 
   /// Full transport jacobian
-  std::unique_ptr<BoundMatrix> transportJacobian = nullptr;
+  std::optional<BoundMatrix> transportJacobian = std::nullopt;
 
   /// Number of propagation steps that were carried out
   unsigned int steps = 0;

Core/include/Acts/Propagator/Propagator.ipp

@@ -160,15 +160,11 @@ auto Acts::Propagator<S, N>::propagate(
   if (result.ok()) {
     /// Convert into return type and fill the result object
     auto curvState = m_stepper.curvilinearState(state.stepping);
-    auto& curvParameters = std::get<CurvilinearTrackParameters>(curvState);
     // Fill the end parameters
-    inputResult.endParameters =
-        std::make_unique<CurvilinearTrackParameters>(std::move(curvParameters));
+    inputResult.endParameters = std::get<CurvilinearTrackParameters>(curvState);
     // Only fill the transport jacobian when covariance transport was done
     if (state.stepping.covTransport) {
-      auto& tJacobian = std::get<Jacobian>(curvState);
-      inputResult.transportJacobian =
-          std::make_unique<Jacobian>(std::move(tJacobian));
+      inputResult.transportJacobian = std::get<Jacobian>(curvState);
     }
     return Result<ResultType>::success(std::forward<ResultType>(inputResult));
   } else {
@@ -258,15 +254,11 @@ auto Acts::Propagator<S, N>::propagate(
 
     const auto& bs = *bsRes;
 
-    auto& boundParams = std::get<BoundTrackParameters>(bs);
     // Fill the end parameters
-    inputResult.endParameters =
-        std::make_unique<BoundTrackParameters>(std::move(boundParams));
+    inputResult.endParameters = std::get<BoundTrackParameters>(bs);
     // Only fill the transport jacobian when covariance transport was done
     if (state.stepping.covTransport) {
-      auto& tJacobian = std::get<Jacobian>(bs);
-      inputResult.transportJacobian =
-          std::make_unique<Jacobian>(std::move(tJacobian));
+      inputResult.transportJacobian = std::get<Jacobian>(bs);
     }
     return Result<ResultType>::success(std::forward<ResultType>(inputResult));
   } else {

Core/include/Acts/TrackFitting/GaussianSumFitter.hpp

@@ -474,7 +474,7 @@ struct GaussianSumFitter {
       ACTS_VERBOSE("+-----------------------------------------------+");
       ACTS_VERBOSE("| Gsf: Do propagation back to reference surface |");
       ACTS_VERBOSE("+-----------------------------------------------+");
-      auto lastResult = [&]() -> Result<std::unique_ptr<BoundTrackParameters>> {
+      auto lastResult = [&]() -> Result<std::optional<BoundTrackParameters>> {
         const auto& [surface, lastSmoothedState] =
             std::get<1>(smoothResult).front();
 

Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp

@@ -206,7 +206,7 @@ Acts::Result<void> Acts::
       return res.error();
     }
     // Set ip3dParams for current trackAtVertex
-    currentVtxInfo.ip3dParams.emplace(trk, *(res.value()));
+    currentVtxInfo.ip3dParams.emplace(trk, res.value());
   }
   return {};
 }
@@ -236,7 +236,7 @@ Acts::AdaptiveMultiVertexFitter<input_track_t, linearizer_t>::
         return res.error();
       }
       // Set ip3dParams for current trackAtVertex
-      currentVtxInfo.ip3dParams.emplace(trk, *(res.value()));
+      currentVtxInfo.ip3dParams.emplace(trk, res.value());
     }
     // Set compatibility with current vertex
     auto compRes = m_cfg.ipEst.get3dVertexCompatibility(
@@ -342,4 +342,4 @@ void Acts::AdaptiveMultiVertexFitter<
       }
     }
   }
-}
+}

Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp

@@ -29,30 +29,29 @@ Acts::Result<Acts::LinearizedTrack> Acts::
                            ? NavigationDirection::Forward
                            : NavigationDirection::Backward;
 
-  const BoundTrackParameters* endParams = nullptr;
   // Do the propagation to linPointPos
   auto result = m_cfg.propagator->propagate(params, *perigeeSurface, pOptions);
-  if (result.ok()) {
-    endParams = (*result).endParameters.get();
-  } else {
+  if (not result.ok()) {
     return result.error();
   }
 
-  BoundVector paramsAtPCA = endParams->parameters();
+  const auto& endParams = *result->endParameters;
+
+  BoundVector paramsAtPCA = endParams.parameters();
   Vector4 positionAtPCA = Vector4::Zero();
   {
-    auto pos = endParams->position(gctx);
+    auto pos = endParams.position(gctx);
     positionAtPCA[ePos0] = pos[ePos0];
     positionAtPCA[ePos1] = pos[ePos1];
     positionAtPCA[ePos2] = pos[ePos2];
-    positionAtPCA[eTime] = endParams->time();
+    positionAtPCA[eTime] = endParams.time();
   }
-  BoundSymMatrix parCovarianceAtPCA = endParams->covariance().value();
+  BoundSymMatrix parCovarianceAtPCA = endParams.covariance().value();
 
   if (parCovarianceAtPCA.determinant() <= 0) {
     // Use the original parameters
     paramsAtPCA = params.parameters();
-    auto pos = endParams->position(gctx);
+    auto pos = endParams.position(gctx);
     positionAtPCA[ePos0] = pos[ePos0];
     positionAtPCA[ePos1] = pos[ePos1];
     positionAtPCA[ePos2] = pos[ePos2];

Core/include/Acts/Vertexing/ImpactPointEstimator.hpp

@@ -112,11 +112,10 @@ class ImpactPointEstimator {
   /// @param state The state object
   ///
   /// @return New track params
-  Result<std::unique_ptr<const BoundTrackParameters>>
-  estimate3DImpactParameters(const GeometryContext& gctx,
-                             const Acts::MagneticFieldContext& mctx,
-                             const BoundTrackParameters& trkParams,
-                             const Vector3& vtxPos, State& state) const;
+  Result<BoundTrackParameters> estimate3DImpactParameters(
+      const GeometryContext& gctx, const Acts::MagneticFieldContext& mctx,
+      const BoundTrackParameters& trkParams, const Vector3& vtxPos,
+      State& state) const;
 
   /// @brief Estimates the compatibility of a
   /// track to a vertex position based on the 3d

Core/include/Acts/Vertexing/ImpactPointEstimator.ipp

@@ -35,7 +35,7 @@ Acts::ImpactPointEstimator<input_track_t, propagator_t, propagator_options_t>::
 
 template <typename input_track_t, typename propagator_t,
           typename propagator_options_t>
-Acts::Result<std::unique_ptr<const Acts::BoundTrackParameters>>
+Acts::Result<Acts::BoundTrackParameters>
 Acts::ImpactPointEstimator<input_track_t, propagator_t, propagator_options_t>::
     estimate3DImpactParameters(const GeometryContext& gctx,
                                const Acts::MagneticFieldContext& mctx,
@@ -79,7 +79,7 @@ Acts::ImpactPointEstimator<input_track_t, propagator_t, propagator_options_t>::
   // Do the propagation to linPointPos
   auto result = m_cfg.propagator->propagate(trkParams, *planeSurface, pOptions);
   if (result.ok()) {
-    return std::move((*result).endParameters);
+    return *result->endParameters;
   } else {
     return result.error();
   }

Tests/UnitTests/Core/Propagator/ExtrapolatorTests.cpp

@@ -109,8 +109,8 @@ BOOST_DATA_TEST_CASE(
   options.maxStepSize = 10_cm;
   options.pathLimit = 25_cm;
 
-  BOOST_CHECK(epropagator.propagate(start, options).value().endParameters !=
-              nullptr);
+  BOOST_CHECK(
+      epropagator.propagate(start, options).value().endParameters.has_value());
 }
 
 // This test case checks that no segmentation fault appears
@@ -176,7 +176,7 @@ BOOST_DATA_TEST_CASE(
     const auto& cresult = epropagator.propagate(start, *csurface, optionsEmpty)
                               .value()
                               .endParameters;
-    BOOST_CHECK(cresult != nullptr);
+    BOOST_CHECK(cresult.has_value());
   }
 }
 

Tests/UnitTests/Core/Propagator/MaterialCollectionTests.cpp

@@ -172,7 +172,7 @@ void runTest(const propagator_t& prop, double pT, double phi, double theta,
     std::cout << ">>> Backward Propagation : start." << std::endl;
   }
   const auto& bwdResult =
-      prop.propagate(*fwdResult.endParameters.get(), startSurface, bwdOptions)
+      prop.propagate(*fwdResult.endParameters, startSurface, bwdOptions)
           .value();
 
   if (debugModeBwd) {
@@ -241,43 +241,42 @@ void runTest(const propagator_t& prop, double pT, double phi, double theta,
   }
 
   // move forward step by step through the surfaces
-  const BoundTrackParameters* sParameters = &start;
-  std::vector<std::unique_ptr<const BoundTrackParameters>> stepParameters;
+  BoundTrackParameters sParameters = start;
+  std::vector<BoundTrackParameters> stepParameters;
   for (auto& fwdSteps : fwdMaterial.materialInteractions) {
     if (debugModeFwdStep) {
       std::cout << ">>> Forward step : "
-                << sParameters->referenceSurface().geometryId() << " --> "
+                << sParameters.referenceSurface().geometryId() << " --> "
                 << fwdSteps.surface->geometryId() << std::endl;
     }
 
     // make a forward step
     const auto& fwdStep =
-        prop.propagate(*sParameters, (*fwdSteps.surface), fwdStepOptions)
+        prop.propagate(sParameters, (*fwdSteps.surface), fwdStepOptions)
             .value();
 
     auto& fwdStepMaterial =
         fwdStep.template get<typename MaterialInteractor::result_type>();
     fwdStepStepMaterialInX0 += fwdStepMaterial.materialInX0;
     fwdStepStepMaterialInL0 += fwdStepMaterial.materialInL0;
 
-    if (fwdStep.endParameters != nullptr) {
+    if (fwdStep.endParameters.has_value()) {
       // make sure the parameters do not run out of scope
-      stepParameters.push_back(std::make_unique<BoundTrackParameters>(
-          (*fwdStep.endParameters.get())));
-      sParameters = stepParameters.back().get();
+      stepParameters.push_back(*fwdStep.endParameters);
+      sParameters = stepParameters.back();
     }
   }
   // final destination surface
   const Surface& dSurface = fwdResult.endParameters->referenceSurface();
 
   if (debugModeFwdStep) {
     std::cout << ">>> Forward step : "
-              << sParameters->referenceSurface().geometryId() << " --> "
+              << sParameters.referenceSurface().geometryId() << " --> "
               << dSurface.geometryId() << std::endl;
   }
 
   const auto& fwdStepFinal =
-      prop.propagate(*sParameters, dSurface, fwdStepOptions).value();
+      prop.propagate(sParameters, dSurface, fwdStepOptions).value();
 
   auto& fwdStepMaterial =
       fwdStepFinal.template get<typename MaterialInteractor::result_type>();
@@ -317,41 +316,40 @@ void runTest(const propagator_t& prop, double pT, double phi, double theta,
   }
 
   // move forward step by step through the surfaces
-  sParameters = fwdResult.endParameters.get();
+  sParameters = *fwdResult.endParameters;
   for (auto& bwdSteps : bwdMaterial.materialInteractions) {
     if (debugModeBwdStep) {
       std::cout << ">>> Backward step : "
-                << sParameters->referenceSurface().geometryId() << " --> "
+                << sParameters.referenceSurface().geometryId() << " --> "
                 << bwdSteps.surface->geometryId() << std::endl;
     }
     // make a forward step
     const auto& bwdStep =
-        prop.propagate(*sParameters, (*bwdSteps.surface), bwdStepOptions)
+        prop.propagate(sParameters, (*bwdSteps.surface), bwdStepOptions)
             .value();
 
     auto& bwdStepMaterial =
         bwdStep.template get<typename MaterialInteractor::result_type>();
     bwdStepStepMaterialInX0 += bwdStepMaterial.materialInX0;
     bwdStepStepMaterialInL0 += bwdStepMaterial.materialInL0;
 
-    if (bwdStep.endParameters != nullptr) {
+    if (bwdStep.endParameters.has_value()) {
       // make sure the parameters do not run out of scope
-      stepParameters.push_back(std::make_unique<BoundTrackParameters>(
-          *(bwdStep.endParameters.get())));
-      sParameters = stepParameters.back().get();
+      stepParameters.push_back(*bwdStep.endParameters);
+      sParameters = stepParameters.back();
     }
   }
   // final destination surface
   const Surface& dbSurface = start.referenceSurface();
 
   if (debugModeBwdStep) {
     std::cout << ">>> Backward step : "
-              << sParameters->referenceSurface().geometryId() << " --> "
+              << sParameters.referenceSurface().geometryId() << " --> "
               << dSurface.geometryId() << std::endl;
   }
 
   const auto& bwdStepFinal =
-      prop.propagate(*sParameters, dbSurface, bwdStepOptions).value();
+      prop.propagate(sParameters, dbSurface, bwdStepOptions).value();
 
   auto& bwdStepMaterial =
       bwdStepFinal.template get<typename MaterialInteractor::result_type>();

Tests/UnitTests/Core/Vertexing/ImpactPointEstimatorTests.cpp

@@ -134,7 +134,7 @@ BOOST_DATA_TEST_CASE(SingleTrackDistanceParametersCompatibility3d, tracks, d0,
   // estimate parameters at the closest point in 3d
   auto res = ipEstimator.estimate3DImpactParameters(
       geoContext, magFieldContext, myTrack, refPosition, state);
-  BoundTrackParameters trackAtIP3d = **res;
+  BoundTrackParameters trackAtIP3d = *res;
   const auto& atPerigee = myTrack.parameters();
   const auto& atIp3d = trackAtIP3d.parameters();
 
@@ -190,7 +190,7 @@ BOOST_AUTO_TEST_CASE(SingleTrackDistanceParametersAthenaRegression) {
   auto res2 = ipEstimator.estimate3DImpactParameters(
       geoContext, magFieldContext, params1, vtxPos, state);
   BOOST_CHECK(res2.ok());
-  BoundTrackParameters endParams = **res2;
+  BoundTrackParameters endParams = *res2;
   Vector3 surfaceCenter = endParams.referenceSurface().center(geoContext);
 
   BOOST_CHECK_EQUAL(surfaceCenter, vtxPos);