Skip to content

Commit

Permalink
refactor!: Use BasePropagator interface in vertexing (acts-project#2886)
Browse files Browse the repository at this point in the history
This PR untemplates the ImpactParameterEstimator, HelicalTrackLinearizer
and NumericalTrackLinearizer

Part of:
- acts-project#2842 

Blocked by:
- acts-project#2881

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
paulgessinger and kodiakhq[bot] authored Feb 15, 2024
1 parent 55d49ec commit 9eac779
Show file tree
Hide file tree
Showing 31 changed files with 267 additions and 353 deletions.
8 changes: 3 additions & 5 deletions Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ namespace Acts {
/// @tparam sfinder_t Seed finder type
template <typename vfitter_t, typename sfinder_t>
class AdaptiveMultiVertexFinder {
using Propagator_t = typename vfitter_t::Propagator_t;
using Linearizer_t = typename vfitter_t::Linearizer_t;
using FitterState_t = typename vfitter_t::State;
using SeedFinderState_t = typename sfinder_t::State;
Expand All @@ -57,9 +56,8 @@ class AdaptiveMultiVertexFinder {
/// @param ipEst ImpactPointEstimator
/// @param lin Track linearizer
/// @param bIn Input magnetic field
Config(vfitter_t fitter, sfinder_t sfinder,
ImpactPointEstimator<Propagator_t> ipEst, Linearizer_t lin,
std::shared_ptr<const MagneticFieldProvider> bIn)
Config(vfitter_t fitter, sfinder_t sfinder, ImpactPointEstimator ipEst,
Linearizer_t lin, std::shared_ptr<const MagneticFieldProvider> bIn)
: vertexFitter(std::move(fitter)),
seedFinder(std::move(sfinder)),
ipEstimator(std::move(ipEst)),
Expand All @@ -73,7 +71,7 @@ class AdaptiveMultiVertexFinder {
sfinder_t seedFinder;

// ImpactPointEstimator
ImpactPointEstimator<Propagator_t> ipEstimator;
ImpactPointEstimator ipEstimator;

// Track linearizer
Linearizer_t linearizer;
Expand Down
18 changes: 6 additions & 12 deletions Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "Acts/Definitions/Algebra.hpp"
#include "Acts/EventData/TrackParameters.hpp"
#include "Acts/MagneticField/MagneticFieldProvider.hpp"
#include "Acts/Utilities/AnnealingUtility.hpp"
#include "Acts/Utilities/Logger.hpp"
#include "Acts/Utilities/Result.hpp"
Expand Down Expand Up @@ -41,30 +42,23 @@ class AdaptiveMultiVertexFitter {
"Linearizer does not fulfill linearizer concept.");

public:
using Propagator_t = typename linearizer_t::Propagator_t;
using Linearizer_t = linearizer_t;

private:
using IPEstimator = ImpactPointEstimator<Propagator_t>;

public:
/// @brief The fitter state
struct State {
State(const MagneticFieldProvider& field,
const Acts::MagneticFieldContext& magContext)
: ipState(field.makeCache(magContext)),
linearizerState(field.makeCache(magContext)) {}
fieldCache(field.makeCache(magContext)) {}
// Vertex collection to be fitted
std::vector<Vertex*> vertexCollection;

// Annealing state
AnnealingUtility::State annealingState;

// IPEstimator state
typename IPEstimator::State ipState;
ImpactPointEstimator::State ipState;

// Linearizer state
typename Linearizer_t::State linearizerState;
MagneticFieldProvider::Cache fieldCache;

// Map to store vertices information
// @TODO Does this have to be a mutable pointer?
Expand Down Expand Up @@ -115,10 +109,10 @@ class AdaptiveMultiVertexFitter {
/// @brief Config constructor
///
/// @param est ImpactPointEstimator
Config(IPEstimator est) : ipEst(std::move(est)) {}
Config(ImpactPointEstimator est) : ipEst(std::move(est)) {}

// ImpactPointEstimator
IPEstimator ipEst;
ImpactPointEstimator ipEst;

/// Annealing tool used for a thermodynamic annealing scheme for the
/// track weight factors in such a way that with high temperature values
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ Acts::AdaptiveMultiVertexFitter<linearizer_t>::setWeightsAndUpdate(
auto result = linearizer.linearizeTrack(
m_cfg.extractParameters(trk), vtxInfo.linPoint[3],
*vtxPerigeeSurface, vertexingOptions.geoContext,
vertexingOptions.magFieldContext, state.linearizerState);
vertexingOptions.magFieldContext, state.fieldCache);
if (!result.ok()) {
return result.error();
}
Expand Down
10 changes: 5 additions & 5 deletions Core/include/Acts/Vertexing/FullBilloirVertexFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#pragma once

#include "Acts/MagneticField/MagneticFieldProvider.hpp"
#include "Acts/Propagator/EigenStepper.hpp"
#include "Acts/Propagator/Propagator.hpp"
#include "Acts/Utilities/Logger.hpp"
Expand Down Expand Up @@ -51,17 +52,16 @@ class FullBilloirVertexFitter {
"Linearizer does not fulfill linearizer concept.");

public:
using Propagator_t = typename linearizer_t::Propagator_t;
using Linearizer_t = linearizer_t;

struct State {
/// @brief The state constructor
///
/// @param fieldCache The magnetic field cache
State(MagneticFieldProvider::Cache fieldCache)
: linearizerState(std::move(fieldCache)) {}
/// The linearizer state
typename Linearizer_t::State linearizerState;
State(MagneticFieldProvider::Cache _fieldCache)
: fieldCache(std::move(_fieldCache)) {}

MagneticFieldProvider::Cache fieldCache;
};

struct Config {
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Vertexing/FullBilloirVertexFitter.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ Acts::Result<Acts::Vertex> Acts::FullBilloirVertexFitter<linearizer_t>::fit(
auto result = linearizer.linearizeTrack(
trackParams, linPoint[3], *perigeeSurface,
vertexingOptions.geoContext, vertexingOptions.magFieldContext,
state.linearizerState);
state.fieldCache);
if (!result.ok()) {
return result.error();
}
Expand Down
61 changes: 18 additions & 43 deletions Core/include/Acts/Vertexing/HelicalTrackLinearizer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@
#include "Acts/MagneticField/NullBField.hpp"
#include "Acts/Propagator/EigenStepper.hpp"
#include "Acts/Propagator/Propagator.hpp"
#include "Acts/Utilities/Delegate.hpp"
#include "Acts/Utilities/Result.hpp"
#include "Acts/Vertexing/LinearizedTrack.hpp"

#include <memory>

namespace Acts {

/// @class HelicalTrackLinearizer
Expand All @@ -38,46 +41,15 @@ namespace Acts {
/// This class computes A and B using the analytic formulae of Ref. (1).
///
/// Ref. (1) - CERN-THESIS-2010-027, Giacinto Piacquadio (Freiburg U.)
///
/// @tparam propagator_t Propagator type
/// @tparam propagator_options_t Propagator options type
template <typename propagator_t,
typename propagator_options_t = PropagatorOptions<>>
class HelicalTrackLinearizer {
public:
using Propagator_t = propagator_t;

/// State struct
struct State {
/// @brief The state constructor
///
/// @param fieldCacheIn The magnetic field cache
State(MagneticFieldProvider::Cache fieldCacheIn)
: fieldCache(std::move(fieldCacheIn)) {}
/// Magnetic field cache
MagneticFieldProvider::Cache fieldCache;
};

/// @brief Configuration struct
struct Config {
/// @ Config constructor if magnetic field is present
///
/// @param bIn The magnetic field
/// @param prop The propagator
Config(std::shared_ptr<const MagneticFieldProvider> bIn,
std::shared_ptr<const Propagator_t> prop)
: bField(std::move(bIn)), propagator(std::move(prop)) {}

/// @brief Config constructor without B field -> uses NullBField
///
/// @param prop The propagator
Config(std::shared_ptr<const Propagator_t> prop)
: bField{std::make_shared<NullBField>()}, propagator(std::move(prop)) {}

// The magnetic field
std::shared_ptr<const MagneticFieldProvider> bField;
// The propagator
std::shared_ptr<const Propagator_t> propagator;
std::shared_ptr<const MagneticFieldProvider> bField =
std::make_shared<NullBField>();

std::shared_ptr<const BasePropagator> propagator;

/// Tolerance determining how close we need to get to the Perigee surface to
/// reach it during propagation
Expand All @@ -91,7 +63,11 @@ class HelicalTrackLinearizer {
HelicalTrackLinearizer(const Config& config,
std::unique_ptr<const Logger> _logger =
getDefaultLogger("HelTrkLinProp", Logging::INFO))
: m_cfg(config), m_logger{std::move(_logger)} {}
: m_cfg(config), m_logger{std::move(_logger)} {
if (!m_cfg.propagator) {
throw std::invalid_argument("HelicalTrackLinearizer: propagator is null");
}
}

/// @brief Function that linearizes BoundTrackParameters at
/// the PCA to a given Perigee surface
Expand All @@ -103,15 +79,14 @@ class HelicalTrackLinearizer {
/// @param perigeeSurface Perigee surface belonging to @p linPoint
/// @param gctx Geometry context
/// @param mctx Magnetic field context
/// @param state Linearizer state object
/// @param fieldCache Magnetic field cache
///
/// @return Linearized track
Result<LinearizedTrack> linearizeTrack(const BoundTrackParameters& params,
double linPointTime,
const Surface& perigeeSurface,
const Acts::GeometryContext& gctx,
const Acts::MagneticFieldContext& mctx,
State& state) const;
Result<LinearizedTrack> linearizeTrack(
const BoundTrackParameters& params, double linPointTime,
const Surface& perigeeSurface, const Acts::GeometryContext& gctx,
const Acts::MagneticFieldContext& mctx,
MagneticFieldProvider::Cache& fieldCache) const;

private:
/// Configuration object
Expand Down
26 changes: 13 additions & 13 deletions Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
#include "Acts/Surfaces/PerigeeSurface.hpp"
#include "Acts/Vertexing/LinearizerTrackParameters.hpp"

template <typename propagator_t, typename propagator_options_t>
Acts::Result<Acts::LinearizedTrack> Acts::
HelicalTrackLinearizer<propagator_t, propagator_options_t>::linearizeTrack(
const BoundTrackParameters& params, double linPointTime,
const Surface& perigeeSurface, const Acts::GeometryContext& gctx,
const Acts::MagneticFieldContext& mctx, State& state) const {
inline Acts::Result<Acts::LinearizedTrack>
Acts::HelicalTrackLinearizer::linearizeTrack(
const BoundTrackParameters& params, double linPointTime,
const Surface& perigeeSurface, const Acts::GeometryContext& gctx,
const Acts::MagneticFieldContext& mctx,
MagneticFieldProvider::Cache& fieldCache) const {
// Create propagator options
propagator_options_t pOptions(gctx, mctx);
PropagatorOptions<> pOptions(gctx, mctx);

// Length scale at which we consider to be sufficiently close to the Perigee
// surface to skip the propagation.
Expand All @@ -39,14 +39,15 @@ Acts::Result<Acts::LinearizedTrack> Acts::
Direction::fromScalarZeroAsPositive(intersection.pathLength());

// Propagate to the PCA of the reference point
auto result = m_cfg.propagator->propagate(params, perigeeSurface, pOptions);
if (!result.ok()) {
return result.error();
const auto res =
m_cfg.propagator->propagateToSurface(params, perigeeSurface, pOptions);
if (!res.ok()) {
return res.error();
}
const auto& endParams = *res;

// Extracting the track parameters at said PCA - this corresponds to the
// Perigee representation of the track wrt the reference point
const auto& endParams = *result->endParameters;
BoundVector paramsAtPCA = endParams.parameters();

// Extracting the 4D position of the PCA in global coordinates
Expand Down Expand Up @@ -91,8 +92,7 @@ Acts::Result<Acts::LinearizedTrack> Acts::
ActsScalar absoluteCharge = params.particleHypothesis().absoluteCharge();

// get the z-component of the B-field at the PCA
auto field =
m_cfg.bField->getField(VectorHelpers::position(pca), state.fieldCache);
auto field = m_cfg.bField->getField(VectorHelpers::position(pca), fieldCache);
if (!field.ok()) {
return field.error();
}
Expand Down
11 changes: 6 additions & 5 deletions Core/include/Acts/Vertexing/ImpactPointEstimator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ struct ImpactParametersAndSigma {
/// A description of the underlying mathematics can be found here:
/// https://github.com/acts-project/acts/pull/2506
/// TODO: Upload reference at a better place
template <typename propagator_t,
typename propagator_options_t = PropagatorOptions<>>
class ImpactPointEstimator {
public:
/// State struct
Expand All @@ -64,20 +62,20 @@ class ImpactPointEstimator {
/// @param bIn The magnetic field
/// @param prop The propagator
Config(std::shared_ptr<const MagneticFieldProvider> bIn,
std::shared_ptr<const propagator_t> prop)
std::shared_ptr<const BasePropagator> prop)
: bField(std::move(bIn)), propagator(std::move(prop)) {}

/// @brief Config constructor without B field -> uses NullBField
/// provided)
///
/// @param prop The propagator
Config(std::shared_ptr<propagator_t> prop)
Config(std::shared_ptr<const BasePropagator> prop)
: bField{std::make_shared<NullBField>()}, propagator(std::move(prop)) {}

/// Magnetic field
std::shared_ptr<const MagneticFieldProvider> bField;
/// Propagator
std::shared_ptr<const propagator_t> propagator;
std::shared_ptr<const BasePropagator> propagator;
/// Max. number of iterations in Newton method
int maxIterations = 20;
/// Desired precision of deltaPhi in Newton method
Expand All @@ -100,6 +98,9 @@ class ImpactPointEstimator {
ImpactPointEstimator(const ImpactPointEstimator& other)
: m_cfg(other.m_cfg), m_logger(other.logger().clone()) {}

/// Move constructor for impact point estimator
ImpactPointEstimator(ImpactPointEstimator&&) = default;

/// @brief Calculates 3D distance between a track and a vertex
///
/// @param gctx The geometry context
Expand Down
Loading

0 comments on commit 9eac779

Please sign in to comment.