From 25969edbddbca80e024f3ffd1ce7418e6767d761 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Tue, 11 Feb 2025 15:54:44 +0100 Subject: [PATCH 1/9] chore: pre-commit check for type_t uses `--fix` (#4082) --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 87251106806..027feb11284 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -70,7 +70,7 @@ repos: - id: type_t name: type_t language: system - entry: CI/check_type_t.py + entry: CI/check_type_t.py --fix files: \.(cpp|hpp|ipp|cu|cuh)$ - repo: local From d781484df9d8e8c4762956571d680b383bbc07c7 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 11 Feb 2025 18:09:26 +0100 Subject: [PATCH 2/9] refactor: Decouple stepping from navigation (#4039) Very similar to https://github.com/acts-project/acts/pull/3449 this decouples the stepping from the navigation. Instead of passing the whole propagation state into stepping function we can just pass in the stepping state. This cuts all dependencies from the navigation and allows to use and test steppers independent from a propagation state and navigation. ## Summary by CodeRabbit ## Release Notes - **New Features** - Introduced a new `IVolumeMaterial` interface across multiple propagation components. - Enhanced stepper interfaces with more explicit state and direction management. - **Improvements** - Simplified method signatures in propagator components. - Removed unnecessary template parameters in stepper implementations. - Streamlined state management across propagation classes. - Improved type safety and code clarity in propagation logic. - **Breaking Changes** - Modified method signatures in steppers, potentially requiring updates to existing code. - Removed mock state structures in test files. - Changed how state and navigation are handled in propagation components. - **Technical Refinements** - Updated type aliases and inheritance structures. - Removed redundant compatibility checks. - Enhanced error handling in propagation methods. --- Core/include/Acts/Propagator/AtlasStepper.hpp | 89 +++++----- Core/include/Acts/Propagator/EigenStepper.hpp | 35 ++-- Core/include/Acts/Propagator/EigenStepper.ipp | 157 +++++++++--------- .../EigenStepperDefaultExtension.hpp | 92 +++++----- .../Propagator/EigenStepperDenseExtension.hpp | 132 +++++++-------- .../Acts/Propagator/MultiEigenStepperLoop.hpp | 21 +-- .../Acts/Propagator/MultiEigenStepperLoop.ipp | 41 ++--- Core/include/Acts/Propagator/Propagator.hpp | 2 +- Core/include/Acts/Propagator/Propagator.ipp | 26 +-- .../Acts/Propagator/PropagatorState.hpp | 2 + .../Acts/Propagator/PropagatorTraits.hpp | 9 +- .../Acts/Propagator/StraightLineStepper.hpp | 106 ++++++------ Core/include/Acts/Propagator/SympyStepper.hpp | 38 ++--- Core/include/Acts/Propagator/SympyStepper.ipp | 22 --- .../include/Acts/Propagator/VoidNavigator.hpp | 10 ++ .../Propagator/detail/LoopStepperUtils.hpp | 8 + .../Acts/TrackFitting/KalmanFitter.hpp | 2 +- .../Acts/TrackFitting/detail/GsfActor.hpp | 10 +- .../detail/KalmanUpdateHelpers.hpp | 15 +- Core/src/Propagator/SympyStepper.cpp | 22 ++- .../Core/Propagator/AtlasStepperTests.cpp | 97 +++++------ .../Core/Propagator/EigenStepperTests.cpp | 91 ++++------ .../Core/Propagator/MultiStepperTests.cpp | 58 +------ .../Propagator/StraightLineStepperTests.cpp | 77 ++++----- .../Core/Propagator/SympyStepperTests.cpp | 77 +++------ 25 files changed, 531 insertions(+), 708 deletions(-) delete mode 100644 Core/include/Acts/Propagator/SympyStepper.ipp diff --git a/Core/include/Acts/Propagator/AtlasStepper.hpp b/Core/include/Acts/Propagator/AtlasStepper.hpp index 6a3f9a52a8e..16cafc17639 100644 --- a/Core/include/Acts/Propagator/AtlasStepper.hpp +++ b/Core/include/Acts/Propagator/AtlasStepper.hpp @@ -32,6 +32,8 @@ namespace Acts { +class IVolumeMaterial; + /// @brief the AtlasStepper implementation for the /// /// This is based original stepper code from the ATLAS RungeKuttaPropagator @@ -523,13 +525,10 @@ class AtlasStepper { /// Compute path length derivatives in case they have not been computed /// yet, which is the case if no step has been executed yet. /// - /// @param [in, out] prop_state State that will be presented as @c BoundState - /// @param [in] navigator the navigator of the propagation + /// @param [in, out] state The stepping state (thread-local cache) /// @return true if nothing is missing after this call, false otherwise. - template - bool prepareCurvilinearState( - [[maybe_unused]] propagator_state_t& prop_state, - [[maybe_unused]] const navigator_t& navigator) const { + bool prepareCurvilinearState(State& state) const { + (void)state; return true; } @@ -1148,33 +1147,43 @@ class AtlasStepper { /// Perform the actual step on the state /// - /// @param state is the provided stepper state (caller keeps thread locality) - template - Result step(propagator_state_t& state, - const navigator_t& /*navigator*/) const { + /// @param [in,out] state State of the stepper + /// @param propDir is the direction of propagation + /// @param material is the optional volume material we are stepping through. + // This is simply ignored if `nullptr`. + /// @return the result of the step + /// + /// @note The state contains the desired step size. It can be negative during + /// backwards track propagation, and since we're using an adaptive + /// algorithm, it can be modified by the stepper class during + /// propagation. + Result step(State& state, Direction propDir, + const IVolumeMaterial* material) const { + (void)material; + // we use h for keeping the nominclature with the original atlas code - auto h = state.stepping.stepSize.value() * state.options.direction; - bool Jac = state.stepping.useJacobian; + auto h = state.stepSize.value() * propDir; + bool Jac = state.useJacobian; - double* R = &(state.stepping.pVector[0]); // Coordinates - double* A = &(state.stepping.pVector[4]); // Directions - double* sA = &(state.stepping.pVector[56]); + double* R = &(state.pVector[0]); // Coordinates + double* A = &(state.pVector[4]); // Directions + double* sA = &(state.pVector[56]); // Invert mometum/2. - double Pi = 0.5 * state.stepping.pVector[7]; + double Pi = 0.5 * state.pVector[7]; // double dltm = 0.0002 * .03; Vector3 f0, f; // if new field is required get it - if (state.stepping.newfield) { + if (state.newfield) { const Vector3 pos(R[0], R[1], R[2]); // This is sd.B_first in EigenStepper - auto fRes = getField(state.stepping, pos); + auto fRes = getField(state, pos); if (!fRes.ok()) { return fRes.error(); } f0 = *fRes; } else { - f0 = state.stepping.field; + f0 = state.field; } bool Helix = false; @@ -1210,7 +1219,7 @@ class AtlasStepper { // This is pos1 in EigenStepper const Vector3 pos(R[0] + A1 * S4, R[1] + B1 * S4, R[2] + C1 * S4); // This is sd.B_middle in EigenStepper - auto fRes = getField(state.stepping, pos); + auto fRes = getField(state, pos); if (!fRes.ok()) { return fRes.error(); } @@ -1240,7 +1249,7 @@ class AtlasStepper { // This is pos2 in EigenStepper const Vector3 pos(R[0] + h * A4, R[1] + h * B4, R[2] + h * C4); // This is sd.B_last in Eigen stepper - auto fRes = getField(state.stepping, pos); + auto fRes = getField(state, pos); if (!fRes.ok()) { return fRes.error(); } @@ -1264,10 +1273,10 @@ class AtlasStepper { 2. * h * (std::abs((A1 + A6) - (A3 + A4)) + std::abs((B1 + B6) - (B3 + B4)) + std::abs((C1 + C6) - (C3 + C4))); - if (std::abs(EST) > std::abs(state.options.stepping.stepTolerance)) { + if (std::abs(EST) > std::abs(state.options.stepTolerance)) { h = h * .5; // neutralize the sign of h again - state.stepping.stepSize.setAccuracy(h * state.options.direction); + state.stepSize.setAccuracy(h * propDir); // dltm = 0.; continue; } @@ -1296,25 +1305,25 @@ class AtlasStepper { sA[1] = B6 * Sl; sA[2] = C6 * Sl; - double mass = particleHypothesis(state.stepping).mass(); - double momentum = absoluteMomentum(state.stepping); + double mass = particleHypothesis(state).mass(); + double momentum = absoluteMomentum(state); // Evaluate the time propagation double dtds = std::sqrt(1 + mass * mass / (momentum * momentum)); - state.stepping.pVector[3] += h * dtds; - state.stepping.pVector[59] = dtds; - state.stepping.field = f; - state.stepping.newfield = false; + state.pVector[3] += h * dtds; + state.pVector[59] = dtds; + state.field = f; + state.newfield = false; if (Jac) { - double dtdl = h * mass * mass * qOverP(state.stepping) / dtds; - state.stepping.pVector[43] += dtdl; + double dtdl = h * mass * mass * qOverP(state) / dtds; + state.pVector[43] += dtdl; // Jacobian calculation // - double* d2A = &state.stepping.pVector[28]; - double* d3A = &state.stepping.pVector[36]; - double* d4A = &state.stepping.pVector[44]; + double* d2A = &state.pVector[28]; + double* d3A = &state.pVector[36]; + double* d4A = &state.pVector[44]; double d2A0 = H0[2] * d2A[1] - H0[1] * d2A[2]; double d2B0 = H0[0] * d2A[2] - H0[2] * d2A[0]; double d2C0 = H0[1] * d2A[0] - H0[0] * d2A[1]; @@ -1373,7 +1382,7 @@ class AtlasStepper { double d4B6 = d4C5 * H2[0] - d4A5 * H2[2]; double d4C6 = d4A5 * H2[1] - d4B5 * H2[0]; - double* dR = &state.stepping.pVector[24]; + double* dR = &state.pVector[24]; dR[0] += (d2A2 + d2A3 + d2A4) * S3; dR[1] += (d2B2 + d2B3 + d2B4) * S3; dR[2] += (d2C2 + d2C3 + d2C4) * S3; @@ -1381,7 +1390,7 @@ class AtlasStepper { d2A[1] = ((d2B0 + 2. * d2B3) + (d2B5 + d2B6)) * (1. / 3.); d2A[2] = ((d2C0 + 2. * d2C3) + (d2C5 + d2C6)) * (1. / 3.); - dR = &state.stepping.pVector[32]; + dR = &state.pVector[32]; dR[0] += (d3A2 + d3A3 + d3A4) * S3; dR[1] += (d3B2 + d3B3 + d3B4) * S3; dR[2] += (d3C2 + d3C3 + d3C4) * S3; @@ -1389,7 +1398,7 @@ class AtlasStepper { d3A[1] = ((d3B0 + 2. * d3B3) + (d3B5 + d3B6)) * (1. / 3.); d3A[2] = ((d3C0 + 2. * d3C3) + (d3C5 + d3C6)) * (1. / 3.); - dR = &state.stepping.pVector[40]; + dR = &state.pVector[40]; dR[0] += (d4A2 + d4A3 + d4A4) * S3; dR[1] += (d4B2 + d4B3 + d4B4) * S3; dR[2] += (d4C2 + d4C3 + d4C4) * S3; @@ -1401,9 +1410,9 @@ class AtlasStepper { break; } - state.stepping.pathAccumulated += h; - ++state.stepping.nSteps; - state.stepping.nStepTrials += nStepTrials; + state.pathAccumulated += h; + ++state.nSteps; + state.nStepTrials += nStepTrials; return h; } diff --git a/Core/include/Acts/Propagator/EigenStepper.hpp b/Core/include/Acts/Propagator/EigenStepper.hpp index 6e3339098da..b17b2160a19 100644 --- a/Core/include/Acts/Propagator/EigenStepper.hpp +++ b/Core/include/Acts/Propagator/EigenStepper.hpp @@ -29,6 +29,8 @@ namespace Acts { +class IVolumeMaterial; + /// @brief Runge-Kutta-Nystroem stepper based on Eigen implementation /// for the following ODE: /// @@ -229,18 +231,18 @@ class EigenStepper { /// @param [in,out] state The stepping state (thread-local cache) /// @param [in] surface The surface provided /// @param [in] index The surface intersection index - /// @param [in] navDir The navigation direction + /// @param [in] propDir The propagation direction /// @param [in] boundaryTolerance The boundary check for this status update /// @param [in] surfaceTolerance Surface tolerance used for intersection /// @param [in] stype The step size type to be set /// @param [in] logger A @c Logger instance IntersectionStatus updateSurfaceStatus( State& state, const Surface& surface, std::uint8_t index, - Direction navDir, const BoundaryTolerance& boundaryTolerance, + Direction propDir, const BoundaryTolerance& boundaryTolerance, double surfaceTolerance, ConstrainedStep::Type stype, const Logger& logger = getDummyLogger()) const { return detail::updateSingleSurfaceStatus( - *this, state, surface, index, navDir, boundaryTolerance, + *this, state, surface, index, propDir, boundaryTolerance, surfaceTolerance, stype, logger); } @@ -323,12 +325,9 @@ class EigenStepper { /// Compute path length derivatives in case they have not been computed /// yet, which is the case if no step has been executed yet. /// - /// @param [in, out] prop_state State that will be presented as @c BoundState - /// @param [in] navigator the navigator of the propagation + /// @param [in, out] state The state of the stepper /// @return true if nothing is missing after this call, false otherwise. - template - bool prepareCurvilinearState(propagator_state_t& prop_state, - const navigator_t& navigator) const; + bool prepareCurvilinearState(State& state) const; /// Create and return a curvilinear state at the current position /// @@ -390,15 +389,18 @@ class EigenStepper { /// Perform a Runge-Kutta track parameter propagation step /// - /// @param [in,out] state the propagation state - /// @param [in] navigator the navigator of the propagation - /// @note The state contains the desired step size. It can be negative during + /// @param [in,out] state State of the stepper + /// @param propDir is the direction of propagation + /// @param material is the optional volume material we are stepping through. + // This is simply ignored if `nullptr`. + /// @return the result of the step + /// + /// @note The state contains the desired step size. It can be negative during /// backwards track propagation, and since we're using an adaptive /// algorithm, it can be modified by the stepper class during /// propagation. - template - Result step(propagator_state_t& state, - const navigator_t& navigator) const; + Result step(State& state, Direction propDir, + const IVolumeMaterial* material) const; /// Method that reset the Jacobian to the Identity for when no bound state are /// available @@ -411,9 +413,8 @@ class EigenStepper { std::shared_ptr m_bField; }; -template -struct SupportsBoundParameters, navigator_t> - : public std::true_type {}; +template <> +struct SupportsBoundParameters> : public std::true_type {}; } // namespace Acts diff --git a/Core/include/Acts/Propagator/EigenStepper.ipp b/Core/include/Acts/Propagator/EigenStepper.ipp index c5ba0ddd9eb..b1e4724013b 100644 --- a/Core/include/Acts/Propagator/EigenStepper.ipp +++ b/Core/include/Acts/Propagator/EigenStepper.ipp @@ -78,35 +78,36 @@ auto Acts::EigenStepper::boundState( } template -template -bool Acts::EigenStepper::prepareCurvilinearState( - propagator_state_t& prop_state, const navigator_t& navigator) const { +bool Acts::EigenStepper::prepareCurvilinearState(State& state) const { // test whether the accumulated path has still its initial value. - if (prop_state.stepping.pathAccumulated == 0.) { - // if no step was executed the path length derivates have not been - // computed but are needed to compute the curvilinear covariance. The - // derivates are given by k1 for a zero step width. - // First Runge-Kutta point (at current position) - auto& sd = prop_state.stepping.stepData; - auto pos = position(prop_state.stepping); - auto fieldRes = getField(prop_state.stepping, pos); - if (fieldRes.ok()) { - sd.B_first = *fieldRes; - if (prop_state.stepping.extension.template k<0>( - prop_state, *this, navigator, sd.k1, sd.B_first, sd.kQoP)) { - // dr/ds : - prop_state.stepping.derivative.template head<3>() = - prop_state.stepping.pars.template segment<3>(eFreeDir0); - // d (dr/ds) / ds : - prop_state.stepping.derivative.template segment<3>(4) = sd.k1; - // to set dt/ds : - prop_state.stepping.extension.finalize( - prop_state, *this, navigator, prop_state.stepping.pathAccumulated); - return true; - } - } + if (state.pathAccumulated != 0) { + return true; + } + + // if no step was executed the path length derivates have not been + // computed but are needed to compute the curvilinear covariance. The + // derivates are given by k1 for a zero step width. + // First Runge-Kutta point (at current position) + auto& sd = state.stepData; + auto pos = position(state); + auto fieldRes = getField(state, pos); + if (!fieldRes.ok()) { + return false; + } + + sd.B_first = *fieldRes; + if (!state.extension.template k<0>(state, *this, nullptr, sd.k1, sd.B_first, + sd.kQoP)) { return false; } + + // dr/ds : + state.derivative.template head<3>() = + state.pars.template segment<3>(eFreeDir0); + // d (dr/ds) / ds : + state.derivative.template segment<3>(4) = sd.k1; + // to set dt/ds : + state.extension.finalize(state, *this, nullptr, state.pathAccumulated); return true; } @@ -160,27 +161,26 @@ void Acts::EigenStepper::transportCovarianceToBound( } template -template Acts::Result Acts::EigenStepper::step( - propagator_state_t& state, const navigator_t& navigator) const { + State& state, Direction propDir, const IVolumeMaterial* material) const { // Runge-Kutta integrator state - auto& sd = state.stepping.stepData; + auto& sd = state.stepData; double errorEstimate = 0; double h2 = 0; double half_h = 0; - auto pos = position(state.stepping); - auto dir = direction(state.stepping); + auto pos = position(state); + auto dir = direction(state); // First Runge-Kutta point (at current position) - auto fieldRes = getField(state.stepping, pos); + auto fieldRes = getField(state, pos); if (!fieldRes.ok()) { return fieldRes.error(); } sd.B_first = *fieldRes; - if (!state.stepping.extension.template k<0>(state, *this, navigator, sd.k1, - sd.B_first, sd.kQoP)) { + if (!state.extension.template k<0>(state, *this, material, sd.k1, sd.B_first, + sd.kQoP)) { return 0.; } @@ -191,7 +191,7 @@ Acts::Result Acts::EigenStepper::step( // This is given by the order of the Runge-Kutta method constexpr double exponent = 0.25; - double x = state.options.stepping.stepTolerance / errorEstimate_; + double x = state.options.stepTolerance / errorEstimate_; if constexpr (exponent == 0.25) { // This is 3x faster than std::pow @@ -207,8 +207,7 @@ Acts::Result Acts::EigenStepper::step( // For details about these values see ATL-SOFT-PUB-2009-001 constexpr double marginFactor = 4.0; - return errorEstimate_ <= - marginFactor * state.options.stepping.stepTolerance; + return errorEstimate_ <= marginFactor * state.options.stepTolerance; }; // The following functor starts to perform a Runge-Kutta step of a certain @@ -226,34 +225,32 @@ Acts::Result Acts::EigenStepper::step( // Second Runge-Kutta point const Vector3 pos1 = pos + half_h * dir + h2 * 0.125 * sd.k1; - auto field = getField(state.stepping, pos1); + auto field = getField(state, pos1); if (!field.ok()) { return failure(field.error()); } sd.B_middle = *field; - if (!state.stepping.extension.template k<1>(state, *this, navigator, sd.k2, - sd.B_middle, sd.kQoP, half_h, - sd.k1)) { + if (!state.extension.template k<1>(state, *this, material, sd.k2, + sd.B_middle, sd.kQoP, half_h, sd.k1)) { return success(false); } // Third Runge-Kutta point - if (!state.stepping.extension.template k<2>(state, *this, navigator, sd.k3, - sd.B_middle, sd.kQoP, half_h, - sd.k2)) { + if (!state.extension.template k<2>(state, *this, material, sd.k3, + sd.B_middle, sd.kQoP, half_h, sd.k2)) { return success(false); } // Last Runge-Kutta point const Vector3 pos2 = pos + h * dir + h2 * 0.5 * sd.k3; - field = getField(state.stepping, pos2); + field = getField(state, pos2); if (!field.ok()) { return failure(field.error()); } sd.B_last = *field; - if (!state.stepping.extension.template k<3>(state, *this, navigator, sd.k4, - sd.B_last, sd.kQoP, h, sd.k3)) { + if (!state.extension.template k<3>(state, *this, material, sd.k4, sd.B_last, + sd.kQoP, h, sd.k3)) { return success(false); } @@ -267,15 +264,14 @@ Acts::Result Acts::EigenStepper::step( return success(isErrorTolerable(errorEstimate)); }; - const double initialH = - state.stepping.stepSize.value() * state.options.direction; + const double initialH = state.stepSize.value() * propDir; double h = initialH; std::size_t nStepTrials = 0; // Select and adjust the appropriate Runge-Kutta step size as given // ATL-SOFT-PUB-2009-001 while (true) { ++nStepTrials; - ++state.stepping.statistics.nAttemptedSteps; + ++state.statistics.nAttemptedSteps; auto res = tryRungeKuttaStep(h); if (!res.ok()) { @@ -285,33 +281,33 @@ Acts::Result Acts::EigenStepper::step( break; } - ++state.stepping.statistics.nRejectedSteps; + ++state.statistics.nRejectedSteps; const double stepSizeScaling = calcStepSizeScaling(errorEstimate); h *= stepSizeScaling; // If step size becomes too small the particle remains at the initial // place - if (std::abs(h) < std::abs(state.options.stepping.stepSizeCutOff)) { + if (std::abs(h) < std::abs(state.options.stepSizeCutOff)) { // Not moving due to too low momentum needs an aborter return EigenStepperError::StepSizeStalled; } // If the parameter is off track too much or given stepSize is not // appropriate - if (nStepTrials > state.options.stepping.maxRungeKuttaStepTrials) { + if (nStepTrials > state.options.maxRungeKuttaStepTrials) { // Too many trials, have to abort return EigenStepperError::StepSizeAdjustmentFailed; } } // When doing error propagation, update the associated Jacobian matrix - if (state.stepping.covTransport) { + if (state.covTransport) { // using the direction before updated below // The step transport matrix in global coordinates FreeMatrix D; - if (!state.stepping.extension.finalize(state, *this, navigator, h, D)) { + if (!state.extension.finalize(state, *this, material, h, D)) { return EigenStepperError::StepInvalid; } @@ -334,56 +330,53 @@ Acts::Result Acts::EigenStepper::step( // sub-matrices at all! assert((D.topLeftCorner<4, 4>().isIdentity())); assert((D.bottomLeftCorner<4, 4>().isZero())); - assert((state.stepping.jacTransport.template topLeftCorner<4, 4>() - .isIdentity())); - assert((state.stepping.jacTransport.template bottomLeftCorner<4, 4>() - .isZero())); + assert((state.jacTransport.template topLeftCorner<4, 4>().isIdentity())); + assert((state.jacTransport.template bottomLeftCorner<4, 4>().isZero())); - state.stepping.jacTransport.template topRightCorner<4, 4>() += + state.jacTransport.template topRightCorner<4, 4>() += D.topRightCorner<4, 4>() * - state.stepping.jacTransport.template bottomRightCorner<4, 4>(); - state.stepping.jacTransport.template bottomRightCorner<4, 4>() = + state.jacTransport.template bottomRightCorner<4, 4>(); + state.jacTransport.template bottomRightCorner<4, 4>() = (D.bottomRightCorner<4, 4>() * - state.stepping.jacTransport.template bottomRightCorner<4, 4>()) + state.jacTransport.template bottomRightCorner<4, 4>()) .eval(); } else { - if (!state.stepping.extension.finalize(state, *this, navigator, h)) { + if (!state.extension.finalize(state, *this, material, h)) { return EigenStepperError::StepInvalid; } } // Update the track parameters according to the equations of motion - state.stepping.pars.template segment<3>(eFreePos0) += + state.pars.template segment<3>(eFreePos0) += h * dir + h2 / 6. * (sd.k1 + sd.k2 + sd.k3); - state.stepping.pars.template segment<3>(eFreeDir0) += + state.pars.template segment<3>(eFreeDir0) += h / 6. * (sd.k1 + 2. * (sd.k2 + sd.k3) + sd.k4); - (state.stepping.pars.template segment<3>(eFreeDir0)).normalize(); + (state.pars.template segment<3>(eFreeDir0)).normalize(); - if (state.stepping.covTransport) { + if (state.covTransport) { // using the updated direction - state.stepping.derivative.template head<3>() = - state.stepping.pars.template segment<3>(eFreeDir0); - state.stepping.derivative.template segment<3>(4) = sd.k4; + state.derivative.template head<3>() = + state.pars.template segment<3>(eFreeDir0); + state.derivative.template segment<3>(4) = sd.k4; } - state.stepping.pathAccumulated += h; - ++state.stepping.nSteps; - state.stepping.nStepTrials += nStepTrials; + state.pathAccumulated += h; + ++state.nSteps; + state.nStepTrials += nStepTrials; - ++state.stepping.statistics.nSuccessfulSteps; - if (state.options.direction != - Direction::fromScalarZeroAsPositive(initialH)) { - ++state.stepping.statistics.nReverseSteps; + ++state.statistics.nSuccessfulSteps; + if (propDir != Direction::fromScalarZeroAsPositive(initialH)) { + ++state.statistics.nReverseSteps; } - state.stepping.statistics.pathLength += h; - state.stepping.statistics.absolutePathLength += std::abs(h); + state.statistics.pathLength += h; + state.statistics.absolutePathLength += std::abs(h); const double stepSizeScaling = calcStepSizeScaling(errorEstimate); const double nextAccuracy = std::abs(h * stepSizeScaling); - const double previousAccuracy = std::abs(state.stepping.stepSize.accuracy()); + const double previousAccuracy = std::abs(state.stepSize.accuracy()); const double initialStepLength = std::abs(initialH); if (nextAccuracy < initialStepLength || nextAccuracy > previousAccuracy) { - state.stepping.stepSize.setAccuracy(nextAccuracy); + state.stepSize.setAccuracy(nextAccuracy); } return h; diff --git a/Core/include/Acts/Propagator/EigenStepperDefaultExtension.hpp b/Core/include/Acts/Propagator/EigenStepperDefaultExtension.hpp index c48bd9bbcc2..8d14627f950 100644 --- a/Core/include/Acts/Propagator/EigenStepperDefaultExtension.hpp +++ b/Core/include/Acts/Propagator/EigenStepperDefaultExtension.hpp @@ -16,6 +16,8 @@ namespace Acts { +class IVolumeMaterial; + /// @brief Default evaluator of the k_i's and elements of the transport matrix /// D of the RKN4 stepping. This is a pure implementation by textbook. struct EigenStepperDefaultExtension { @@ -23,12 +25,11 @@ struct EigenStepperDefaultExtension { /// step sets up qop, too. /// /// @tparam i Index of the k_i, i = [0, 3] - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper - /// @tparam navigator_t Type of the navigator /// - /// @param [in] state State of the propagator + /// @param [in] state State of the stepper /// @param [in] stepper Stepper of the propagation + /// @param [in] volumeMaterial Material of the volume /// @param [out] knew Next k_i that is evaluated /// @param [in] bField B-Field at the evaluation position /// @param [out] kQoP k_i elements of the momenta @@ -36,22 +37,22 @@ struct EigenStepperDefaultExtension { /// @param [in] kprev Evaluated k_{i - 1} /// /// @return Boolean flag if the calculation is valid - template - bool k(const propagator_state_t& state, const stepper_t& stepper, - const navigator_t& /*navigator*/, Vector3& knew, const Vector3& bField, - std::array& kQoP, const double h = 0., - const Vector3& kprev = Vector3::Zero()) + template + bool k(const typename stepper_t::State& state, const stepper_t& stepper, + const IVolumeMaterial* volumeMaterial, Vector3& knew, + const Vector3& bField, std::array& kQoP, + const double h = 0., const Vector3& kprev = Vector3::Zero()) requires(i >= 0 && i <= 3) { - auto qop = stepper.qOverP(state.stepping); + (void)volumeMaterial; + + auto qop = stepper.qOverP(state); // First step does not rely on previous data if constexpr (i == 0) { - knew = qop * stepper.direction(state.stepping).cross(bField); + knew = qop * stepper.direction(state).cross(bField); kQoP = {0., 0., 0., 0.}; } else { - knew = - qop * (stepper.direction(state.stepping) + h * kprev).cross(bField); + knew = qop * (stepper.direction(state) + h * kprev).cross(bField); } return true; } @@ -60,19 +61,19 @@ struct EigenStepperDefaultExtension { /// error of the step. Since the textbook does not deliver further vetos, /// this is a dummy function. /// - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper - /// @tparam navigator_t Type of the navigator /// - /// @param [in] state State of the propagator + /// @param [in] state State of the stepper /// @param [in] stepper Stepper of the propagation + /// @param [in] volumeMaterial Material of the volume /// @param [in] h Step size /// /// @return Boolean flag if the calculation is valid - template - bool finalize(propagator_state_t& state, const stepper_t& stepper, - const navigator_t& /*navigator*/, const double h) const { + template + bool finalize(typename stepper_t::State& state, const stepper_t& stepper, + const IVolumeMaterial* volumeMaterial, const double h) const { + (void)volumeMaterial; + propagateTime(state, stepper, h); return true; } @@ -81,21 +82,21 @@ struct EigenStepperDefaultExtension { /// error of the step. Since the textbook does not deliver further vetos, /// this is just for the evaluation of the transport matrix. /// - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper - /// @tparam navigator_t Type of the navigator /// - /// @param [in] state State of the propagator + /// @param [in] state State of the stepper /// @param [in] stepper Stepper of the propagation + /// @param [in] volumeMaterial Material of the volume /// @param [in] h Step size /// @param [out] D Transport matrix /// /// @return Boolean flag if the calculation is valid - template - bool finalize(propagator_state_t& state, const stepper_t& stepper, - const navigator_t& /*navigator*/, const double h, + template + bool finalize(typename stepper_t::State& state, const stepper_t& stepper, + const IVolumeMaterial* volumeMaterial, const double h, FreeMatrix& D) const { + (void)volumeMaterial; + propagateTime(state, stepper, h); return transportMatrix(state, stepper, h, D); } @@ -103,41 +104,40 @@ struct EigenStepperDefaultExtension { private: /// @brief Propagation function for the time coordinate /// - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper /// - /// @param [in, out] state State of the propagator + /// @param [in, out] state State of the stepper /// @param [in] stepper Stepper of the propagation /// @param [in] h Step size - template - void propagateTime(propagator_state_t& state, const stepper_t& stepper, + template + void propagateTime(typename stepper_t::State& state, const stepper_t& stepper, const double h) const { /// This evaluation is based on dt/ds = 1/v = 1/(beta * c) with the velocity /// v, the speed of light c and beta = v/c. This can be re-written as dt/ds /// = sqrt(m^2/p^2 + c^{-2}) with the mass m and the momentum p. - auto m = stepper.particleHypothesis(state.stepping).mass(); - auto p = stepper.absoluteMomentum(state.stepping); + auto m = stepper.particleHypothesis(state).mass(); + auto p = stepper.absoluteMomentum(state); auto dtds = std::sqrt(1 + m * m / (p * p)); - state.stepping.pars[eFreeTime] += h * dtds; - if (state.stepping.covTransport) { - state.stepping.derivative(3) = dtds; + state.pars[eFreeTime] += h * dtds; + if (state.covTransport) { + state.derivative(3) = dtds; } } /// @brief Calculates the transport matrix D for the jacobian /// - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper /// - /// @param [in] state State of the propagator + /// @param [in] state State of the stepper /// @param [in] stepper Stepper of the propagation /// @param [in] h Step size /// @param [out] D Transport matrix /// /// @return Boolean flag if evaluation is valid - template - bool transportMatrix(propagator_state_t& state, const stepper_t& stepper, - const double h, FreeMatrix& D) const { + template + bool transportMatrix(typename stepper_t::State& state, + const stepper_t& stepper, const double h, + FreeMatrix& D) const { /// The calculations are based on ATL-SOFT-PUB-2009-002. The update of the /// Jacobian matrix is requires only the calculation of eq. 17 and 18. /// Since the terms of eq. 18 are currently 0, this matrix is not needed @@ -157,11 +157,11 @@ struct EigenStepperDefaultExtension { /// constant offset does not exist for rectangular matrix dGdu' (due to the /// missing Lambda part) and only exists for dFdu' in dlambda/dlambda. - auto m = state.stepping.particleHypothesis.mass(); - auto& sd = state.stepping.stepData; - auto dir = stepper.direction(state.stepping); - auto qop = stepper.qOverP(state.stepping); - auto p = stepper.absoluteMomentum(state.stepping); + auto m = state.particleHypothesis.mass(); + auto& sd = state.stepData; + auto dir = stepper.direction(state); + auto qop = stepper.qOverP(state); + auto p = stepper.absoluteMomentum(state); auto dtds = std::sqrt(1 + m * m / (p * p)); D = FreeMatrix::Identity(); diff --git a/Core/include/Acts/Propagator/EigenStepperDenseExtension.hpp b/Core/include/Acts/Propagator/EigenStepperDenseExtension.hpp index 935e747f3aa..6ecae8856b2 100644 --- a/Core/include/Acts/Propagator/EigenStepperDenseExtension.hpp +++ b/Core/include/Acts/Propagator/EigenStepperDenseExtension.hpp @@ -12,11 +12,11 @@ #include "Acts/Utilities/detail/ReferenceWrapperAnyCompat.hpp" #include "Acts/Definitions/Algebra.hpp" +#include "Acts/Material/IVolumeMaterial.hpp" #include "Acts/Material/Interactions.hpp" #include "Acts/Material/Material.hpp" #include "Acts/Material/MaterialSlab.hpp" #include "Acts/Propagator/EigenStepperDefaultExtension.hpp" -#include "Acts/Propagator/Propagator.hpp" #include "Acts/Utilities/MathHelpers.hpp" namespace Acts { @@ -58,13 +58,11 @@ struct EigenStepperDenseExtension { /// step sets up member parameters, too. /// /// @tparam i Index of the k_i, i = [0, 3] - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper - /// @tparam navigator_t Type of the navigator /// - /// @param [in] state State of the propagator + /// @param [in] state State of the stepper /// @param [in] stepper Stepper of the propagator - /// @param [in] navigator Navigator of the propagator + /// @param [in] volumeMaterial Material of the volume /// @param [out] knew Next k_i that is evaluated /// @param [out] kQoP k_i elements of the momenta /// @param [in] bField B-Field at the evaluation position @@ -72,36 +70,33 @@ struct EigenStepperDenseExtension { /// @param [in] kprev Evaluated k_{i - 1} /// /// @return Boolean flag if the calculation is valid - template - bool k(const propagator_state_t& state, const stepper_t& stepper, - const navigator_t& navigator, Vector3& knew, const Vector3& bField, - std::array& kQoP, const double h = 0., - const Vector3& kprev = Vector3::Zero()) + template + bool k(const typename stepper_t::State& state, const stepper_t& stepper, + const IVolumeMaterial* volumeMaterial, Vector3& knew, + const Vector3& bField, std::array& kQoP, + const double h = 0., const Vector3& kprev = Vector3::Zero()) requires(i >= 0 && i <= 3) { - const auto* volumeMaterial = - navigator.currentVolumeMaterial(state.navigation); if (volumeMaterial == nullptr) { - return defaultExtension.template k(state, stepper, navigator, knew, - bField, kQoP, h, kprev); + return defaultExtension.template k(state, stepper, volumeMaterial, + knew, bField, kQoP, h, kprev); } - double q = stepper.charge(state.stepping); - const auto& particleHypothesis = stepper.particleHypothesis(state.stepping); + double q = stepper.charge(state); + const auto& particleHypothesis = stepper.particleHypothesis(state); float mass = particleHypothesis.mass(); // i = 0 is used for setup and evaluation of k if constexpr (i == 0) { // Set up for energy loss - Vector3 position = stepper.position(state.stepping); + Vector3 position = stepper.position(state); material = volumeMaterial->material(position.template cast()); - initialMomentum = stepper.absoluteMomentum(state.stepping); + initialMomentum = stepper.absoluteMomentum(state); currentMomentum = initialMomentum; - qop[0] = stepper.qOverP(state.stepping); + qop[0] = stepper.qOverP(state); initializeEnergyLoss(state, stepper); // Evaluate k - knew = qop[0] * stepper.direction(state.stepping).cross(bField); + knew = qop[0] * stepper.direction(state).cross(bField); // Evaluate k for the time propagation Lambdappi[0] = -qop[0] * qop[0] * qop[0] * g * energy[0] / (q * q); //~ tKi[0] = std::hypot(1, mass / initialMomentum); @@ -110,12 +105,11 @@ struct EigenStepperDenseExtension { } else { // Update parameters and check for momentum condition updateEnergyLoss(mass, h, state, stepper, i); - if (currentMomentum < state.options.stepping.dense.momentumCutOff) { + if (currentMomentum < state.options.dense.momentumCutOff) { return false; } // Evaluate k - knew = qop[i] * - (stepper.direction(state.stepping) + h * kprev).cross(bField); + knew = qop[i] * (stepper.direction(state) + h * kprev).cross(bField); // Evaluate k_i for the time propagation auto qopNew = qop[0] + h * Lambdappi[i - 1]; Lambdappi[i] = -qopNew * qopNew * qopNew * g * energy[i] / (q * q); @@ -130,50 +124,44 @@ struct EigenStepperDenseExtension { /// of the energy loss and the therewith constrained to keep the momentum /// after the step in reasonable values. /// - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper - /// @tparam navigator_t Type of the navigator /// - /// @param [in] state State of the propagator + /// @param [in] state State of the stepper /// @param [in] stepper Stepper of the propagator - /// @param [in] navigator Navigator of the propagator + /// @param [in] volumeMaterial Material of the volume /// @param [in] h Step size /// /// @return Boolean flag if the calculation is valid - template - bool finalize(propagator_state_t& state, const stepper_t& stepper, - const navigator_t& navigator, const double h) const { - const auto* volumeMaterial = - navigator.currentVolumeMaterial(state.navigation); + template + bool finalize(typename stepper_t::State& state, const stepper_t& stepper, + const IVolumeMaterial* volumeMaterial, const double h) const { if (volumeMaterial == nullptr) { - return defaultExtension.finalize(state, stepper, navigator, h); + return defaultExtension.finalize(state, stepper, volumeMaterial, h); } - const auto& particleHypothesis = stepper.particleHypothesis(state.stepping); + const auto& particleHypothesis = stepper.particleHypothesis(state); float mass = particleHypothesis.mass(); // Evaluate the new momentum auto newMomentum = - stepper.absoluteMomentum(state.stepping) + + stepper.absoluteMomentum(state) + (h / 6.) * (dPds[0] + 2. * (dPds[1] + dPds[2]) + dPds[3]); // Break propagation if momentum becomes below cut-off - if (newMomentum < state.options.stepping.dense.momentumCutOff) { + if (newMomentum < state.options.dense.momentumCutOff) { return false; } // Add derivative dlambda/ds = Lambda'' - state.stepping.derivative(7) = -fastHypot(mass, newMomentum) * g / - (newMomentum * newMomentum * newMomentum); + state.derivative(7) = -fastHypot(mass, newMomentum) * g / + (newMomentum * newMomentum * newMomentum); // Update momentum - state.stepping.pars[eFreeQOverP] = - stepper.charge(state.stepping) / newMomentum; + state.pars[eFreeQOverP] = stepper.charge(state) / newMomentum; // Add derivative dt/ds = 1/(beta * c) = sqrt(m^2 * p^{-2} + c^{-2}) - state.stepping.derivative(3) = fastHypot(1, mass / newMomentum); + state.derivative(3) = fastHypot(1, mass / newMomentum); // Update time - state.stepping.pars[eFreeTime] += + state.pars[eFreeTime] += (h / 6.) * (tKi[0] + 2. * (tKi[1] + tKi[2]) + tKi[3]); return true; @@ -186,29 +174,24 @@ struct EigenStepperDenseExtension { /// after the step in reasonable values and the evaluation of the transport /// matrix. /// - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper - /// @tparam navigator_t Type of the navigator /// - /// @param [in] state State of the propagator + /// @param [in] state State of the stepper /// @param [in] stepper Stepper of the propagator - /// @param [in] navigator Navigator of the propagator + /// @param [in] volumeMaterial Material of the volume /// @param [in] h Step size /// @param [out] D Transport matrix /// /// @return Boolean flag if the calculation is valid - template - bool finalize(propagator_state_t& state, const stepper_t& stepper, - const navigator_t& navigator, const double h, + template + bool finalize(typename stepper_t::State& state, const stepper_t& stepper, + const IVolumeMaterial* volumeMaterial, const double h, FreeMatrix& D) const { - const auto* volumeMaterial = - navigator.currentVolumeMaterial(state.navigation); if (volumeMaterial == nullptr) { - return defaultExtension.finalize(state, stepper, navigator, h, D); + return defaultExtension.finalize(state, stepper, volumeMaterial, h, D); } - return finalize(state, stepper, navigator, h) && + return finalize(state, stepper, volumeMaterial, h) && transportMatrix(state, stepper, h, D); } @@ -224,9 +207,10 @@ struct EigenStepperDenseExtension { /// @param [out] D Transport matrix /// /// @return Boolean flag if evaluation is valid - template - bool transportMatrix(propagator_state_t& state, const stepper_t& stepper, - const double h, FreeMatrix& D) const { + template + bool transportMatrix(typename stepper_t::State& state, + const stepper_t& stepper, const double h, + FreeMatrix& D) const { /// The calculations are based on ATL-SOFT-PUB-2009-002. The update of the /// Jacobian matrix is requires only the calculation of eq. 17 and 18. /// Since the terms of eq. 18 are currently 0, this matrix is not needed @@ -247,9 +231,9 @@ struct EigenStepperDenseExtension { /// constant offset does not exist for rectangular matrix dFdu' (due to the /// missing Lambda part) and only exists for dGdu' in dlambda/dlambda. - auto& sd = state.stepping.stepData; - auto dir = stepper.direction(state.stepping); - const auto& particleHypothesis = stepper.particleHypothesis(state.stepping); + auto& sd = state.stepData; + auto dir = stepper.direction(state); + const auto& particleHypothesis = stepper.particleHypothesis(state); float mass = particleHypothesis.mass(); D = FreeMatrix::Identity(); @@ -360,15 +344,14 @@ struct EigenStepperDenseExtension { /// @brief Initializer of all parameters related to a RKN4 step with energy /// loss of a particle in material /// - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper /// /// @param [in] state Deliverer of configurations /// @param [in] stepper Stepper of the propagator - template - void initializeEnergyLoss(const propagator_state_t& state, + template + void initializeEnergyLoss(const typename stepper_t::State& state, const stepper_t& stepper) { - const auto& particleHypothesis = stepper.particleHypothesis(state.stepping); + const auto& particleHypothesis = stepper.particleHypothesis(state); float mass = particleHypothesis.mass(); PdgParticle absPdg = particleHypothesis.absolutePdg(); float absQ = particleHypothesis.absoluteCharge(); @@ -377,7 +360,7 @@ struct EigenStepperDenseExtension { // use unit length as thickness to compute the energy loss per unit length MaterialSlab slab(material, 1); // Use the same energy loss throughout the step. - if (state.options.stepping.dense.meanEnergyLoss) { + if (state.options.dense.meanEnergyLoss) { g = -computeEnergyLossMean(slab, absPdg, mass, static_cast(qop[0]), absQ); } else { @@ -389,11 +372,11 @@ struct EigenStepperDenseExtension { // Change of the momentum per path length // dPds = dPdE * dEds dPds[0] = g * energy[0] / initialMomentum; - if (state.stepping.covTransport) { + if (state.covTransport) { // Calculate the change of the energy loss per path length and // inverse momentum - if (state.options.stepping.dense.includeGradient) { - if (state.options.stepping.dense.meanEnergyLoss) { + if (state.options.dense.includeGradient) { + if (state.options.dense.meanEnergyLoss) { dgdqopValue = deriveEnergyLossMeanQOverP( slab, absPdg, mass, static_cast(qop[0]), absQ); } else { @@ -413,7 +396,6 @@ struct EigenStepperDenseExtension { /// @brief Update of the kinematic parameters of the RKN4 sub-steps after /// initialization with energy loss of a particle in material /// - /// @tparam propagator_state_t Type of the state of the propagator /// @tparam stepper_t Type of the stepper /// /// @param [in] h Stepped distance of the sub-step (1-3) @@ -421,17 +403,17 @@ struct EigenStepperDenseExtension { /// @param [in] state State of the stepper /// @param [in] stepper Stepper of the propagator /// @param [in] i Index of the sub-step (1-3) - template + template void updateEnergyLoss(const double mass, const double h, - const propagator_state_t& state, + const typename stepper_t::State& state, const stepper_t& stepper, const int i) { // Update parameters related to a changed momentum currentMomentum = initialMomentum + h * dPds[i - 1]; energy[i] = fastHypot(currentMomentum, mass); dPds[i] = g * energy[i] / currentMomentum; - qop[i] = stepper.charge(state.stepping) / currentMomentum; + qop[i] = stepper.charge(state) / currentMomentum; // Calculate term for later error propagation - if (state.stepping.covTransport) { + if (state.covTransport) { dLdl[i] = (-qop[i] * qop[i] * g * energy[i] * (3. - (currentMomentum * currentMomentum) / (energy[i] * energy[i])) - diff --git a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp index babe5836f73..6ecdeeb5d7b 100644 --- a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp +++ b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp @@ -665,13 +665,10 @@ class MultiEigenStepperLoop : public EigenStepper { /// Compute path length derivatives in case they have not been computed /// yet, which is the case if no step has been executed yet. /// - /// @param [in, out] prop_state State that will be presented as @c BoundState - /// @param [in] navigator the navigator of the propagation + /// @param [in, out] state The stepping state (thread-local cache) /// @return true if nothing is missing after this call, false otherwise. - template - bool prepareCurvilinearState( - [[maybe_unused]] propagator_state_t& prop_state, - [[maybe_unused]] const navigator_t& navigator) const { + bool prepareCurvilinearState(State& state) const { + (void)state; return true; } @@ -726,16 +723,16 @@ class MultiEigenStepperLoop : public EigenStepper { /// Perform a Runge-Kutta track parameter propagation step /// - /// @param [in,out] state is the propagation state associated with the track - /// parameters that are being propagated. - /// @param [in] navigator is the navigator of the propagation + /// @param [in,out] state The state of the stepper + /// @param propDir is the direction of propagation + /// @param material is the material properties + /// @return the result of the step /// /// The state contains the desired step size. It can be negative during /// backwards track propagation, and since we're using an adaptive /// algorithm, it can be modified by the stepper class during propagation. - template - Result step(propagator_state_t& state, - const navigator_t& navigator) const; + Result step(State& state, Direction propDir, + const IVolumeMaterial* material) const; }; } // namespace Acts diff --git a/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp b/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp index 01b392dbf2a..0d9cb418d38 100644 --- a/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp +++ b/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp @@ -90,25 +90,23 @@ auto MultiEigenStepperLoop::curvilinearState( } template -template Result MultiEigenStepperLoop::step( - propagator_state_t& state, const navigator_t& navigator) const { + State& state, Direction propDir, const IVolumeMaterial* material) const { using Status = Acts::IntersectionStatus; - State& stepping = state.stepping; - auto& components = stepping.components; + auto& components = state.components; const Logger& logger = *m_logger; // Update step count - stepping.steps++; + state.steps++; // Check if we abort because of m_stepLimitAfterFirstComponentOnSurface - if (stepping.stepCounterAfterFirstComponentOnSurface) { - (*stepping.stepCounterAfterFirstComponentOnSurface)++; + if (state.stepCounterAfterFirstComponentOnSurface) { + (*state.stepCounterAfterFirstComponentOnSurface)++; // If the limit is reached, remove all components which are not on a // surface, reweight the components, perform no step and return 0 - if (*stepping.stepCounterAfterFirstComponentOnSurface >= + if (*state.stepCounterAfterFirstComponentOnSurface >= m_stepLimitAfterFirstComponentOnSurface) { for (auto& cmp : components) { if (cmp.status != Status::onSurface) { @@ -122,13 +120,13 @@ Result MultiEigenStepperLoop::step( ACTS_VERBOSE( "-> remove all components not on a surface, perform no step"); - removeMissedComponents(stepping); - reweightComponents(stepping); + removeMissedComponents(state); + reweightComponents(state); ACTS_VERBOSE(components.size() << " components left after removing missed components"); - stepping.stepCounterAfterFirstComponentOnSurface.reset(); + state.stepCounterAfterFirstComponentOnSurface.reset(); return 0.0; } @@ -145,7 +143,7 @@ Result MultiEigenStepperLoop::step( [&](auto& cmp) { return cmp.status == IntersectionStatus::onSurface; }); if (cmpsOnSurface > 0) { - removeMissedComponents(stepping); + removeMissedComponents(state); reweightNecessary = true; } @@ -155,15 +153,10 @@ Result MultiEigenStepperLoop::step( double accumulatedPathLength = 0.0; std::size_t errorSteps = 0; - // Type of the proxy single propagation2 state - using ThisSinglePropState = - detail::SinglePropState; - // Lambda that performs the step for a component and returns false if the step // went ok and true if there was an error - auto errorInStep = [&](auto& component) { + auto errorInStep = [this, &results, propDir, material, &accumulatedPathLength, + &errorSteps, &reweightNecessary](auto& component) { if (component.status == Status::onSurface) { // We need to add these, so the propagation does not fail if we have only // components on surfaces and failing states @@ -171,10 +164,8 @@ Result MultiEigenStepperLoop::step( return false; } - ThisSinglePropState single_state(component.state, state.navigation, - state.options, state.geoContext); - - results.emplace_back(SingleStepper::step(single_state, navigator)); + results.emplace_back( + SingleStepper::step(component.state, propDir, material)); if (results.back()->ok()) { accumulatedPathLength += component.weight * results.back()->value(); @@ -193,7 +184,7 @@ Result MultiEigenStepperLoop::step( // Reweight if necessary if (reweightNecessary) { - reweightComponents(stepping); + reweightComponents(state); } // Print the result vector to a string so we can log it @@ -231,7 +222,7 @@ Result MultiEigenStepperLoop::step( } // Return the weighted accumulated path length of all successful steps - stepping.pathAccumulated += accumulatedPathLength; + state.pathAccumulated += accumulatedPathLength; return accumulatedPathLength; } diff --git a/Core/include/Acts/Propagator/Propagator.hpp b/Core/include/Acts/Propagator/Propagator.hpp index 3c85c50e09e..4129a7dc7bc 100644 --- a/Core/include/Acts/Propagator/Propagator.hpp +++ b/Core/include/Acts/Propagator/Propagator.hpp @@ -92,7 +92,7 @@ class BasePropagatorHelper : public BasePropagator { template class Propagator final : public std::conditional_t< - SupportsBoundParameters_v, + SupportsBoundParameters_v, detail::BasePropagatorHelper>, detail::PropagatorStub> { /// Re-define bound track parameters dependent on the stepper diff --git a/Core/include/Acts/Propagator/Propagator.ipp b/Core/include/Acts/Propagator/Propagator.ipp index ef1a6383461..fbeb79df36a 100644 --- a/Core/include/Acts/Propagator/Propagator.ipp +++ b/Core/include/Acts/Propagator/Propagator.ipp @@ -18,14 +18,6 @@ #include -namespace Acts::detail { -template -concept propagator_stepper_compatible_with = - requires(const Stepper& s, StateType& st, const N& n) { - { s.step(st, n) } -> std::same_as>; - }; -} // namespace Acts::detail - template template Acts::Result Acts::Propagator::propagate( @@ -89,7 +81,9 @@ Acts::Result Acts::Propagator::propagate( // Stepping loop for (; state.steps < state.options.maxSteps; ++state.steps) { // Perform a step - Result res = m_stepper.step(state, m_navigator); + Result res = + m_stepper.step(state.stepping, state.options.direction, + m_navigator.currentVolumeMaterial(state.navigation)); if (!res.ok()) { ACTS_ERROR("Step failed with " << res.error() << ": " << res.error().message()); @@ -263,12 +257,6 @@ auto Acts::Propagator::makeState( actor_list_t_state_t; - static_assert( - detail::propagator_stepper_compatible_with, - "Step method of the Stepper is not compatible with the propagator " - "state"); - - // Initialize the internal propagator state StateType state{eOptions, m_stepper.makeState(eOptions.stepping), m_navigator.makeState(eOptions.navigation)}; @@ -297,12 +285,6 @@ auto Acts::Propagator::makeState( actor_list_t_state_t; - static_assert( - detail::propagator_stepper_compatible_with, - "Step method of the Stepper is not compatible with the propagator " - "state"); - - // Initialize the internal propagator state StateType state{eOptions, m_stepper.makeState(eOptions.stepping), m_navigator.makeState(eOptions.navigation)}; @@ -369,7 +351,7 @@ auto Acts::Propagator::makeResult(propagator_state_t state, moveStateToResult(state, result); if (makeCurvilinear) { - if (!m_stepper.prepareCurvilinearState(state, m_navigator)) { + if (!m_stepper.prepareCurvilinearState(state.stepping)) { // information to compute curvilinearState is incomplete. return propagationResult.error(); } diff --git a/Core/include/Acts/Propagator/PropagatorState.hpp b/Core/include/Acts/Propagator/PropagatorState.hpp index f8b639d1faa..e7abb7fc6ac 100644 --- a/Core/include/Acts/Propagator/PropagatorState.hpp +++ b/Core/include/Acts/Propagator/PropagatorState.hpp @@ -36,6 +36,8 @@ template struct PropagatorState : private detail::Extendable { using options_type = propagator_options_t; + using stepper_state_type = stepper_state_t; + using navigator_state_type = navigator_state_t; /// Create the propagator state from the options /// diff --git a/Core/include/Acts/Propagator/PropagatorTraits.hpp b/Core/include/Acts/Propagator/PropagatorTraits.hpp index 2bd1567fdb2..a19cd9c6617 100644 --- a/Core/include/Acts/Propagator/PropagatorTraits.hpp +++ b/Core/include/Acts/Propagator/PropagatorTraits.hpp @@ -9,11 +9,14 @@ #pragma once #include + namespace Acts { -template + +template struct SupportsBoundParameters : public std::false_type {}; -template +template constexpr bool SupportsBoundParameters_v = - SupportsBoundParameters::value; + SupportsBoundParameters::value; + } // namespace Acts diff --git a/Core/include/Acts/Propagator/StraightLineStepper.hpp b/Core/include/Acts/Propagator/StraightLineStepper.hpp index ca62a5e62d4..b527ea3a04c 100644 --- a/Core/include/Acts/Propagator/StraightLineStepper.hpp +++ b/Core/include/Acts/Propagator/StraightLineStepper.hpp @@ -35,6 +35,8 @@ namespace Acts { +class IVolumeMaterial; + /// @brief straight line stepper based on Surface intersection /// /// The straight line stepper is a simple navigation stepper @@ -280,28 +282,24 @@ class StraightLineStepper { /// Compute path length derivatives in case they have not been computed /// yet, which is the case if no step has been executed yet. /// - /// @param [in, out] prop_state State that will be presented as @c BoundState - /// @param [in] navigator the navigator of the propagation + /// @param [in, out] state The stepping state (thread-local cache) /// @return true if nothing is missing after this call, false otherwise. - template - bool prepareCurvilinearState( - [[maybe_unused]] propagator_state_t& prop_state, - [[maybe_unused]] const navigator_t& navigator) const { + bool prepareCurvilinearState(State& state) const { // test whether the accumulated path has still its initial value. - if (prop_state.stepping.pathAccumulated == 0.) { - // dr/ds : - prop_state.stepping.derivative.template head<3>() = - direction(prop_state.stepping); - // dt / ds - prop_state.stepping.derivative(eFreeTime) = - fastHypot(1., prop_state.stepping.particleHypothesis.mass() / - absoluteMomentum(prop_state.stepping)); - // d (dr/ds) / ds : == 0 - prop_state.stepping.derivative.template segment<3>(4) = - Acts::Vector3::Zero().transpose(); - // d qop / ds == 0 - prop_state.stepping.derivative(eFreeQOverP) = 0.; + if (state.pathAccumulated != 0) { + return true; } + + // dr/ds : + state.derivative.template head<3>() = direction(state); + // dt / ds + state.derivative(eFreeTime) = fastHypot( + 1., state.particleHypothesis.mass() / absoluteMomentum(state)); + // d (dr/ds) / ds : == 0 + state.derivative.template segment<3>(4) = Acts::Vector3::Zero().transpose(); + // d qop / ds == 0 + state.derivative(eFreeQOverP) = 0.; + return true; } @@ -366,62 +364,62 @@ class StraightLineStepper { /// Perform a straight line propagation step /// - /// @param [in,out] state is the propagation state associated with the track - /// parameters that are being propagated. - /// The state contains the desired step size, - /// it can be negative during backwards track propagation, - /// and since we're using an adaptive algorithm, it can - /// be modified by the stepper class during propagation. - /// - /// @return the step size taken - template - Result step(propagator_state_t& state, - const navigator_t& /*navigator*/) const { + /// @param [in,out] state State of the stepper + /// @param propDir is the direction of propagation + /// @param material is the optional volume material we are stepping through. + // This is simply ignored if `nullptr`. + /// @return the result of the step + /// + /// @note The state contains the desired step size. It can be negative during + /// backwards track propagation. + Result step(State& state, Direction propDir, + const IVolumeMaterial* material) const { + (void)material; + // use the adjusted step size - const auto h = state.stepping.stepSize.value() * state.options.direction; - const auto m = state.stepping.particleHypothesis.mass(); - const auto p = absoluteMomentum(state.stepping); + const auto h = state.stepSize.value() * propDir; + const auto m = state.particleHypothesis.mass(); + const auto p = absoluteMomentum(state); // time propagates along distance as 1/b = sqrt(1 + m²/p²) const auto dtds = fastHypot(1., m / p); // Update the track parameters according to the equations of motion - Vector3 dir = direction(state.stepping); - state.stepping.pars.template segment<3>(eFreePos0) += h * dir; - state.stepping.pars[eFreeTime] += h * dtds; + Vector3 dir = direction(state); + state.pars.template segment<3>(eFreePos0) += h * dir; + state.pars[eFreeTime] += h * dtds; // Propagate the jacobian - if (state.stepping.covTransport) { + if (state.covTransport) { // The step transport matrix in global coordinates FreeMatrix D = FreeMatrix::Identity(); D.block<3, 3>(0, 4) = ActsSquareMatrix<3>::Identity() * h; // Extend the calculation by the time propagation // Evaluate dt/dlambda - D(3, 7) = h * m * m * state.stepping.pars[eFreeQOverP] / dtds; + D(3, 7) = h * m * m * state.pars[eFreeQOverP] / dtds; // Set the derivative factor the time - state.stepping.derivative(3) = dtds; + state.derivative(3) = dtds; // Update jacobian and derivative - state.stepping.jacTransport = D * state.stepping.jacTransport; - state.stepping.derivative.template head<3>() = dir; + state.jacTransport = D * state.jacTransport; + state.derivative.template head<3>() = dir; } // state the path length - state.stepping.pathAccumulated += h; - ++state.stepping.nSteps; - ++state.stepping.nStepTrials; - - ++state.stepping.statistics.nAttemptedSteps; - ++state.stepping.statistics.nSuccessfulSteps; - if (state.options.direction != Direction::fromScalarZeroAsPositive(h)) { - ++state.stepping.statistics.nReverseSteps; + state.pathAccumulated += h; + ++state.nSteps; + ++state.nStepTrials; + + ++state.statistics.nAttemptedSteps; + ++state.statistics.nSuccessfulSteps; + if (propDir != Direction::fromScalarZeroAsPositive(h)) { + ++state.statistics.nReverseSteps; } - state.stepping.statistics.pathLength += h; - state.stepping.statistics.absolutePathLength += std::abs(h); + state.statistics.pathLength += h; + state.statistics.absolutePathLength += std::abs(h); return h; } }; -template -struct SupportsBoundParameters - : public std::true_type {}; +template <> +struct SupportsBoundParameters : public std::true_type {}; } // namespace Acts diff --git a/Core/include/Acts/Propagator/SympyStepper.hpp b/Core/include/Acts/Propagator/SympyStepper.hpp index 38d7a59e0c7..f64be7cb8f0 100644 --- a/Core/include/Acts/Propagator/SympyStepper.hpp +++ b/Core/include/Acts/Propagator/SympyStepper.hpp @@ -24,6 +24,8 @@ namespace Acts { +class IVolumeMaterial; + class SympyStepper { public: /// Jacobian, Covariance and State definitions @@ -291,15 +293,9 @@ class SympyStepper { /// Compute path length derivatives in case they have not been computed /// yet, which is the case if no step has been executed yet. /// - /// @param [in, out] prop_state State that will be presented as @c BoundState - /// @param [in] navigator the navigator of the propagation + /// @param [in, out] state State of the stepper /// @return true if nothing is missing after this call, false otherwise. - template - bool prepareCurvilinearState( - [[maybe_unused]] propagator_state_t& prop_state, - [[maybe_unused]] const navigator_t& navigator) const { - return true; - } + bool prepareCurvilinearState(State& state) const; /// Create and return a curvilinear state at the current position /// @@ -361,15 +357,18 @@ class SympyStepper { /// Perform a Runge-Kutta track parameter propagation step /// - /// @param [in,out] state the propagation state - /// @param [in] navigator the navigator of the propagation - /// @note The state contains the desired step size. It can be negative during + /// @param [in,out] state State of the stepper + /// @param propDir is the direction of propagation + /// @param material is the optional volume material we are stepping through. + // This is simply ignored if `nullptr`. + /// @return the result of the step + /// + /// @note The state contains the desired step size. It can be negative during /// backwards track propagation, and since we're using an adaptive /// algorithm, it can be modified by the stepper class during /// propagation. - template - Result step(propagator_state_t& state, - const navigator_t& navigator) const; + Result step(State& state, Direction propDir, + const IVolumeMaterial* material) const; /// Method that reset the Jacobian to the Identity for when no bound state are /// available @@ -382,15 +381,12 @@ class SympyStepper { std::shared_ptr m_bField; private: - Result stepImpl(State& state, Direction stepDirection, - double stepTolerance, double stepSizeCutOff, + Result stepImpl(State& state, Direction propDir, double stepTolerance, + double stepSizeCutOff, std::size_t maxRungeKuttaStepTrials) const; }; -template -struct SupportsBoundParameters - : public std::true_type {}; +template <> +struct SupportsBoundParameters : public std::true_type {}; } // namespace Acts - -#include "Acts/Propagator/SympyStepper.ipp" diff --git a/Core/include/Acts/Propagator/SympyStepper.ipp b/Core/include/Acts/Propagator/SympyStepper.ipp deleted file mode 100644 index 30e91427026..00000000000 --- a/Core/include/Acts/Propagator/SympyStepper.ipp +++ /dev/null @@ -1,22 +0,0 @@ -// 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 "Acts/Propagator/EigenStepperError.hpp" - -namespace Acts { - -template -Result SympyStepper::step(propagator_state_t& state, - const navigator_t& /*navigator*/) const { - return stepImpl(state.stepping, state.options.direction, - state.options.stepping.stepTolerance, - state.options.stepping.stepSizeCutOff, - state.options.stepping.maxRungeKuttaStepTrials); -} - -} // namespace Acts diff --git a/Core/include/Acts/Propagator/VoidNavigator.hpp b/Core/include/Acts/Propagator/VoidNavigator.hpp index 66a4a0c875e..c5df6647d71 100644 --- a/Core/include/Acts/Propagator/VoidNavigator.hpp +++ b/Core/include/Acts/Propagator/VoidNavigator.hpp @@ -17,6 +17,8 @@ namespace Acts { +class TrackingVolume; +class IVolumeMaterial; class Surface; /// @brief A navigator that does nothing @@ -57,6 +59,14 @@ class VoidNavigator { return nullptr; } + const TrackingVolume* currentVolume(const State& /*state*/) const { + return nullptr; + } + + const IVolumeMaterial* currentVolumeMaterial(const State& /*state*/) const { + return nullptr; + } + const Surface* startSurface(const State& /*state*/) const { return nullptr; } const Surface* targetSurface(const State& /*state*/) const { return nullptr; } diff --git a/Core/include/Acts/Propagator/detail/LoopStepperUtils.hpp b/Core/include/Acts/Propagator/detail/LoopStepperUtils.hpp index 26ddb590a8f..02b9d2e5b41 100644 --- a/Core/include/Acts/Propagator/detail/LoopStepperUtils.hpp +++ b/Core/include/Acts/Propagator/detail/LoopStepperUtils.hpp @@ -8,6 +8,11 @@ #pragma once +#include +#include +#include +#include + namespace Acts::detail { /// A helper type for providinig a propagation state which can be used with @@ -41,6 +46,7 @@ struct LoopComponentProxyBase { // These are the const accessors, which are shared between the mutable // ComponentProxy and the ConstComponentProxy + const auto& state() const { return cmp.state; } auto status() const { return cmp.status; } auto weight() const { return cmp.weight; } auto pathAccumulated() const { return cmp.state.pathAccumulated; } @@ -92,6 +98,7 @@ struct LoopComponentProxy using Base::pathAccumulated; using Base::singleState; using Base::singleStepper; + using Base::state; using Base::status; using Base::weight; @@ -103,6 +110,7 @@ struct LoopComponentProxy // These are the mutable accessors, the const ones are inherited from the // ComponentProxyBase + auto& state() { return cmp.state; } auto& status() { return cmp.status; } auto& weight() { return cmp.weight; } auto& pathAccumulated() { return cmp.state.pathAccumulated; } diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp index bb940acbdb4..f2d0a22a75d 100644 --- a/Core/include/Acts/TrackFitting/KalmanFitter.hpp +++ b/Core/include/Acts/TrackFitting/KalmanFitter.hpp @@ -657,7 +657,7 @@ class KalmanFitter { // detected or if the surface has material (no holes before the first // measurement) auto trackStateProxyRes = detail::kalmanHandleNoMeasurement( - state, stepper, *surface, *result.fittedStates, + state.stepping, stepper, *surface, *result.fittedStates, result.lastTrackIndex, true, logger(), precedingMeasurementExists, freeToBoundCorrection); diff --git a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp index 0b3a8053870..9791cb55ca6 100644 --- a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp +++ b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp @@ -235,9 +235,8 @@ struct GsfActor { } for (auto cmp : stepper.componentIterable(state.stepping)) { - auto singleState = cmp.singleState(state); - cmp.singleStepper(stepper).transportCovarianceToBound( - singleState.stepping, surface); + cmp.singleStepper(stepper).transportCovarianceToBound(cmp.state(), + surface); } if (haveMaterial) { @@ -640,9 +639,8 @@ struct GsfActor { // components should behave the same bool isHole = true; - auto cmps = stepper.componentIterable(state.stepping); - for (auto cmp : cmps) { - auto singleState = cmp.singleState(state); + for (auto cmp : stepper.componentIterable(state.stepping)) { + auto& singleState = cmp.state(); const auto& singleStepper = cmp.singleStepper(stepper); // There is some redundant checking inside this function, but do this for diff --git a/Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp b/Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp index f4230e0e18d..1098c342672 100644 --- a/Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp +++ b/Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp @@ -10,7 +10,6 @@ #include "Acts/EventData/MultiTrajectory.hpp" #include "Acts/EventData/SourceLink.hpp" -#include "Acts/EventData/TrackParameters.hpp" #include "Acts/EventData/detail/CorrectedTransformationFreeToBound.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Utilities/CalibrationContext.hpp" @@ -121,7 +120,6 @@ auto kalmanHandleMeasurement( /// If there are no source links on surface, add either a hole or passive /// material TrackState entry multi trajectory. No storage allocation for /// uncalibrated/calibrated measurement and filtered parameter -/// @tparam propagator_state_t The propagator state type /// @tparam stepper_t The stepper type /// @param state The propagator state /// @param stepper The stepper @@ -131,11 +129,12 @@ auto kalmanHandleMeasurement( /// @param doCovTransport Whether to perform a covariance transport when /// computing the bound state or not /// @param freeToBoundCorrection Correction for non-linearity effect during transform from free to bound (only corrected when performing CovTransport) -template +template auto kalmanHandleNoMeasurement( - propagator_state_t &state, const stepper_t &stepper, const Surface &surface, - traj_t &fittedStates, const std::size_t lastTrackIndex, bool doCovTransport, - const Logger &logger, const bool precedingMeasurementExists, + typename stepper_t::State &state, const stepper_t &stepper, + const Surface &surface, traj_t &fittedStates, + const std::size_t lastTrackIndex, bool doCovTransport, const Logger &logger, + const bool precedingMeasurementExists, const FreeToBoundCorrection &freeToBoundCorrection = FreeToBoundCorrection( false)) -> Result { // Add a TrackState entry multi trajectory. This allocates storage for @@ -151,7 +150,7 @@ auto kalmanHandleNoMeasurement( { trackStateProxy.setReferenceSurface(surface.getSharedPtr()); // Bind the transported state to the current surface - auto res = stepper.boundState(state.stepping, surface, doCovTransport, + auto res = stepper.boundState(state, surface, doCovTransport, freeToBoundCorrection); if (!res.ok()) { return res.error(); @@ -160,7 +159,7 @@ auto kalmanHandleNoMeasurement( // Fill the track state trackStateProxy.predicted() = boundParams.parameters(); - trackStateProxy.predictedCovariance() = state.stepping.cov; + trackStateProxy.predictedCovariance() = state.cov; trackStateProxy.jacobian() = jacobian; trackStateProxy.pathLength() = pathLength; diff --git a/Core/src/Propagator/SympyStepper.cpp b/Core/src/Propagator/SympyStepper.cpp index da2be207f13..bc0cf3c2eb4 100644 --- a/Core/src/Propagator/SympyStepper.cpp +++ b/Core/src/Propagator/SympyStepper.cpp @@ -8,7 +8,7 @@ #include "Acts/Propagator/SympyStepper.hpp" -#include "Acts/Definitions/TrackParametrization.hpp" +#include "Acts/Propagator/EigenStepperError.hpp" #include "Acts/Propagator/detail/SympyCovarianceEngine.hpp" #include "Acts/Propagator/detail/SympyJacobianEngine.hpp" @@ -77,6 +77,12 @@ SympyStepper::boundState( state.pathAccumulated, freeToBoundCorrection); } +bool SympyStepper::prepareCurvilinearState(State& state) const { + // TODO implement like in EigenStepper + (void)state; + return true; +} + std::tuple SympyStepper::curvilinearState(State& state, bool transportCov) const { return detail::sympy::curvilinearState( @@ -120,8 +126,16 @@ void SympyStepper::transportCovarianceToBound( freeToBoundCorrection); } +Result SympyStepper::step(State& state, Direction propDir, + const IVolumeMaterial* material) const { + (void)material; + return stepImpl(state, propDir, state.options.stepTolerance, + state.options.stepSizeCutOff, + state.options.maxRungeKuttaStepTrials); +} + Result SympyStepper::stepImpl( - State& state, Direction stepDirection, double stepTolerance, + State& state, Direction propDir, double stepTolerance, double stepSizeCutOff, std::size_t maxRungeKuttaStepTrials) const { auto pos = position(state); auto dir = direction(state); @@ -153,7 +167,7 @@ Result SympyStepper::stepImpl( return std::clamp(x, lower, upper); }; - double h = state.stepSize.value() * stepDirection; + double h = state.stepSize.value() * propDir; double initialH = h; std::size_t nStepTrials = 0; double errorEstimate = 0.; @@ -205,7 +219,7 @@ Result SympyStepper::stepImpl( state.nStepTrials += nStepTrials; ++state.statistics.nSuccessfulSteps; - if (stepDirection != Direction::fromScalarZeroAsPositive(initialH)) { + if (propDir != Direction::fromScalarZeroAsPositive(initialH)) { ++state.statistics.nReverseSteps; } state.statistics.pathLength += h; diff --git a/Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp b/Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp index f589e240f48..e16604e063a 100644 --- a/Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp @@ -55,26 +55,6 @@ using Covariance = BoundSquareMatrix; using Jacobian = BoundMatrix; using Stepper = Acts::AtlasStepper; -/// Simplified propagator state. -struct MockPropagatorState { - MockPropagatorState(Stepper::State stepperState) - : stepping(std::move(stepperState)) {} - - /// Stepper state. - Stepper::State stepping; - /// Propagator options with only the relevant components. - struct { - Direction direction = Direction::Backward(); - struct { - double stepTolerance = 10_um; - } stepping; - } options; -}; - -struct MockNavigator {}; - -static constexpr MockNavigator navigator; - // epsilon for floating point comparisons static constexpr auto eps = 1024 * std::numeric_limits::epsilon(); @@ -317,32 +297,32 @@ BOOST_AUTO_TEST_CASE(Step) { Stepper::Options options(geoCtx, magCtx); options.maxStepSize = stepSize; - MockPropagatorState state(stepper.makeState(options)); - stepper.initialize(state.stepping, cp); - state.stepping.covTransport = false; + auto state = stepper.makeState(options); + stepper.initialize(state, cp); + state.covTransport = false; // ensure step does not result in an error - auto res = stepper.step(state, navigator); + auto res = stepper.step(state, Direction::Backward(), nullptr); BOOST_CHECK(res.ok()); // extract the actual step size auto h = res.value(); - BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), stepSize); - BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), h * navDir); + BOOST_CHECK_EQUAL(state.stepSize.value(), stepSize); + BOOST_CHECK_EQUAL(state.stepSize.value(), h * navDir); // check that the position has moved - auto deltaPos = (stepper.position(state.stepping) - pos).eval(); + auto deltaPos = (stepper.position(state) - pos).eval(); BOOST_CHECK_LT(0, deltaPos.norm()); // check that time has changed - auto deltaTime = stepper.time(state.stepping) - time; + auto deltaTime = stepper.time(state) - time; BOOST_CHECK_LT(0, std::abs(deltaTime)); // check that the direction was rotated - auto projDir = stepper.direction(state.stepping).dot(unitDir); + auto projDir = stepper.direction(state).dot(unitDir); BOOST_CHECK_LT(projDir, 1); // momentum and charge should not change - CHECK_CLOSE_ABS(stepper.absoluteMomentum(state.stepping), absMom, eps); - BOOST_CHECK_EQUAL(stepper.charge(state.stepping), charge); + CHECK_CLOSE_ABS(stepper.absoluteMomentum(state), absMom, eps); + BOOST_CHECK_EQUAL(stepper.charge(state), charge); } // test step method with covariance transport @@ -355,35 +335,35 @@ BOOST_AUTO_TEST_CASE(StepWithCovariance) { Stepper::Options options(geoCtx, magCtx); options.maxStepSize = stepSize; - MockPropagatorState state(stepper.makeState(options)); - stepper.initialize(state.stepping, cp); - state.stepping.covTransport = true; + auto state = stepper.makeState(options); + stepper.initialize(state, cp); + state.covTransport = true; // ensure step does not result in an error - auto res = stepper.step(state, navigator); + auto res = stepper.step(state, Direction::Backward(), nullptr); BOOST_CHECK(res.ok()); // extract the actual step size auto h = res.value(); - BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), stepSize); - BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), h * navDir); + BOOST_CHECK_EQUAL(state.stepSize.value(), stepSize); + BOOST_CHECK_EQUAL(state.stepSize.value(), h * navDir); // check that the position has moved - auto deltaPos = (stepper.position(state.stepping) - pos).eval(); + auto deltaPos = (stepper.position(state) - pos).eval(); BOOST_CHECK_LT(0, deltaPos.norm()); // check that time has changed - auto deltaTime = stepper.time(state.stepping) - time; + auto deltaTime = stepper.time(state) - time; BOOST_CHECK_LT(0, std::abs(deltaTime)); // check that the direction was rotated - auto projDir = stepper.direction(state.stepping).dot(unitDir); + auto projDir = stepper.direction(state).dot(unitDir); BOOST_CHECK_LT(projDir, 1); // momentum and charge should not change - CHECK_CLOSE_ABS(stepper.absoluteMomentum(state.stepping), absMom, eps); - BOOST_CHECK_EQUAL(stepper.charge(state.stepping), charge); + CHECK_CLOSE_ABS(stepper.absoluteMomentum(state), absMom, eps); + BOOST_CHECK_EQUAL(stepper.charge(state), charge); - stepper.transportCovarianceToCurvilinear(state.stepping); - BOOST_CHECK_NE(state.stepping.cov, cov); + stepper.transportCovarianceToCurvilinear(state); + BOOST_CHECK_NE(state.cov, cov); } // test state reset method @@ -396,12 +376,12 @@ BOOST_AUTO_TEST_CASE(Reset) { Stepper::Options options(geoCtx, magCtx); options.maxStepSize = stepSize; - MockPropagatorState state(stepper.makeState(options)); - stepper.initialize(state.stepping, cp); - state.stepping.covTransport = true; + auto state = stepper.makeState(options); + stepper.initialize(state, cp); + state.covTransport = true; // ensure step does not result in an error - stepper.step(state, navigator); + stepper.step(state, Direction::Backward(), nullptr); // Construct the parameters Vector3 newPos(1.5, -2.5, 3.5); @@ -418,7 +398,7 @@ BOOST_AUTO_TEST_CASE(Reset) { auto copyState = [&](auto& field, const auto& other) { using field_t = std::decay_t; std::decay_t copy = stepper.makeState(options); - stepper.initialize(state.stepping, cp); + stepper.initialize(state, cp); copy.state_ready = other.state_ready; copy.useJacobian = other.useJacobian; @@ -452,7 +432,7 @@ BOOST_AUTO_TEST_CASE(Reset) { }; // Reset all possible parameters - Stepper::State stateCopy = copyState(*magneticField, state.stepping); + Stepper::State stateCopy = copyState(*magneticField, state); BOOST_CHECK(cp.covariance().has_value()); stepper.initialize(stateCopy, cp.parameters(), *cp.covariance(), cp.particleHypothesis(), cp.referenceSurface()); @@ -465,12 +445,11 @@ BOOST_AUTO_TEST_CASE(Reset) { freeParams.template segment<3>(eFreeDir0).normalized()); BOOST_CHECK_EQUAL(stepper.absoluteMomentum(stateCopy), std::abs(1. / freeParams[eFreeQOverP])); - BOOST_CHECK_EQUAL(stepper.charge(stateCopy), stepper.charge(state.stepping)); + BOOST_CHECK_EQUAL(stepper.charge(stateCopy), stepper.charge(state)); BOOST_CHECK_EQUAL(stepper.time(stateCopy), freeParams[eFreeTime]); BOOST_CHECK_EQUAL(stateCopy.pathAccumulated, 0.); BOOST_CHECK_EQUAL(stateCopy.stepSize.value(), stepSize); - BOOST_CHECK_EQUAL(stateCopy.previousStepSize, - state.stepping.previousStepSize); + BOOST_CHECK_EQUAL(stateCopy.previousStepSize, state.previousStepSize); // Reset using different surface shapes // 1) Disc surface @@ -488,13 +467,13 @@ BOOST_AUTO_TEST_CASE(Reset) { .value(); // Reset the state and test - Stepper::State stateDisc = copyState(*magneticField, state.stepping); + Stepper::State stateDisc = copyState(*magneticField, state); BOOST_CHECK(boundDisc.covariance().has_value()); stepper.initialize(stateDisc, boundDisc.parameters(), *boundDisc.covariance(), cp.particleHypothesis(), boundDisc.referenceSurface()); CHECK_NE_COLLECTIONS(stateDisc.pVector, stateCopy.pVector); - CHECK_NE_COLLECTIONS(stateDisc.pVector, state.stepping.pVector); + CHECK_NE_COLLECTIONS(stateDisc.pVector, state.pVector); // 2) Perigee surface // Setting some parameters @@ -511,13 +490,13 @@ BOOST_AUTO_TEST_CASE(Reset) { .value(); // Reset the state and test - Stepper::State statePerigee = copyState(*magneticField, state.stepping); + Stepper::State statePerigee = copyState(*magneticField, state); BOOST_CHECK(boundPerigee.covariance().has_value()); stepper.initialize(statePerigee, boundPerigee.parameters(), *boundPerigee.covariance(), cp.particleHypothesis(), boundPerigee.referenceSurface()); CHECK_NE_COLLECTIONS(statePerigee.pVector, stateCopy.pVector); - CHECK_NE_COLLECTIONS(statePerigee.pVector, state.stepping.pVector); + CHECK_NE_COLLECTIONS(statePerigee.pVector, state.pVector); CHECK_NE_COLLECTIONS(statePerigee.pVector, stateDisc.pVector); // 3) Straw surface @@ -529,13 +508,13 @@ BOOST_AUTO_TEST_CASE(Reset) { .value(); // Reset the state and test - Stepper::State stateStraw = copyState(*magneticField, state.stepping); + Stepper::State stateStraw = copyState(*magneticField, state); BOOST_CHECK(boundStraw.covariance().has_value()); stepper.initialize(stateStraw, boundStraw.parameters(), *boundStraw.covariance(), cp.particleHypothesis(), boundStraw.referenceSurface()); CHECK_NE_COLLECTIONS(stateStraw.pVector, stateCopy.pVector); - CHECK_NE_COLLECTIONS(stateStraw.pVector, state.stepping.pVector); + CHECK_NE_COLLECTIONS(stateStraw.pVector, state.pVector); CHECK_NE_COLLECTIONS(stateStraw.pVector, stateDisc.pVector); BOOST_CHECK_EQUAL_COLLECTIONS( stateStraw.pVector.begin(), stateStraw.pVector.end(), diff --git a/Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp b/Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp index a5f81ab5581..e5f856fa66d 100644 --- a/Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp @@ -49,7 +49,6 @@ #include "Acts/Utilities/Result.hpp" #include "Acts/Utilities/UnitVectors.hpp" -#include #include #include #include @@ -73,31 +72,6 @@ static constexpr auto eps = 3 * std::numeric_limits::epsilon(); GeometryContext tgContext = GeometryContext(); MagneticFieldContext mfContext = MagneticFieldContext(); -/// @brief Simplified propagator state -template -struct PropState { - /// @brief Constructor - PropState(Direction direction, stepper_state_t sState) - : stepping(std::move(sState)) { - options.direction = direction; - } - /// State of the eigen stepper - stepper_state_t stepping; - /// Propagator options which only carry the relevant components - struct { - Direction direction = Direction::Forward(); - struct { - double stepTolerance = 1e-4; - double stepSizeCutOff = 0.; - unsigned int maxRungeKuttaStepTrials = 10000; - } stepping; - } options; -}; - -struct MockNavigator {}; - -static constexpr MockNavigator mockNavigator; - /// @brief Aborter for the case that a particle leaves the detector or reaches /// a custom made threshold. /// @@ -304,27 +278,26 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) { // Perform a step without and with covariance transport esState.cov = cov; - PropState ps(navDir, std::move(esState)); - - ps.stepping.covTransport = false; - es.step(ps, mockNavigator).value(); - CHECK_CLOSE_COVARIANCE(ps.stepping.cov, cov, eps); - BOOST_CHECK_NE(es.position(ps.stepping).norm(), newPos.norm()); - BOOST_CHECK_NE(es.direction(ps.stepping), newMom.normalized()); - BOOST_CHECK_EQUAL(es.charge(ps.stepping), charge); - BOOST_CHECK_LT(es.time(ps.stepping), newTime); - BOOST_CHECK_EQUAL(ps.stepping.derivative, FreeVector::Zero()); - BOOST_CHECK_EQUAL(ps.stepping.jacTransport, FreeMatrix::Identity()); - - ps.stepping.covTransport = true; - es.step(ps, mockNavigator).value(); - CHECK_CLOSE_COVARIANCE(ps.stepping.cov, cov, eps); - BOOST_CHECK_NE(es.position(ps.stepping).norm(), newPos.norm()); - BOOST_CHECK_NE(es.direction(ps.stepping), newMom.normalized()); - BOOST_CHECK_EQUAL(es.charge(ps.stepping), charge); - BOOST_CHECK_LT(es.time(ps.stepping), newTime); - BOOST_CHECK_NE(ps.stepping.derivative, FreeVector::Zero()); - BOOST_CHECK_NE(ps.stepping.jacTransport, FreeMatrix::Identity()); + + esState.covTransport = false; + es.step(esState, navDir, nullptr).value(); + CHECK_CLOSE_COVARIANCE(esState.cov, cov, eps); + BOOST_CHECK_NE(es.position(esState).norm(), newPos.norm()); + BOOST_CHECK_NE(es.direction(esState), newMom.normalized()); + BOOST_CHECK_EQUAL(es.charge(esState), charge); + BOOST_CHECK_LT(es.time(esState), newTime); + BOOST_CHECK_EQUAL(esState.derivative, FreeVector::Zero()); + BOOST_CHECK_EQUAL(esState.jacTransport, FreeMatrix::Identity()); + + esState.covTransport = true; + es.step(esState, navDir, nullptr).value(); + CHECK_CLOSE_COVARIANCE(esState.cov, cov, eps); + BOOST_CHECK_NE(es.position(esState).norm(), newPos.norm()); + BOOST_CHECK_NE(es.direction(esState), newMom.normalized()); + BOOST_CHECK_EQUAL(es.charge(esState), charge); + BOOST_CHECK_LT(es.time(esState), newTime); + BOOST_CHECK_NE(esState.derivative, FreeVector::Zero()); + BOOST_CHECK_NE(esState.jacTransport, FreeMatrix::Identity()); /// Test the state reset // Construct the parameters @@ -367,13 +340,13 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) { }; // Reset all possible parameters - EigenStepper<>::State esStateCopy = copyState(*bField, ps.stepping); + EigenStepper<>::State esStateCopy = copyState(*bField, esState); BOOST_CHECK(cp2.covariance().has_value()); es.initialize(esStateCopy, cp2.parameters(), *cp2.covariance(), cp2.particleHypothesis(), cp2.referenceSurface()); // Test all components BOOST_CHECK_NE(esStateCopy.jacToGlobal, BoundToFreeMatrix::Zero()); - BOOST_CHECK_NE(esStateCopy.jacToGlobal, ps.stepping.jacToGlobal); + BOOST_CHECK_NE(esStateCopy.jacToGlobal, esState.jacToGlobal); BOOST_CHECK_EQUAL(esStateCopy.jacTransport, FreeMatrix::Identity()); BOOST_CHECK_EQUAL(esStateCopy.derivative, FreeVector::Zero()); BOOST_CHECK(esStateCopy.covTransport); @@ -384,7 +357,7 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) { freeParams.template segment<3>(eFreeDir0).normalized()); BOOST_CHECK_EQUAL(es.absoluteMomentum(esStateCopy), std::abs(1. / freeParams[eFreeQOverP])); - BOOST_CHECK_EQUAL(es.charge(esStateCopy), -es.charge(ps.stepping)); + BOOST_CHECK_EQUAL(es.charge(esStateCopy), -es.charge(esState)); BOOST_CHECK_EQUAL(es.time(esStateCopy), freeParams[eFreeTime]); BOOST_CHECK_EQUAL(esStateCopy.pathAccumulated, 0.); BOOST_CHECK_EQUAL(esStateCopy.stepSize.value(), stepSize); @@ -461,9 +434,9 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) { CHECK_CLOSE_COVARIANCE(esState.cov, Covariance(2. * cov), eps); // Test a case where no step size adjustment is required - ps.options.stepping.stepTolerance = 2. * 4.4258e+09; + esState.options.stepTolerance = 2. * 4.4258e+09; double h0 = esState.stepSize.value(); - es.step(ps, mockNavigator); + es.step(esState, navDir, nullptr); CHECK_CLOSE_ABS(h0, esState.stepSize.value(), eps); // Produce some errors @@ -472,18 +445,18 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) { EigenStepper<>::Options nesOptions(tgContext, mfContext); EigenStepper<>::State nesState = nes.makeState(nesOptions); nes.initialize(nesState, cp); - PropState nps(navDir, copyState(*nBfield, nesState)); + auto nEsState = copyState(*nBfield, nesState); // Test that we can reach the minimum step size - nps.options.stepping.stepTolerance = 1e-21; - nps.options.stepping.stepSizeCutOff = 1e20; - auto res = nes.step(nps, mockNavigator); + nEsState.options.stepTolerance = 1e-21; + nEsState.options.stepSizeCutOff = 1e20; + auto res = nes.step(nEsState, navDir, nullptr); BOOST_CHECK(!res.ok()); BOOST_CHECK_EQUAL(res.error(), EigenStepperError::StepSizeStalled); // Test that the number of trials exceeds - nps.options.stepping.stepSizeCutOff = 0.; - nps.options.stepping.maxRungeKuttaStepTrials = 0.; - res = nes.step(nps, mockNavigator); + nEsState.options.stepSizeCutOff = 0.; + nEsState.options.maxRungeKuttaStepTrials = 0.; + res = nes.step(nEsState, navDir, nullptr); BOOST_CHECK(!res.ok()); BOOST_CHECK_EQUAL(res.error(), EigenStepperError::StepSizeAdjustmentFailed); } diff --git a/Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp b/Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp index e3d699e6836..8274ecb9062 100644 --- a/Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp @@ -30,7 +30,6 @@ #include "Acts/Surfaces/PlaneSurface.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Utilities/Intersection.hpp" -#include "Acts/Utilities/Logger.hpp" #include #include @@ -75,40 +74,6 @@ const auto defaultNullBField = std::make_shared(); const auto particleHypothesis = ParticleHypothesis::pion(); -struct Options { - Direction direction = defaultNDir; - - const Acts::Logger &logger = Acts::getDummyLogger(); - - struct { - double stepTolerance = 1e-4; - double stepSizeCutOff = 0.0; - std::size_t maxRungeKuttaStepTrials = 10; - } stepping; -}; - -struct MockNavigator {}; - -static constexpr MockNavigator mockNavigator; - -struct Navigation {}; - -template -struct DummyPropState { - stepper_state_t &stepping; - Options options; - Navigation navigation; - GeometryContext geoContext; - - DummyPropState(Direction direction, stepper_state_t &ss) - : stepping(ss), - options(Options{}), - navigation(Navigation{}), - geoContext(geoCtx) { - options.direction = direction; - } -}; - // Makes random bound parameters and covariance and a plane surface at {0,0,0} // with normal {1,0,0}. Optionally some external fixed bound parameters can be // supplied @@ -343,13 +308,12 @@ void test_multi_stepper_vs_eigen_stepper() { // Do some steps and check that the results match for (int i = 0; i < 10; ++i) { // Single stepper - auto single_prop_state = DummyPropState(defaultNDir, single_state); - auto single_result = single_stepper.step(single_prop_state, mockNavigator); + auto single_result = + single_stepper.step(single_state, defaultNDir, nullptr); single_stepper.transportCovarianceToCurvilinear(single_state); // Multi stepper; - auto multi_prop_state = DummyPropState(defaultNDir, multi_state); - auto multi_result = multi_stepper.step(multi_prop_state, mockNavigator); + auto multi_result = multi_stepper.step(multi_state, defaultNDir, nullptr); multi_stepper.transportCovarianceToCurvilinear(multi_state); // Check equality @@ -538,12 +502,10 @@ void test_multi_stepper_surface_status_update() { // Step forward now { - auto multi_prop_state = DummyPropState(Direction::Forward(), multi_state); - multi_stepper.step(multi_prop_state, mockNavigator); + multi_stepper.step(multi_state, Direction::Forward(), nullptr); // Single stepper - auto single_prop_state = DummyPropState(Direction::Forward(), single_state); - single_stepper.step(single_prop_state, mockNavigator); + single_stepper.step(single_state, Direction::Forward(), nullptr); } // Update surface status and check again @@ -641,16 +603,14 @@ void test_component_bound_state() { multi_state, *right_surface, 0, Direction::Forward(), BoundaryTolerance::Infinite(), s_onSurfaceTolerance, ConstrainedStep::Type::Navigator); - auto multi_prop_state = DummyPropState(Direction::Forward(), multi_state); - multi_stepper.step(multi_prop_state, mockNavigator); + multi_stepper.step(multi_state, Direction::Forward(), nullptr); // Single stepper single_stepper.updateSurfaceStatus( single_state, *right_surface, 0, Direction::Forward(), BoundaryTolerance::Infinite(), s_onSurfaceTolerance, ConstrainedStep::Type::Navigator); - auto single_prop_state = DummyPropState(Direction::Forward(), single_state); - single_stepper.step(single_prop_state, mockNavigator); + single_stepper.step(single_state, Direction::Forward(), nullptr); } // Check component-wise bound-state @@ -797,12 +757,10 @@ void test_single_component_interface_function() { multi_stepper.initialize(multi_state, multi_pars); - DummyPropState multi_prop_state(defaultNDir, multi_state); - // Check at least some properties at the moment auto check = [&](auto cmp) { auto sstepper = cmp.singleStepper(multi_stepper); - auto &sstepping = cmp.singleState(multi_prop_state).stepping; + auto &sstepping = cmp.state(); BOOST_CHECK_EQUAL(sstepper.position(sstepping), cmp.pars().template segment<3>(eFreePos0)); diff --git a/Tests/UnitTests/Core/Propagator/StraightLineStepperTests.cpp b/Tests/UnitTests/Core/Propagator/StraightLineStepperTests.cpp index 76086a84070..b6b75602db7 100644 --- a/Tests/UnitTests/Core/Propagator/StraightLineStepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/StraightLineStepperTests.cpp @@ -30,7 +30,6 @@ #include #include #include -#include using Acts::VectorHelpers::makeVector4; @@ -38,25 +37,6 @@ namespace Acts::Test { using Covariance = BoundSquareMatrix; -/// @brief Simplified propagator state -struct PropState { - /// @brief Constructor - PropState(Direction direction, StraightLineStepper::State sState) - : stepping(std::move(sState)) { - options.direction = direction; - } - /// State of the straight line stepper - StraightLineStepper::State stepping; - /// Propagator options which only carry the particle's mass - struct { - Direction direction = Direction::Forward(); - } options; -}; - -struct MockNavigator {}; - -static constexpr MockNavigator mockNavigator; - static constexpr auto eps = 2 * std::numeric_limits::epsilon(); /// These tests are aiming to test whether the state setup is working properly @@ -193,33 +173,32 @@ BOOST_AUTO_TEST_CASE(straight_line_stepper_test) { // Perform a step without and with covariance transport slsState.cov = cov; - PropState ps(navDir, slsState); - - ps.stepping.covTransport = false; - double h = sls.step(ps, mockNavigator).value(); - BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), stepSize); - BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), h * navDir); - CHECK_CLOSE_COVARIANCE(ps.stepping.cov, cov, 1e-6); - BOOST_CHECK_GT(sls.position(ps.stepping).norm(), newPos.norm()); - CHECK_CLOSE_ABS(sls.direction(ps.stepping), newMom.normalized(), 1e-6); - CHECK_CLOSE_ABS(sls.absoluteMomentum(ps.stepping), newMom.norm(), 1e-6); - CHECK_CLOSE_ABS(sls.charge(ps.stepping), charge, 1e-6); - BOOST_CHECK_LT(sls.time(ps.stepping), newTime); - BOOST_CHECK_EQUAL(ps.stepping.derivative, FreeVector::Zero()); - BOOST_CHECK_EQUAL(ps.stepping.jacTransport, FreeMatrix::Identity()); - - ps.stepping.covTransport = true; - double h2 = sls.step(ps, mockNavigator).value(); - BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), stepSize); + + slsState.covTransport = false; + double h = sls.step(slsState, navDir, nullptr).value(); + BOOST_CHECK_EQUAL(slsState.stepSize.value(), stepSize); + BOOST_CHECK_EQUAL(slsState.stepSize.value(), h * navDir); + CHECK_CLOSE_COVARIANCE(slsState.cov, cov, 1e-6); + BOOST_CHECK_GT(sls.position(slsState).norm(), newPos.norm()); + CHECK_CLOSE_ABS(sls.direction(slsState), newMom.normalized(), 1e-6); + CHECK_CLOSE_ABS(sls.absoluteMomentum(slsState), newMom.norm(), 1e-6); + CHECK_CLOSE_ABS(sls.charge(slsState), charge, 1e-6); + BOOST_CHECK_LT(sls.time(slsState), newTime); + BOOST_CHECK_EQUAL(slsState.derivative, FreeVector::Zero()); + BOOST_CHECK_EQUAL(slsState.jacTransport, FreeMatrix::Identity()); + + slsState.covTransport = true; + double h2 = sls.step(slsState, navDir, nullptr).value(); + BOOST_CHECK_EQUAL(slsState.stepSize.value(), stepSize); BOOST_CHECK_EQUAL(h2, h); - CHECK_CLOSE_COVARIANCE(ps.stepping.cov, cov, 1e-6); - BOOST_CHECK_GT(sls.position(ps.stepping).norm(), newPos.norm()); - CHECK_CLOSE_ABS(sls.direction(ps.stepping), newMom.normalized(), 1e-6); - CHECK_CLOSE_ABS(sls.absoluteMomentum(ps.stepping), newMom.norm(), 1e-6); - CHECK_CLOSE_ABS(sls.charge(ps.stepping), charge, 1e-6); - BOOST_CHECK_LT(sls.time(ps.stepping), newTime); - BOOST_CHECK_NE(ps.stepping.derivative, FreeVector::Zero()); - BOOST_CHECK_NE(ps.stepping.jacTransport, FreeMatrix::Identity()); + CHECK_CLOSE_COVARIANCE(slsState.cov, cov, 1e-6); + BOOST_CHECK_GT(sls.position(slsState).norm(), newPos.norm()); + CHECK_CLOSE_ABS(sls.direction(slsState), newMom.normalized(), 1e-6); + CHECK_CLOSE_ABS(sls.absoluteMomentum(slsState), newMom.norm(), 1e-6); + CHECK_CLOSE_ABS(sls.charge(slsState), charge, 1e-6); + BOOST_CHECK_LT(sls.time(slsState), newTime); + BOOST_CHECK_NE(slsState.derivative, FreeVector::Zero()); + BOOST_CHECK_NE(slsState.jacTransport, FreeMatrix::Identity()); /// Test the state reset // Construct the parameters @@ -238,12 +217,12 @@ BOOST_AUTO_TEST_CASE(straight_line_stepper_test) { navDir = Direction::Forward(); // Reset all possible parameters - StraightLineStepper::State slsStateCopy = ps.stepping; + StraightLineStepper::State slsStateCopy = slsState; sls.initialize(slsStateCopy, cp2.parameters(), cp2.covariance(), cp2.particleHypothesis(), cp2.referenceSurface()); // Test all components BOOST_CHECK_NE(slsStateCopy.jacToGlobal, BoundToFreeMatrix::Zero()); - BOOST_CHECK_NE(slsStateCopy.jacToGlobal, ps.stepping.jacToGlobal); + BOOST_CHECK_NE(slsStateCopy.jacToGlobal, slsState.jacToGlobal); BOOST_CHECK_EQUAL(slsStateCopy.jacTransport, FreeMatrix::Identity()); BOOST_CHECK_EQUAL(slsStateCopy.derivative, FreeVector::Zero()); BOOST_CHECK(slsStateCopy.covTransport); @@ -254,7 +233,7 @@ BOOST_AUTO_TEST_CASE(straight_line_stepper_test) { freeParams.template segment<3>(eFreeDir0).normalized(), 1e-6); CHECK_CLOSE_ABS(sls.absoluteMomentum(slsStateCopy), std::abs(1. / freeParams[eFreeQOverP]), 1e-6); - CHECK_CLOSE_ABS(sls.charge(slsStateCopy), -sls.charge(ps.stepping), 1e-6); + CHECK_CLOSE_ABS(sls.charge(slsStateCopy), -sls.charge(slsState), 1e-6); CHECK_CLOSE_ABS(sls.time(slsStateCopy), freeParams[eFreeTime], 1e-6); BOOST_CHECK_EQUAL(slsStateCopy.pathAccumulated, 0.); BOOST_CHECK_EQUAL(slsStateCopy.stepSize.value(), stepSize); diff --git a/Tests/UnitTests/Core/Propagator/SympyStepperTests.cpp b/Tests/UnitTests/Core/Propagator/SympyStepperTests.cpp index 70322ef1f71..0ce81de6bee 100644 --- a/Tests/UnitTests/Core/Propagator/SympyStepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/SympyStepperTests.cpp @@ -30,7 +30,6 @@ #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/Result.hpp" -#include #include #include #include @@ -53,31 +52,6 @@ static constexpr auto eps = 3 * std::numeric_limits::epsilon(); GeometryContext tgContext = GeometryContext(); MagneticFieldContext mfContext = MagneticFieldContext(); -/// @brief Simplified propagator state -template -struct PropState { - /// @brief Constructor - PropState(Direction direction, stepper_state_t sState) - : stepping(std::move(sState)) { - options.direction = direction; - } - /// State of the sympy stepper - stepper_state_t stepping; - /// Propagator options which only carry the relevant components - struct { - Direction direction = Direction::Forward(); - struct { - double stepTolerance = 1e-4; - double stepSizeCutOff = 0.; - unsigned int maxRungeKuttaStepTrials = 10000; - } stepping; - } options; -}; - -struct MockNavigator {}; - -static constexpr MockNavigator mockNavigator; - /// @brief Aborter for the case that a particle leaves the detector or reaches /// a custom made threshold. /// @@ -279,27 +253,26 @@ BOOST_AUTO_TEST_CASE(sympy_stepper_test) { // Perform a step without and with covariance transport esState.cov = cov; - PropState ps(navDir, std::move(esState)); - - ps.stepping.covTransport = false; - es.step(ps, mockNavigator).value(); - CHECK_CLOSE_COVARIANCE(ps.stepping.cov, cov, eps); - BOOST_CHECK_NE(es.position(ps.stepping).norm(), newPos.norm()); - BOOST_CHECK_NE(es.direction(ps.stepping), newMom.normalized()); - BOOST_CHECK_EQUAL(es.charge(ps.stepping), charge); - BOOST_CHECK_LT(es.time(ps.stepping), newTime); - BOOST_CHECK_EQUAL(ps.stepping.derivative, FreeVector::Zero()); - BOOST_CHECK_EQUAL(ps.stepping.jacTransport, FreeMatrix::Identity()); - - ps.stepping.covTransport = true; - es.step(ps, mockNavigator).value(); - CHECK_CLOSE_COVARIANCE(ps.stepping.cov, cov, eps); - BOOST_CHECK_NE(es.position(ps.stepping).norm(), newPos.norm()); - BOOST_CHECK_NE(es.direction(ps.stepping), newMom.normalized()); - BOOST_CHECK_EQUAL(es.charge(ps.stepping), charge); - BOOST_CHECK_LT(es.time(ps.stepping), newTime); - BOOST_CHECK_NE(ps.stepping.derivative, FreeVector::Zero()); - BOOST_CHECK_NE(ps.stepping.jacTransport, FreeMatrix::Identity()); + + esState.covTransport = false; + es.step(esState, navDir, nullptr).value(); + CHECK_CLOSE_COVARIANCE(esState.cov, cov, eps); + BOOST_CHECK_NE(es.position(esState).norm(), newPos.norm()); + BOOST_CHECK_NE(es.direction(esState), newMom.normalized()); + BOOST_CHECK_EQUAL(es.charge(esState), charge); + BOOST_CHECK_LT(es.time(esState), newTime); + BOOST_CHECK_EQUAL(esState.derivative, FreeVector::Zero()); + BOOST_CHECK_EQUAL(esState.jacTransport, FreeMatrix::Identity()); + + esState.covTransport = true; + es.step(esState, navDir, nullptr).value(); + CHECK_CLOSE_COVARIANCE(esState.cov, cov, eps); + BOOST_CHECK_NE(es.position(esState).norm(), newPos.norm()); + BOOST_CHECK_NE(es.direction(esState), newMom.normalized()); + BOOST_CHECK_EQUAL(es.charge(esState), charge); + BOOST_CHECK_LT(es.time(esState), newTime); + BOOST_CHECK_NE(esState.derivative, FreeVector::Zero()); + BOOST_CHECK_NE(esState.jacTransport, FreeMatrix::Identity()); /// Test the state reset // Construct the parameters @@ -339,13 +312,13 @@ BOOST_AUTO_TEST_CASE(sympy_stepper_test) { }; // Reset all possible parameters - SympyStepper::State esStateCopy = copyState(*bField, ps.stepping); + SympyStepper::State esStateCopy = copyState(*bField, esState); BOOST_CHECK(cp2.covariance().has_value()); es.initialize(esStateCopy, cp2.parameters(), *cp2.covariance(), cp2.particleHypothesis(), cp2.referenceSurface()); // Test all components BOOST_CHECK_NE(esStateCopy.jacToGlobal, BoundToFreeMatrix::Zero()); - BOOST_CHECK_NE(esStateCopy.jacToGlobal, ps.stepping.jacToGlobal); + BOOST_CHECK_NE(esStateCopy.jacToGlobal, esState.jacToGlobal); BOOST_CHECK_EQUAL(esStateCopy.jacTransport, FreeMatrix::Identity()); BOOST_CHECK_EQUAL(esStateCopy.derivative, FreeVector::Zero()); BOOST_CHECK(esStateCopy.covTransport); @@ -356,7 +329,7 @@ BOOST_AUTO_TEST_CASE(sympy_stepper_test) { freeParams.template segment<3>(eFreeDir0).normalized()); BOOST_CHECK_EQUAL(es.absoluteMomentum(esStateCopy), std::abs(1. / freeParams[eFreeQOverP])); - BOOST_CHECK_EQUAL(es.charge(esStateCopy), -es.charge(ps.stepping)); + BOOST_CHECK_EQUAL(es.charge(esStateCopy), -es.charge(esState)); BOOST_CHECK_EQUAL(es.time(esStateCopy), freeParams[eFreeTime]); BOOST_CHECK_EQUAL(esStateCopy.pathAccumulated, 0.); BOOST_CHECK_EQUAL(esStateCopy.stepSize.value(), stepSize); @@ -435,9 +408,9 @@ BOOST_AUTO_TEST_CASE(sympy_stepper_test) { CHECK_CLOSE_COVARIANCE(esState.cov, Covariance(2. * cov), eps); // Test a case where no step size adjustment is required - ps.options.stepping.stepTolerance = 2. * 4.4258e+09; + esState.options.stepTolerance = 2. * 4.4258e+09; double h0 = esState.stepSize.value(); - es.step(ps, mockNavigator); + es.step(esState, Direction::Forward(), nullptr); CHECK_CLOSE_ABS(h0, esState.stepSize.value(), eps); } From 7b2b9360dfdac717726cd88a66d1d000c0aa726e Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 12 Feb 2025 09:31:29 +0100 Subject: [PATCH 3/9] refactor: `ProtoLayer` can (optionally) hold mutable surfaces (#4030) In the Gen3 building context, using mutable pointers makes more sense since we store as much as possible internally as mutable. This is implemented in a API backwards compatible way, because the Gen1 building assumes the opposite. PLEASE DESCRIBE YOUR CHANGES. THIS MESSAGE ENDS UP AS THE COMMIT MESSAGE. DO NOT USE @-MENTIONS HERE! --- Core/include/Acts/Geometry/ProtoLayer.hpp | 187 ++++++++++++------ Core/src/Geometry/ProtoLayer.cpp | 86 +++----- .../Core/Geometry/ProtoLayerTests.cpp | 35 ++-- 3 files changed, 180 insertions(+), 128 deletions(-) diff --git a/Core/include/Acts/Geometry/ProtoLayer.hpp b/Core/include/Acts/Geometry/ProtoLayer.hpp index 93ea871eb55..2a9356167f5 100644 --- a/Core/include/Acts/Geometry/ProtoLayer.hpp +++ b/Core/include/Acts/Geometry/ProtoLayer.hpp @@ -20,13 +20,13 @@ namespace Acts { -/// @struct ProtoLayer -/// -/// Encapsulates min/max boundaries that will be turned into a layer. -/// The struct allows this information to be obtained in a consistent -/// way, or be caller provided. +namespace detail { -struct ProtoLayer { +/// @class ProtoLayerBase +/// +/// Base class containing common functionality for ProtoLayer implementations +/// @note This will go away once we remove the Gen1 geometry which assumes this only takes const pointers. +struct ProtoLayerBase { public: /// The extent of the ProtoLayer Extent extent; @@ -37,6 +37,52 @@ struct ProtoLayer { /// The local transform Transform3 transform = Transform3::Identity(); + /// Get the parameters : min + /// @param aDir The accessed axis direction + /// @param addenv The steering if enevlope is added or not + double min(AxisDirection aDir, bool addenv = true) const; + + // Get the parameters : max + /// @param aDir The accessed axis direction + /// @param addenv The steering if enevlope is added or not + double max(AxisDirection aDir, bool addenv = true) const; + + // Get the parameters : medium + /// @param aDir The accessed axis direction + /// @param addenv The steering if enevlope is added or not + double medium(AxisDirection aDir, bool addenv = true) const; + + // Get the parameters : range + /// @param aDir The accessed axis direction + /// @param addenv The steering if enevlope is added or not + double range(AxisDirection aDir, bool addenv = true) const; + + /// Output to ostream + /// @param sl the input ostream + std::ostream& toStream(std::ostream& sl) const; + + protected: + /// Helper method which performs the actual min/max calculation + /// + /// @param gctx The current geometry context object, e.g. alignment + /// @param surfaces The surfaces to build this protolayer out of + /// @param extent The extent to modify + /// @param transform The transform to use + static void measureImpl(const GeometryContext& gctx, + const std::vector& surfaces, + Extent& extent, const Transform3& transform); +}; + +/// @struct ProtoLayerT +/// +/// Encapsulates min/max boundaries that will be turned into a layer. +/// The struct allows this information to be obtained in a consistent +/// way, or be caller provided. +template +struct ProtoLayerT : public ProtoLayerBase { + using SurfacePtr = std::conditional_t; + using SurfaceType = std::conditional_t; + /// Constructor /// /// Loops over a provided vector of surface and calculates the various @@ -46,9 +92,22 @@ struct ProtoLayer { /// @param gctx The current geometry context object, e.g. alignment /// @param surfaces The vector of surfaces to consider /// @param transformIn The local transform to evaluate the sizing in - ProtoLayer(const GeometryContext& gctx, - const std::vector& surfaces, - const Transform3& transformIn = Transform3::Identity()); + ProtoLayerT(const GeometryContext& gctx, + const std::vector& surfaces, + const Transform3& transformIn = Transform3::Identity()) + : m_surfaces(surfaces) { + transform = transformIn; + std::vector constSurfaces; + if constexpr (!IsConst) { + constSurfaces.reserve(surfaces.size()); + for (auto* sf : surfaces) { + constSurfaces.push_back(sf); + } + measureImpl(gctx, constSurfaces, extent, transform); + } else { + measureImpl(gctx, surfaces, extent, transform); + } + } /// Constructor /// @@ -59,79 +118,89 @@ struct ProtoLayer { /// @param gctx The current geometry context object, e.g. alignment /// @param surfaces The vector of surfaces to consider /// @param transformIn The local transform to evaluate the sizing in - ProtoLayer(const GeometryContext& gctx, - const std::vector>& surfaces, - const Transform3& transformIn = Transform3::Identity()); + ProtoLayerT(const GeometryContext& gctx, + const std::vector>& surfaces, + const Transform3& transformIn = Transform3::Identity()) { + transform = transformIn; + m_surfaces.reserve(surfaces.size()); + for (const auto& sf : surfaces) { + m_surfaces.push_back(sf.get()); + } + std::vector constSurfaces; + if constexpr (!IsConst) { + constSurfaces.reserve(surfaces.size()); + for (auto* sf : m_surfaces) { + constSurfaces.push_back(sf); + } + measureImpl(gctx, constSurfaces, extent, transform); + } else { + measureImpl(gctx, m_surfaces, extent, transform); + } + } - /// Constructor - /// - /// Loops over a provided vector of surface and calculates the various - /// min/max values in one go. Also takes into account the thickness - /// of an associated DetectorElement, if it exists. + /// Constructor that accepts non-const shared pointers even when IsConst is + /// true /// /// @param gctx The current geometry context object, e.g. alignment /// @param surfaces The vector of surfaces to consider /// @param transformIn The local transform to evaluate the sizing in - ProtoLayer(const GeometryContext& gctx, - const std::vector>& surfaces, - const Transform3& transformIn = Transform3::Identity()); - - ProtoLayer() = default; - - /// Get the parameters : min - /// @param aDir The accessed axis direction - /// @param addenv The steering if enevlope is added or not - double min(AxisDirection aDir, bool addenv = true) const; - - // Get the parameters : max - /// @param aDir The accessed axis direction - /// @param addenv The steering if enevlope is added or not - double max(AxisDirection aDir, bool addenv = true) const; - - // Get the parameters : max - /// @param aDir The accessed axis direction - /// @param addenv The steering if enevlope is added or not - double medium(AxisDirection aDir, bool addenv = true) const; - - // Get the parameters : max - /// @param aDir The accessed axis direction - /// @param addenv The steering if enevlope is added or not - double range(AxisDirection aDir, bool addenv = true) const; + ProtoLayerT(const GeometryContext& gctx, + const std::vector>& surfaces, + const Transform3& transformIn = Transform3::Identity()) + requires IsConst + { + transform = transformIn; + m_surfaces.reserve(surfaces.size()); + for (const auto& sf : surfaces) { + m_surfaces.push_back(sf.get()); + } + measureImpl(gctx, m_surfaces, extent, transform); + } - /// Output to ostream - /// @param sl the input ostream - std::ostream& toStream(std::ostream& sl) const; + ProtoLayerT() = default; /// Output stream operator /// @param sl the input ostream /// @param pl the ProtoLayer to be printed /// @return the output ostream - friend std::ostream& operator<<(std::ostream& sl, const ProtoLayer& pl) { + friend std::ostream& operator<<(std::ostream& sl, const ProtoLayerT& pl) { return pl.toStream(sl); } /// Give access to the surfaces used/assigned to the ProtoLayer - const std::vector& surfaces() const; + const std::vector& surfaces() const { return m_surfaces; } /// Add a surface, this will also increase the extent /// @param gctx The current geometry context object, e.g. alignment /// @param surface The surface which is added to the ProtoLayer - void add(const GeometryContext& gctx, const Surface& surface); + void add(const GeometryContext& gctx, SurfaceType& surface) { + m_surfaces.push_back(&surface); + std::vector constSurfaces; + if constexpr (!IsConst) { + constSurfaces.reserve(m_surfaces.size()); + for (auto* sf : m_surfaces) { + constSurfaces.push_back(sf); + } + measureImpl(gctx, constSurfaces, extent, transform); + } else { + measureImpl(gctx, m_surfaces, extent, transform); + } + } private: - /// Helper method which performs the actual min/max calculation - /// - /// @param gctx The current geometry context object, e.g. alignment - /// @param surfaces The surfaces to build this protolayer out of - void measure(const GeometryContext& gctx, - const std::vector& surfaces); - /// Store the list of surfaces used for this proto layer - std::vector m_surfaces = {}; + std::vector m_surfaces = {}; }; -inline const std::vector& ProtoLayer::surfaces() const { - return m_surfaces; -} +} // namespace detail + +// Forward-declaration friendly class for backward compatibility +struct ProtoLayer : public detail::ProtoLayerT { + using detail::ProtoLayerT::ProtoLayerT; +}; + +struct MutableProtoLayer : public detail::ProtoLayerT { + using detail::ProtoLayerT::ProtoLayerT; +}; } // namespace Acts diff --git a/Core/src/Geometry/ProtoLayer.cpp b/Core/src/Geometry/ProtoLayer.cpp index 2956e6a2788..6ba568f1434 100644 --- a/Core/src/Geometry/ProtoLayer.cpp +++ b/Core/src/Geometry/ProtoLayer.cpp @@ -16,89 +16,59 @@ using Acts::VectorHelpers::perp; using Acts::VectorHelpers::phi; -namespace Acts { +namespace Acts::detail { -ProtoLayer::ProtoLayer(const GeometryContext& gctx, - const std::vector& surfaces, - const Transform3& transformIn) - : transform(transformIn), m_surfaces(surfaces) { - measure(gctx, surfaces); -} - -ProtoLayer::ProtoLayer( - const GeometryContext& gctx, - const std::vector>& surfaces, - const Transform3& transformIn) - : transform(transformIn), m_surfaces(unpack_shared_vector(surfaces)) { - measure(gctx, m_surfaces); -} - -ProtoLayer::ProtoLayer(const GeometryContext& gctx, - const std::vector>& surfaces, - const Transform3& transformIn) - : transform(transformIn) { - m_surfaces.reserve(surfaces.size()); +void ProtoLayerBase::measureImpl(const GeometryContext& gctx, + const std::vector& surfaces, + Extent& extent, const Transform3& transform) { for (const auto& sf : surfaces) { - m_surfaces.push_back(sf.get()); + // To prevent problematic isInsidePolygon check for straw surfaces with only + // one lseg + int lseg = (sf->type() != Surface::Straw) ? 1 : 2; + auto sfPolyhedron = sf->polyhedronRepresentation(gctx, lseg); + const DetectorElementBase* element = sf->associatedDetectorElement(); + const auto* regSurface = dynamic_cast(sf); + if (element != nullptr && regSurface != nullptr) { + // Take the thickness in account if necessary + double thickness = element->thickness(); + // We need a translation along and opposite half thickness + Vector3 sfNormal = regSurface->normal(gctx, sf->center(gctx)); + for (const auto& dT : {-0.5 * thickness, 0.5 * thickness}) { + Transform3 dtransform = transform * Translation3{dT * sfNormal}; + extent.extend(sfPolyhedron.extent(dtransform)); + } + continue; + } + extent.extend(sfPolyhedron.extent(transform)); } - measure(gctx, m_surfaces); } -double ProtoLayer::min(AxisDirection aDir, bool addenv) const { +double ProtoLayerBase::min(AxisDirection aDir, bool addenv) const { if (addenv) { return extent.min(aDir) - envelope[aDir][0u]; } return extent.min(aDir); } -double ProtoLayer::max(AxisDirection aDir, bool addenv) const { +double ProtoLayerBase::max(AxisDirection aDir, bool addenv) const { if (addenv) { return extent.max(aDir) + envelope[aDir][1u]; } return extent.max(aDir); } -double ProtoLayer::medium(AxisDirection aDir, bool addenv) const { +double ProtoLayerBase::medium(AxisDirection aDir, bool addenv) const { return 0.5 * (min(aDir, addenv) + max(aDir, addenv)); } -double ProtoLayer::range(AxisDirection aDir, bool addenv) const { +double ProtoLayerBase::range(AxisDirection aDir, bool addenv) const { return std::abs(max(aDir, addenv) - min(aDir, addenv)); } -std::ostream& ProtoLayer::toStream(std::ostream& sl) const { +std::ostream& ProtoLayerBase::toStream(std::ostream& sl) const { sl << "ProtoLayer with dimensions (min/max)" << std::endl; sl << extent.toString(); return sl; } -void ProtoLayer::measure(const GeometryContext& gctx, - const std::vector& surfaces) { - for (const auto& sf : surfaces) { - // To prevent problematic isInsidePolygon check for straw surfaces with only - // one lseg - int lseg = (sf->type() != Surface::Straw) ? 1 : 2; - auto sfPolyhedron = sf->polyhedronRepresentation(gctx, lseg); - const DetectorElementBase* element = sf->associatedDetectorElement(); - const auto* regSurface = dynamic_cast(sf); - if (element != nullptr && regSurface != nullptr) { - // Take the thickness in account if necessary - double thickness = element->thickness(); - // We need a translation along and opposite half thickness - Vector3 sfNormal = regSurface->normal(gctx, sf->center(gctx)); - for (const auto& dT : {-0.5 * thickness, 0.5 * thickness}) { - Transform3 dtransform = transform * Translation3{dT * sfNormal}; - extent.extend(sfPolyhedron.extent(dtransform)); - } - continue; - } - extent.extend(sfPolyhedron.extent(transform)); - } -} - -void ProtoLayer::add(const GeometryContext& gctx, const Surface& surface) { - m_surfaces.push_back(&surface); - measure(gctx, m_surfaces); -} - -} // namespace Acts +} // namespace Acts::detail diff --git a/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp b/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp index a0cc315b4b1..88fd5fbaeb5 100644 --- a/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp +++ b/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp @@ -35,8 +35,12 @@ GeometryContext tgContext = GeometryContext(); BOOST_AUTO_TEST_SUITE(Geometry) -BOOST_AUTO_TEST_CASE(ProtoLayerTests) { +// Test both const and non-const versions +template +void testProtoLayer() { using enum AxisDirection; + using TestProtoLayer = detail::ProtoLayerT; + using SurfacePtr = typename TestProtoLayer::SurfacePtr; // Create a proto layer with 4 surfaces on the x/y grid auto recBounds = std::make_shared(3., 6.); @@ -51,11 +55,13 @@ BOOST_AUTO_TEST_CASE(ProtoLayerTests) { AngleAxis3(-std::numbers::pi / 2., Vector3::UnitZ()) * Transform3::Identity(); - std::vector> surfaceStore; + std::vector< + std::shared_ptr>> + surfaceStore; surfaceStore.reserve(100); auto createProtoLayer = [&](const Transform3& trf, - bool shared = false) -> ProtoLayer { + bool shared = false) -> TestProtoLayer { auto atNegX = Surface::makeShared( Transform3(trf * Translation3(Vector3(-3., 0., 0.)) * planeYZ), recBounds); @@ -72,17 +78,18 @@ BOOST_AUTO_TEST_CASE(ProtoLayerTests) { Transform3(trf * Translation3(Vector3(0., 3., 0.)) * planeZX), recBounds); - std::vector> sharedSurfaces = { - atNegX, atNegY, atPosX, atPosY}; + std::vector< + std::shared_ptr>> + sharedSurfaces = {atNegX, atNegY, atPosX, atPosY}; surfaceStore.insert(surfaceStore.begin(), sharedSurfaces.begin(), sharedSurfaces.end()); if (!shared) { - std::vector surfaces = {atNegX.get(), atNegY.get(), - atPosX.get(), atPosY.get()}; + std::vector surfaces = {atNegX.get(), atNegY.get(), + atPosX.get(), atPosY.get()}; - return ProtoLayer(tgContext, surfaces); + return TestProtoLayer(tgContext, surfaces); } - return ProtoLayer(tgContext, sharedSurfaces); + return TestProtoLayer(tgContext, sharedSurfaces); }; // Test 0 - check constructor with surfaces and shared surfaces @@ -126,8 +133,6 @@ BOOST_AUTO_TEST_CASE(ProtoLayerTests) { CHECK_CLOSE_ABS(protoLayer.max(AxisR), std::hypot(3, 6), 1e-8); CHECK_CLOSE_ABS(protoLayer.min(AxisR), 3., 1e-8); - // Test 1a - // Test 2 - rotate around Z-Axis, should leave R, Z untouched, // only preserves medium values auto protoLayerRot = createProtoLayer(AngleAxis3(-0.345, Vector3::UnitZ()) * @@ -160,6 +165,14 @@ Extent in space : BOOST_CHECK_EQUAL(sstream.str(), oString); } +BOOST_AUTO_TEST_CASE(ProtoLayerTests) { + testProtoLayer(); // Test const version +} + +BOOST_AUTO_TEST_CASE(ProtoLayerTestsNonConst) { + testProtoLayer(); // Test non-const version +} + BOOST_AUTO_TEST_CASE(OrientedLayer) { using enum AxisDirection; using namespace Acts::UnitLiterals; From 36cc0cb978e6e797f840ba2560bd18cd2daa92fc Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 12 Feb 2025 11:07:38 +0100 Subject: [PATCH 4/9] chore: readability-redundant-smartptr-get (#4064) First without the necessary code changes so I can see if #4062 (which this is based on) works correctly. --- .clang-tidy | 1 + CI/clang_tidy/limits.yml | 1 + Core/include/Acts/Geometry/BoundarySurfaceT.hpp | 2 +- Core/src/Detector/DetectorVolume.cpp | 4 ++-- Core/src/Detector/Portal.cpp | 4 ++-- Core/src/Material/MaterialMapper.cpp | 4 ++-- Core/src/Surfaces/ConeSurface.cpp | 2 +- Core/src/Surfaces/CylinderSurface.cpp | 2 +- Core/src/Surfaces/DiscSurface.cpp | 2 +- Core/src/Surfaces/LineSurface.cpp | 2 +- Core/src/Surfaces/PlaneSurface.cpp | 2 +- Examples/Algorithms/Geant4/src/Geant4Simulation.cpp | 2 +- .../ContextualDetector/src/ExternalAlignmentDecorator.cpp | 2 +- .../ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp | 6 +++--- Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp | 5 ++--- Examples/Io/Json/src/JsonSurfacesWriter.cpp | 2 +- Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp | 2 +- Plugins/Geant4/src/Geant4Converters.cpp | 8 ++++---- .../Acts/Plugins/GeoModel/GeoModelBlueprintCreater.hpp | 2 +- .../Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp | 2 +- Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp | 2 +- Plugins/Json/src/SurfaceJsonConverter.cpp | 4 ++-- Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp | 2 +- Tests/UnitTests/Core/TrackFitting/FitterTestsCommon.hpp | 1 + .../Plugins/Geant4/Geant4DetectorElementTests.cpp | 4 ++-- Tests/UnitTests/Plugins/TGeo/TGeoParserTests.cpp | 4 ++-- 26 files changed, 38 insertions(+), 36 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index f761f7f3c47..8411466091e 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -21,6 +21,7 @@ Checks: > readability-inconsistent-declaration-parameter-name, readability-named-parameter, readability-operators-representation, + readability-redundant-smartptr-get HeaderFilterRegex: '.*(? inline const RegularSurface& BoundarySurfaceT::surfaceRepresentation() const { - return (*(m_surface.get())); + return *m_surface; } template diff --git a/Core/src/Detector/DetectorVolume.cpp b/Core/src/Detector/DetectorVolume.cpp index eb74a2b8b57..199a4d88d76 100644 --- a/Core/src/Detector/DetectorVolume.cpp +++ b/Core/src/Detector/DetectorVolume.cpp @@ -100,7 +100,7 @@ Acts::Vector3 Acts::Experimental::DetectorVolume::center( const Acts::VolumeBounds& Acts::Experimental::DetectorVolume::volumeBounds() const { - return (*m_bounds.get()); + return *m_bounds; } std::vector>& @@ -195,7 +195,7 @@ void Acts::Experimental::DetectorVolume::construct( const GeometryContext& gctx, const PortalGenerator& portalGenerator) { // Create portals with the given generator auto portalSurfaces = - portalGenerator(transform(gctx), *(m_bounds.get()), getSharedPtr()); + portalGenerator(transform(gctx), *m_bounds, getSharedPtr()); m_portals = ObjectStore>(portalSurfaces); createBoundingBox(gctx); } diff --git a/Core/src/Detector/Portal.cpp b/Core/src/Detector/Portal.cpp index e3caa8c7c06..f5340093c40 100644 --- a/Core/src/Detector/Portal.cpp +++ b/Core/src/Detector/Portal.cpp @@ -26,11 +26,11 @@ Portal::Portal(std::shared_ptr surface) } const Acts::RegularSurface& Portal::surface() const { - return *m_surface.get(); + return *m_surface; } Acts::RegularSurface& Portal::surface() { - return *m_surface.get(); + return *m_surface; } const std::array& diff --git a/Core/src/Material/MaterialMapper.cpp b/Core/src/Material/MaterialMapper.cpp index bd635bfd608..2797d7e5f93 100644 --- a/Core/src/Material/MaterialMapper.cpp +++ b/Core/src/Material/MaterialMapper.cpp @@ -63,7 +63,7 @@ Acts::MaterialMapper::mapMaterial(State& state, const GeometryContext& gctx, // The material interactions m_cfg.surfaceMaterialAccumulater->accumulate( - *state.surfaceMaterialAccumulaterState.get(), assigned, emptyBinSurfaces); + *state.surfaceMaterialAccumulaterState, assigned, emptyBinSurfaces); // The function to calculate the total material before returning auto calculateTotalMaterial = [](RecordedMaterialTrack& rTrack) -> void { @@ -86,7 +86,7 @@ Acts::MaterialMapper::DetectorMaterialMaps Acts::MaterialMapper::finalizeMaps( // The surface maps detectorMaterialMaps.first = m_cfg.surfaceMaterialAccumulater->finalizeMaterial( - *state.surfaceMaterialAccumulaterState.get()); + *state.surfaceMaterialAccumulaterState); return detectorMaterialMaps; } diff --git a/Core/src/Surfaces/ConeSurface.cpp b/Core/src/Surfaces/ConeSurface.cpp index 1b9c1906856..39a0d8ac9e8 100644 --- a/Core/src/Surfaces/ConeSurface.cpp +++ b/Core/src/Surfaces/ConeSurface.cpp @@ -177,7 +177,7 @@ Vector3 ConeSurface::normal(const GeometryContext& gctx, const ConeBounds& ConeSurface::bounds() const { // is safe because no constructor w/o bounds exists - return (*m_bounds.get()); + return *m_bounds; } Polyhedron ConeSurface::polyhedronRepresentation( diff --git a/Core/src/Surfaces/CylinderSurface.cpp b/Core/src/Surfaces/CylinderSurface.cpp index 45ec85ae1a8..7c78a2e50ae 100644 --- a/Core/src/Surfaces/CylinderSurface.cpp +++ b/Core/src/Surfaces/CylinderSurface.cpp @@ -177,7 +177,7 @@ double CylinderSurface::pathCorrection(const GeometryContext& gctx, } const CylinderBounds& CylinderSurface::bounds() const { - return (*m_bounds.get()); + return *m_bounds; } Polyhedron CylinderSurface::polyhedronRepresentation( diff --git a/Core/src/Surfaces/DiscSurface.cpp b/Core/src/Surfaces/DiscSurface.cpp index 64369c5dcc9..42d566336c2 100644 --- a/Core/src/Surfaces/DiscSurface.cpp +++ b/Core/src/Surfaces/DiscSurface.cpp @@ -145,7 +145,7 @@ std::string DiscSurface::name() const { const SurfaceBounds& DiscSurface::bounds() const { if (m_bounds) { - return (*(m_bounds.get())); + return *m_bounds; } return s_noBounds; } diff --git a/Core/src/Surfaces/LineSurface.cpp b/Core/src/Surfaces/LineSurface.cpp index 6f6350625d6..9fb437bfe91 100644 --- a/Core/src/Surfaces/LineSurface.cpp +++ b/Core/src/Surfaces/LineSurface.cpp @@ -132,7 +132,7 @@ Vector3 LineSurface::normal(const GeometryContext& gctx, const Vector3& pos, const SurfaceBounds& LineSurface::bounds() const { if (m_bounds) { - return (*m_bounds.get()); + return *m_bounds; } return s_noBounds; } diff --git a/Core/src/Surfaces/PlaneSurface.cpp b/Core/src/Surfaces/PlaneSurface.cpp index 9437937afc4..aaaac177213 100644 --- a/Core/src/Surfaces/PlaneSurface.cpp +++ b/Core/src/Surfaces/PlaneSurface.cpp @@ -87,7 +87,7 @@ std::string PlaneSurface::name() const { const SurfaceBounds& PlaneSurface::bounds() const { if (m_bounds) { - return (*m_bounds.get()); + return *m_bounds; } return s_noBounds; } diff --git a/Examples/Algorithms/Geant4/src/Geant4Simulation.cpp b/Examples/Algorithms/Geant4/src/Geant4Simulation.cpp index 9ae93e97087..5b99ece34ff 100644 --- a/Examples/Algorithms/Geant4/src/Geant4Simulation.cpp +++ b/Examples/Algorithms/Geant4/src/Geant4Simulation.cpp @@ -93,7 +93,7 @@ void Geant4SimulationBase::commonInitialization() { } G4RunManager& Geant4SimulationBase::runManager() const { - return *m_geant4Instance->runManager.get(); + return *m_geant4Instance->runManager; } Geant4::EventStore& Geant4SimulationBase::eventStore() const { diff --git a/Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp b/Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp index 5120a29c334..dacdcaa99f0 100644 --- a/Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp +++ b/Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp @@ -26,7 +26,7 @@ ExternalAlignmentDecorator::ExternalAlignmentDecorator( : m_cfg(cfg), m_logger(std::move(logger)) { if (m_cfg.trackingGeometry != nullptr) { // parse and populate - parseGeometry(*m_cfg.trackingGeometry.get()); + parseGeometry(*m_cfg.trackingGeometry); } } diff --git a/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp b/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp index 9eca8f20de5..8ddad182380 100644 --- a/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp +++ b/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp @@ -269,7 +269,7 @@ ProtoLayerCreatorT::centralProtoLayers( m_cfg.centralModuleFrontsideStereo.at(icl) != 0.) { // twist by the stereo angle double stereo = m_cfg.centralModuleFrontsideStereo.at(icl); - (*mutableModuleTransform.get()) *= + (*mutableModuleTransform) *= Acts::AngleAxis3(-stereo, Acts::Vector3::UnitZ()); } // count the modules @@ -304,7 +304,7 @@ ProtoLayerCreatorT::centralProtoLayers( if (!m_cfg.centralModuleBacksideStereo.empty()) { // twist by the stereo angle double stereoBackSide = m_cfg.centralModuleBacksideStereo.at(icl); - (*mutableModuleTransform.get()) *= + (*mutableModuleTransform) *= Acts::AngleAxis3(-stereoBackSide, Acts::Vector3::UnitZ()); } // Finalize the transform @@ -478,7 +478,7 @@ ProtoLayerCreatorT::createProtoLayers( // twist by the stereo angle double stereoBackSide = m_cfg.posnegModuleBacksideStereo.at(ipnl).at(ipnR); - (*mutableModuleTransform.get()) *= + (*mutableModuleTransform) *= Acts::AngleAxis3(-stereoBackSide, Acts::Vector3::UnitZ()); } // Finalize the transform diff --git a/Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp b/Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp index e40c679a055..c101b3efcf5 100644 --- a/Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp +++ b/Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp @@ -351,9 +351,8 @@ void writeVolume(SurfaceWriter& sfWriter, SurfaceGridWriter& sfGridWriter, // step down into hierarchy to process all child volumnes if (volume.confinedVolumes()) { for (const auto& confined : volume.confinedVolumes()->arrayObjects()) { - writeVolume(sfWriter, sfGridWriter, lvWriter, *confined.get(), - writeSensitive, writeBoundary, writeSurfaceGrid, - writeLayerVolume, geoCtx); + writeVolume(sfWriter, sfGridWriter, lvWriter, *confined, writeSensitive, + writeBoundary, writeSurfaceGrid, writeLayerVolume, geoCtx); } } } diff --git a/Examples/Io/Json/src/JsonSurfacesWriter.cpp b/Examples/Io/Json/src/JsonSurfacesWriter.cpp index 9e6a5cc0135..d8f9c00b5e5 100644 --- a/Examples/Io/Json/src/JsonSurfacesWriter.cpp +++ b/Examples/Io/Json/src/JsonSurfacesWriter.cpp @@ -109,7 +109,7 @@ void collectSurfaces(std::vector& cSurfaces, // Step down into hierarchy to process all child volumnes if (volume.confinedVolumes()) { for (const auto& confined : volume.confinedVolumes()->arrayObjects()) { - collectSurfaces(cSurfaces, *confined.get(), writeLayer, writeApproach, + collectSurfaces(cSurfaces, *confined, writeLayer, writeApproach, writeSensitive, writeBoundary); } } diff --git a/Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp b/Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp index 7f0275ed1a8..ad142070626 100644 --- a/Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp +++ b/Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp @@ -113,7 +113,7 @@ Acts::DD4hepDetectorSurfaceFactory::constructSensitiveComponents( } // Attach surface material if present - attachSurfaceMaterial(gctx, "acts_surface_", dd4hepElement, *sSurface.get(), + attachSurfaceMaterial(gctx, "acts_surface_", dd4hepElement, *sSurface, dd4hepDetElement->thickness(), options); // return the surface return {dd4hepDetElement, sSurface}; diff --git a/Plugins/Geant4/src/Geant4Converters.cpp b/Plugins/Geant4/src/Geant4Converters.cpp index a2450fdf41f..645d8defc76 100644 --- a/Plugins/Geant4/src/Geant4Converters.cpp +++ b/Plugins/Geant4/src/Geant4Converters.cpp @@ -369,7 +369,7 @@ std::shared_ptr Acts::Geant4PhysicalVolumeConverter::surface( auto orientedToGlobal = axesOriented(toGlobal, axes); surface = Acts::Surface::makeShared(orientedToGlobal, std::move(bounds)); - assignMaterial(*surface.get(), original, compressed); + assignMaterial(*surface, original, compressed); return surface; } else { throw std::runtime_error("Can not convert 'G4Box' into forced shape."); @@ -386,7 +386,7 @@ std::shared_ptr Acts::Geant4PhysicalVolumeConverter::surface( auto orientedToGlobal = axesOriented(toGlobal, axes); surface = Acts::Surface::makeShared(orientedToGlobal, std::move(bounds)); - assignMaterial(*surface.get(), original, compressed); + assignMaterial(*surface, original, compressed); return surface; } else { throw std::runtime_error("Can not convert 'G4Trd' into forced shape."); @@ -403,7 +403,7 @@ std::shared_ptr Acts::Geant4PhysicalVolumeConverter::surface( auto orientedToGlobal = axesOriented(toGlobal, axes); surface = Acts::Surface::makeShared(orientedToGlobal, std::move(bounds)); - assignMaterial(*surface.get(), original, compressed); + assignMaterial(*surface, original, compressed); return surface; } else { throw std::runtime_error("Can not convert 'G4Trap' into forced shape."); @@ -437,7 +437,7 @@ std::shared_ptr Acts::Geant4PhysicalVolumeConverter::surface( } else { throw std::runtime_error("Can not convert 'G4Tubs' into forced shape."); } - assignMaterial(*surface.get(), original, compressed); + assignMaterial(*surface, original, compressed); return surface; } diff --git a/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelBlueprintCreater.hpp b/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelBlueprintCreater.hpp index 78dfca36996..ada9e3d8071 100644 --- a/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelBlueprintCreater.hpp +++ b/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelBlueprintCreater.hpp @@ -73,7 +73,7 @@ class GeoModelBlueprintCreater { throw std::runtime_error( "GeoModelBlueprintCreater::Blueprint: No top node created"); } - return *(topNode.get()); + return *topNode; } }; diff --git a/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp b/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp index 12df4605a92..8fd670c9a65 100644 --- a/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp +++ b/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp @@ -55,7 +55,7 @@ class GeoModelDetectorElement : public DetectorElementBase { // First create the detector element with a nullptr auto detElement = std::make_shared( geoPhysVol, nullptr, sfTransform, thickness); - auto surface = Surface::makeShared(bounds, *detElement.get()); + auto surface = Surface::makeShared(bounds, *detElement); detElement->attachSurface(surface); return detElement; } diff --git a/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp b/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp index 1f45a10d391..bdf168188de 100644 --- a/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp +++ b/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp @@ -100,7 +100,7 @@ Acts::GeoModelDetectorElementITk::convertFromGeomodel( detEl->physicalVolume(), nullptr, detEl->transform(gctx), detEl->thickness(), hardware, barrelEndcap, layerWheel, etaModule, phiModule, side); - auto surface = Surface::makeShared(bounds, *itkEl.get()); + auto surface = Surface::makeShared(bounds, *itkEl); itkEl->attachSurface(surface); itkEl->setDatabaseEntryName(detEl->databaseEntryName()); diff --git a/Plugins/Json/src/SurfaceJsonConverter.cpp b/Plugins/Json/src/SurfaceJsonConverter.cpp index fb6b0e7b08d..c924fd205db 100644 --- a/Plugins/Json/src/SurfaceJsonConverter.cpp +++ b/Plugins/Json/src/SurfaceJsonConverter.cpp @@ -44,13 +44,13 @@ void Acts::to_json(nlohmann::json& j, const Acts::Surface& surface) { void Acts::to_json(nlohmann::json& j, const std::shared_ptr& surface) { Acts::GeometryContext gctx; - j = SurfaceJsonConverter::toJson(gctx, *(surface.get())); + j = SurfaceJsonConverter::toJson(gctx, *surface); } void Acts::toJson(nlohmann::json& j, const std::shared_ptr& surface, const Acts::GeometryContext& gctx) { - j = SurfaceJsonConverter::toJson(gctx, *(surface.get())); + j = SurfaceJsonConverter::toJson(gctx, *surface); } std::shared_ptr Acts::SurfaceJsonConverter::fromJson( diff --git a/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp b/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp index 88fd5fbaeb5..d40de262466 100644 --- a/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp +++ b/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp @@ -108,7 +108,7 @@ void testProtoLayer() { auto addSurface = Surface::makeShared(Transform3::Identity(), rB); - pLayerSf.add(tgContext, *addSurface.get()); + pLayerSf.add(tgContext, *addSurface); // CHECK That if you now have 5 surfaces BOOST_CHECK_EQUAL(pLayerSf.surfaces().size(), 5); diff --git a/Tests/UnitTests/Core/TrackFitting/FitterTestsCommon.hpp b/Tests/UnitTests/Core/TrackFitting/FitterTestsCommon.hpp index 757e6f5da81..b2eed053510 100644 --- a/Tests/UnitTests/Core/TrackFitting/FitterTestsCommon.hpp +++ b/Tests/UnitTests/Core/TrackFitting/FitterTestsCommon.hpp @@ -22,6 +22,7 @@ #include "Acts/MagneticField/ConstantBField.hpp" #include "Acts/MagneticField/MagneticFieldContext.hpp" #include "Acts/Propagator/Navigator.hpp" +#include "Acts/Propagator/Propagator.hpp" #include "Acts/Propagator/StraightLineStepper.hpp" #include "Acts/Surfaces/CurvilinearSurface.hpp" #include "Acts/Tests/CommonHelpers/CubicTrackingGeometry.hpp" diff --git a/Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp b/Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp index d0ee306a18c..5abf5d62c94 100644 --- a/Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp +++ b/Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp @@ -36,8 +36,8 @@ BOOST_AUTO_TEST_CASE(Geant4DetectorElement_construction) { auto rSurface = Acts::Surface::makeShared( rTransform, std::move(rBounds)); // A detector element - Acts::Geant4DetectorElement g4DetElement(rSurface, *g4physVol.get(), - rTransform, 0.1); + Acts::Geant4DetectorElement g4DetElement(rSurface, *g4physVol, rTransform, + 0.1); BOOST_CHECK_EQUAL(g4DetElement.thickness(), 0.1); BOOST_CHECK_EQUAL(&g4DetElement.surface(), rSurface.get()); diff --git a/Tests/UnitTests/Plugins/TGeo/TGeoParserTests.cpp b/Tests/UnitTests/Plugins/TGeo/TGeoParserTests.cpp index 78fd673d0bf..1e8ff4e5515 100644 --- a/Tests/UnitTests/Plugins/TGeo/TGeoParserTests.cpp +++ b/Tests/UnitTests/Plugins/TGeo/TGeoParserTests.cpp @@ -61,7 +61,7 @@ BOOST_AUTO_TEST_CASE(TGeoParser_Pixel) { ObjVisualization3D objVis; for (auto& snode : tgpState.selectedNodes) { const auto& shape = *(snode.node->GetVolume()->GetShape()); - const auto& transform = *(snode.transform.get()); + const auto& transform = *snode.transform; auto [surface, thickness] = TGeoSurfaceConverter::toSurface(shape, transform, axes, scale); GeometryView3D::drawSurface(objVis, *surface, tgContext); @@ -97,7 +97,7 @@ BOOST_AUTO_TEST_CASE(TGeoParser_Pixel_SelectInnermost) { ObjVisualization3D objVis; for (auto& snode : tgpState.selectedNodes) { const auto& shape = *(snode.node->GetVolume()->GetShape()); - const auto& transform = *(snode.transform.get()); + const auto& transform = *snode.transform; auto [surface, thickness] = TGeoSurfaceConverter::toSurface( shape, transform, axes, tgpOptions.unit); GeometryView3D::drawSurface(objVis, *surface, tgContext); From 260a1435f6f364b84ca67d30bf954f61feaf71f6 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 13 Feb 2025 12:08:29 +0100 Subject: [PATCH 5/9] chore: Update particle data script (#4081) - Encode the `particle` package version used to run (also you can run this with `uv run` now) - Make the output use `std::type_t` as per our guidelines - Make the output use `std::array` over C-style arrays --- Core/src/Definitions/ParticleDataTable.hpp | 9 +++++---- Fatras/scripts/generate_particle_data_table.py | 16 ++++++++++++---- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/Core/src/Definitions/ParticleDataTable.hpp b/Core/src/Definitions/ParticleDataTable.hpp index a030f3ed82f..fc78327c504 100644 --- a/Core/src/Definitions/ParticleDataTable.hpp +++ b/Core/src/Definitions/ParticleDataTable.hpp @@ -12,6 +12,7 @@ #pragma once +#include #include #include @@ -21,7 +22,7 @@ // within all column arrays. static constexpr std::uint32_t kParticlesCount = 6502u; -static const std::int32_t kParticlesPdgNumber[kParticlesCount] = { +static const std::array kParticlesPdgNumber = { // Og294~ -1001182940, // Ts294~ @@ -13027,7 +13028,7 @@ static const std::int32_t kParticlesPdgNumber[kParticlesCount] = { // Og294 1001182940, }; -static const std::int16_t kParticlesThreeCharge[kParticlesCount] = { +static const std::array kParticlesThreeCharge = { // Og294~ -354, // Ts294~ @@ -26033,7 +26034,7 @@ static const std::int16_t kParticlesThreeCharge[kParticlesCount] = { // Og294 354, }; -static const float kParticlesMassMeV[kParticlesCount] = { +static const std::array kParticlesMassMeV = { // Og294~ 0.0f, // Ts294~ @@ -39039,7 +39040,7 @@ static const float kParticlesMassMeV[kParticlesCount] = { // Og294 0.0f, }; -static const char* const kParticlesName[kParticlesCount] = { +static const std::array kParticlesName = { "Og294~", "Ts294~", "Lv293~", diff --git a/Fatras/scripts/generate_particle_data_table.py b/Fatras/scripts/generate_particle_data_table.py index ebbbab2720b..d231ee231f2 100755 --- a/Fatras/scripts/generate_particle_data_table.py +++ b/Fatras/scripts/generate_particle_data_table.py @@ -1,4 +1,11 @@ #!/usr/bin/env python3 + +# /// script +# dependencies = [ +# "particle==0.24.0", +# ] +# /// + # # use scikit-hep/particle to generate c++ code for the particle data table. # @@ -40,6 +47,7 @@ def main(output_file): #pragma once #include +#include #include // Rows within the particle data table are sorted by their signed PDG particle @@ -59,10 +67,10 @@ def generate_code(table): num_rows = len(table) # name, c++ type, and output format for each column columns = [ - ("PdgNumber", "int32_t", "{}"), - ("ThreeCharge", "int16_t", "{}"), + ("PdgNumber", "std::int32_t", "{}"), + ("ThreeCharge", "std::int16_t", "{}"), ("MassMeV", "float", "{}f"), - ("Name", "char* const ", '"{}"'), + ("Name", " const char* const", '"{}"'), ] lines = [ CODE_HEADER, @@ -71,7 +79,7 @@ def generate_code(table): # build a separate array for each column for i, (variable_name, type_name, value_format) in enumerate(columns): lines.append( - f"static const {type_name} kParticles{variable_name}[kParticlesCount] = {{" + f"static const std::array<{type_name}, kParticlesCount> kParticles{variable_name} = {{" ) for row in table: From 9b9747306aa107a081087e388045060e0302092a Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 13 Feb 2025 18:44:00 +0100 Subject: [PATCH 6/9] test: Python test fixes: GSF + EDM4hep (#4088) This fixes a few small issues with the GSF debugger (shebangs) and using the python interpreter that runs pytest (otherwise modules might not be available). It also forces DD4hep to write EDM4hep for the EDM4hep ddsim run, which gives a better error message if DD4hep was built without EDM4hep. --- Examples/Python/tests/test_misc.py | 4 ++-- Examples/Python/tests/test_reader.py | 1 + Examples/Scripts/GsfDebugger/make_gsf_verbose_log.py | 2 +- Examples/Scripts/GsfDebugger/src/main.py | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Examples/Python/tests/test_misc.py b/Examples/Python/tests/test_misc.py index 066fd4c999f..7a5d627106b 100644 --- a/Examples/Python/tests/test_misc.py +++ b/Examples/Python/tests/test_misc.py @@ -44,7 +44,7 @@ def test_gsf_debugger(tmp_path): env = os.environ.copy() env["PYTHONPATH"] = f"{scriptdir}:{env['PYTHONPATH']}" gsf_result = subprocess.run( - [gsf_script], capture_output=True, cwd=tmp_path, env=env + [sys.executable, gsf_script], capture_output=True, cwd=tmp_path, env=env ) logfile = tmp_path / "test.log" @@ -54,6 +54,6 @@ def test_gsf_debugger(tmp_path): assert gsf_result.returncode == 0 debugger_result = subprocess.run( - [debugger, f"--logfile={logfile}", "--nogui"], cwd=tmp_path + [sys.executable, debugger, f"--logfile={logfile}", "--nogui"], cwd=tmp_path ) assert debugger_result.returncode == 0 diff --git a/Examples/Python/tests/test_reader.py b/Examples/Python/tests/test_reader.py index 0153cc3b8d7..928ec8e5d35 100644 --- a/Examples/Python/tests/test_reader.py +++ b/Examples/Python/tests/test_reader.py @@ -278,6 +278,7 @@ def generate_input_test_edm4hep_simhit_reader(input, output): ddsim.gun.distribution = "eta" ddsim.numberOfEvents = 10 ddsim.outputFile = output + ddsim.outputConfig.forceEDM4HEP = True ddsim.run() diff --git a/Examples/Scripts/GsfDebugger/make_gsf_verbose_log.py b/Examples/Scripts/GsfDebugger/make_gsf_verbose_log.py index 7f4b466ee84..d19a2463fb8 100755 --- a/Examples/Scripts/GsfDebugger/make_gsf_verbose_log.py +++ b/Examples/Scripts/GsfDebugger/make_gsf_verbose_log.py @@ -1,4 +1,4 @@ -#!/bin/python3 +#!/usr/bin/env python3 from pathlib import Path diff --git a/Examples/Scripts/GsfDebugger/src/main.py b/Examples/Scripts/GsfDebugger/src/main.py index 81712c5e4d4..6acc7e55207 100755 --- a/Examples/Scripts/GsfDebugger/src/main.py +++ b/Examples/Scripts/GsfDebugger/src/main.py @@ -1,4 +1,4 @@ -#!/bin/env python3 +#!/usr/bin/env python3 import argparse import os import sys From 316a385cafadcf985851236ed1951e048384904d Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 13 Feb 2025 22:03:33 +0100 Subject: [PATCH 7/9] build: Add cmake helper function for header compilation (#4083) This PR adds a CMake function that will - Take all headers given explicitly either by glob or explicitly - The first argument is a sort of scope target - The function creates a new target with the suffix `_HEADERS` - It loops over all input headers, creates a `.cpp` file which includes that header, and adds it to the `_HEADERS` target for compilation - The target is excluded from the "default" build to not slow it down. This will help static analysis tools like clang-tidy, because the `compile_commands.json` will be populated. This PR also removes a leftover `ACTS_BUILD_NONCOMPILE_TESTS` that was unused. --- CMakeLists.txt | 5 +- Core/CMakeLists.txt | 4 + .../Propagator/detail/LoopStepperUtils.hpp | 2 + .../Algorithms/Digitization/CMakeLists.txt | 2 + .../ActsExamples/Digitization/Smearers.hpp | 1 + .../Algorithms/MaterialMapping/CMakeLists.txt | 2 + .../MappingMaterialDecorator.hpp | 1 + .../Algorithms/Propagation/CMakeLists.txt | 2 + .../src/SimHitToSummaryConversion.cpp | 149 ------------------ Examples/Algorithms/Traccc/CMakeLists.txt | 2 + .../Algorithms/TrackFinding/CMakeLists.txt | 8 +- .../TrackFinding/HoughVectors.hpp | 11 -- .../Algorithms/TrackFindingML/CMakeLists.txt | 2 + .../Algorithms/TrackFitting/CMakeLists.txt | 2 + .../ContextualDetector/CMakeLists.txt | 2 + .../Detectors/GenericDetector/CMakeLists.txt | 2 + .../Detectors/MagneticField/CMakeLists.txt | 2 + .../Detectors/TGeoDetector/CMakeLists.txt | 2 + Examples/Framework/CMakeLists.txt | 2 + Examples/Io/Csv/src/CsvOutputData.hpp | 1 + Examples/Io/Json/CMakeLists.txt | 3 + Examples/Io/Obj/CMakeLists.txt | 2 + .../ActsExamples/Io/Obj/ObjWriterOptions.hpp | 97 ------------ .../Plugins/Podio/PodioTrackContainer.hpp | 1 + Tests/CommonHelpers/CMakeLists.txt | 10 +- .../Acts/Tests/CommonHelpers/Assertions.hpp | 0 .../Tests/CommonHelpers/BenchmarkTools.hpp | 0 .../CommonHelpers/CubicTrackingGeometry.hpp | 0 .../CommonHelpers/CylindricalDetector.hpp | 0 .../CylindricalTrackingGeometry.hpp | 0 .../Tests/CommonHelpers/DataDirectory.hpp | 0 .../CommonHelpers/DetectorElementStub.hpp | 0 .../Tests/CommonHelpers/FloatComparisons.hpp | 0 .../Tests/CommonHelpers/LineSurfaceStub.hpp | 0 .../CommonHelpers/MeasurementsCreator.hpp | 0 .../CommonHelpers/PredefinedMaterials.hpp | 0 .../Tests/CommonHelpers/TestSpacePoint.hpp | 1 + .../CommonHelpers/WhiteBoardUtilities.hpp | 0 .../CylindricalDetector.cpp | 0 .../CommonHelpers => src}/DataDirectory.cpp | 0 cmake/ActsFunctions.cmake | 75 +++++++++ docs/getting_started.md | 2 +- 42 files changed, 127 insertions(+), 268 deletions(-) delete mode 100644 Examples/Algorithms/Propagation/src/SimHitToSummaryConversion.cpp delete mode 100644 Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/HoughVectors.hpp delete mode 100644 Examples/Io/Obj/include/ActsExamples/Io/Obj/ObjWriterOptions.hpp rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/Assertions.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/BenchmarkTools.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/CubicTrackingGeometry.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/CylindricalDetector.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/CylindricalTrackingGeometry.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/DataDirectory.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/DetectorElementStub.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/FloatComparisons.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/LineSurfaceStub.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/MeasurementsCreator.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/PredefinedMaterials.hpp (100%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/TestSpacePoint.hpp (98%) rename Tests/CommonHelpers/{ => include}/Acts/Tests/CommonHelpers/WhiteBoardUtilities.hpp (100%) rename Tests/CommonHelpers/{Acts/Tests/CommonHelpers => src}/CylindricalDetector.cpp (100%) rename Tests/CommonHelpers/{Acts/Tests/CommonHelpers => src}/DataDirectory.cpp (100%) create mode 100644 cmake/ActsFunctions.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index a0dad0cf109..a4c313f8784 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -114,7 +114,6 @@ else() set(_default_examples_unit_tests OFF) endif() option(ACTS_BUILD_EXAMPLES_UNITTESTS "Build unit tests" ${_default_examples_unit_tests}) # default: ACTS_BUILD_UNITTESTS AND ACTS_BUILD_EXAMPLES -option(ACTS_BUILD_NONCOMPILE_TESTS "Build tests that check build failure invariants" OFF) option(ACTS_RUN_CLANG_TIDY "Run clang-tidy static analysis" OFF) # other options option(ACTS_BUILD_DOCS "Build documentation" OFF) @@ -128,6 +127,7 @@ set(ACTS_GPERF_INSTALL_DIR "" CACHE STRING "Hint to help find gperf if profiling option(ACTS_ENABLE_LOG_FAILURE_THRESHOLD "Enable failing on log messages with level above certain threshold" OFF) set(ACTS_LOG_FAILURE_THRESHOLD "" CACHE STRING "Log level above which an exception should be automatically thrown. If ACTS_ENABLE_LOG_FAILURE_THRESHOLD is set and this is unset, this will enable a runtime check of the log level.") +option(ACTS_COMPILE_HEADERS "Generate targets to compile header files" ON) # gersemi: on # handle option inter-dependencies and the everything flag @@ -280,6 +280,9 @@ set(_acts_annoy_version 1.17.3) # this version we will try so. set(_acts_boost_recommended_version 1.78.0) +# Help with compiler flags discovery +include(ActsFunctions) + # Include the sources for the external dependencies. include(ActsExternSources) diff --git a/Core/CMakeLists.txt b/Core/CMakeLists.txt index 25af0f03d49..e42936b9639 100644 --- a/Core/CMakeLists.txt +++ b/Core/CMakeLists.txt @@ -111,3 +111,7 @@ add_subdirectory(src/Utilities) add_subdirectory(src/Vertexing) add_subdirectory(src/Visualization) add_subdirectory(src/AmbiguityResolution) + +acts_compile_headers(ActsCore GLOB + include/Acts/**/*.hpp +) diff --git a/Core/include/Acts/Propagator/detail/LoopStepperUtils.hpp b/Core/include/Acts/Propagator/detail/LoopStepperUtils.hpp index 02b9d2e5b41..6e4686e44e9 100644 --- a/Core/include/Acts/Propagator/detail/LoopStepperUtils.hpp +++ b/Core/include/Acts/Propagator/detail/LoopStepperUtils.hpp @@ -13,6 +13,8 @@ #include #include +#include + namespace Acts::detail { /// A helper type for providinig a propagation state which can be used with diff --git a/Examples/Algorithms/Digitization/CMakeLists.txt b/Examples/Algorithms/Digitization/CMakeLists.txt index de98c7e74d4..0ea629028c9 100644 --- a/Examples/Algorithms/Digitization/CMakeLists.txt +++ b/Examples/Algorithms/Digitization/CMakeLists.txt @@ -17,6 +17,8 @@ target_link_libraries( PUBLIC ActsCore ActsExamplesFramework ) +acts_compile_headers(ActsExamplesDigitization GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesDigitization LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Algorithms/Digitization/include/ActsExamples/Digitization/Smearers.hpp b/Examples/Algorithms/Digitization/include/ActsExamples/Digitization/Smearers.hpp index 08c1ba06a74..36719fb836e 100644 --- a/Examples/Algorithms/Digitization/include/ActsExamples/Digitization/Smearers.hpp +++ b/Examples/Algorithms/Digitization/include/ActsExamples/Digitization/Smearers.hpp @@ -9,6 +9,7 @@ #pragma once #include "Acts/Utilities/AxisDefinitions.hpp" +#include "Acts/Utilities/BinningData.hpp" #include "Acts/Utilities/Result.hpp" #include "ActsExamples/Framework/RandomNumbers.hpp" #include "ActsFatras/Digitization/DigitizationError.hpp" diff --git a/Examples/Algorithms/MaterialMapping/CMakeLists.txt b/Examples/Algorithms/MaterialMapping/CMakeLists.txt index ac2bd9c5d0e..dace24c03e9 100644 --- a/Examples/Algorithms/MaterialMapping/CMakeLists.txt +++ b/Examples/Algorithms/MaterialMapping/CMakeLists.txt @@ -15,6 +15,8 @@ target_link_libraries( PUBLIC ActsCore ActsExamplesFramework ) +acts_compile_headers(ActsExamplesMaterialMapping GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesMaterialMapping LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Algorithms/MaterialMapping/include/ActsExamples/MaterialMapping/MappingMaterialDecorator.hpp b/Examples/Algorithms/MaterialMapping/include/ActsExamples/MaterialMapping/MappingMaterialDecorator.hpp index c343fbe58d6..5f3bcca5936 100644 --- a/Examples/Algorithms/MaterialMapping/include/ActsExamples/MaterialMapping/MappingMaterialDecorator.hpp +++ b/Examples/Algorithms/MaterialMapping/include/ActsExamples/MaterialMapping/MappingMaterialDecorator.hpp @@ -8,6 +8,7 @@ #pragma once +#include "Acts/Geometry/TrackingGeometry.hpp" #include "Acts/Geometry/TrackingVolume.hpp" #include "Acts/Material/IMaterialDecorator.hpp" #include "Acts/Material/ISurfaceMaterial.hpp" diff --git a/Examples/Algorithms/Propagation/CMakeLists.txt b/Examples/Algorithms/Propagation/CMakeLists.txt index c9b2b68ca36..15060d6b69d 100644 --- a/Examples/Algorithms/Propagation/CMakeLists.txt +++ b/Examples/Algorithms/Propagation/CMakeLists.txt @@ -10,6 +10,8 @@ target_link_libraries( PUBLIC ActsCore ActsExamplesFramework ) +acts_compile_headers(ActsExamplesPropagation GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesPropagation LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Algorithms/Propagation/src/SimHitToSummaryConversion.cpp b/Examples/Algorithms/Propagation/src/SimHitToSummaryConversion.cpp deleted file mode 100644 index 567923776c1..00000000000 --- a/Examples/Algorithms/Propagation/src/SimHitToSummaryConversion.cpp +++ /dev/null @@ -1,149 +0,0 @@ -// 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 "ActsExamples/Propagation/SimHitToSummaryConversion.hpp" - -#include "ActsExamples/EventData/GeometryContainers.hpp" - -namespace { -/// Helper method to cancatenate the steps and push them onto the surface -/// -/// @param gctx is the geometry context -/// @param steps is the vector of steps to concatenate -/// @param surface is the surface to push the steps onto -/// -/// @return the concatenated step -Acts::detail::Step concatenateSteps( - const Acts::GeometryContext& gctx, - const std::vector& steps, - const Acts::Surface& surface) { - // Average the position and direction - Acts::detail::Step concatStep; - if (steps.size() > 1) { - for (const Acts::detail::Step& step : steps) { - concatStep.position += step.position; - concatStep.momentum += step.momentum; - } - double weight = 1. / steps.size(); - concatStep.position *= weight; - concatStep.momentum *= weight; - } else { - concatStep = steps.front(); - } - // Re-evaulate the position with a surface intersection - auto intersection = - surface.intersect(gctx, concatStep.position, concatStep.momentum); - for (const auto& rsIntersection : intersection.split()) { - if (rsIntersection.isValid()) { - concatStep.position = rsIntersection.position(); - break; - } - } - // Set the surface identifier - concatStep.geoID = surface.geometryId(); - return concatStep; -} -} // namespace - -ActsExamples::SimHitToSummaryConversion::SimHitToSummaryConversion( - const Config& config, Acts::Logging::Level level) - : IAlgorithm("SimHitToSummaryConversion", level), m_cfg(config) { - if (m_cfg.inputSimHits.empty()) { - throw std::invalid_argument("Missing simulated hits input collection"); - } - if (m_cfg.inputParticles.empty()) { - throw std::invalid_argument("Missing simulated particles input collection"); - } - m_inputSimHits.initialize(m_cfg.inputSimHits); - m_inputParticles.initialize(m_cfg.inputParticles); - m_outputSummary.initialize(m_cfg.outputSummaryCollection); -} - -ActsExamples::ProcessCode ActsExamples::SimHitToSummaryConversion::execute( - const AlgorithmContext& context) const { - // Retrieve the simulated hits - const auto& simHits = m_inputSimHits(context); - // Retrieve the particles at its basis - const auto& particles = m_inputParticles(context); - - PropagationSummaries propagationSummaries; - propagationSummaries.reserve(particles.size()); - - // Prepare and sort - std::unordered_map>> - trackSteps; - for (const auto& simHitsGroup : groupByModule(simHits)) { - // Manual pair unpacking instead of using - // auto [moduleGeoId, moduleSimHits] : ... - // otherwise clang on macos complains that it is unable to capture the local - // binding in the lambda used for visiting the smearer below. - Acts::GeometryIdentifier moduleGeoId = simHitsGroup.first; - const auto& moduleSimHits = simHitsGroup.second; - std::unordered_map> - moduleSteps; - for (const auto& simHit : moduleSimHits) { - unsigned long paritcleId = simHit.particleId().value(); - if (!moduleSteps.contains(paritcleId)) { - moduleSteps[paritcleId] = std::vector(); - } - double hx = simHit.fourPosition().x() / Acts::UnitConstants::mm; - double hy = simHit.fourPosition().y() / Acts::UnitConstants::mm; - double hz = simHit.fourPosition().z() / Acts::UnitConstants::mm; - Acts::detail::Step step; - step.position = Acts::Vector3(hx, hy, hz); - step.momentum = simHit.direction(); - step.geoID = moduleGeoId; - step.navDir = Acts::Direction::Forward(); - moduleSteps[paritcleId].push_back(step); - } - // Loop over and fill into the trackSteps - for (const auto& [particleId, steps] : moduleSteps) { - if (!trackSteps.contains(particleId)) { - trackSteps[particleId] = std::vector>(); - } - trackSteps[particleId].push_back(steps); - } - } - - // Loop over the particles and create the propagation summaries - for (const auto& particle : particles) { - // Create the propagation summary - Acts::CurvilinearTrackParameters start( - particle.fourPosition(), particle.direction(), - particle.charge() / particle.momentum().norm(), std::nullopt, - particle.hypothesis()); - PropagationSummary propagationSummary(start); - // Find the associated steps - auto steps = trackSteps.find(particle.particleId().value()); - if (steps != trackSteps.end()) { - for (const std::vector& moduleSteps : steps->second) { - // Get the GeometryIdentifier of the surface - Acts::GeometryIdentifier surface = moduleSteps.front().geoID; - // Find the surface - auto surfaceIt = m_cfg.surfaceByIdentifier.find(surface); - if (surfaceIt == m_cfg.surfaceByIdentifier.end()) { - throw std::invalid_argument("Surface not found, should not happen"); - } - Acts::detail::Step concatStep = concatenateSteps( - context.geoContext, moduleSteps, *surfaceIt->second); - propagationSummary.steps.push_back(concatStep); - } - } else { - ACTS_WARNING("No steps found for particle " - << particle.particleId().value()); - } - - propagationSummaries.push_back(std::move(propagationSummary)); - } - - // Write the propagation step data to the event store - m_outputSummary(context, std::move(propagationSummaries)); - - return ProcessCode::SUCCESS; -} diff --git a/Examples/Algorithms/Traccc/CMakeLists.txt b/Examples/Algorithms/Traccc/CMakeLists.txt index 11837f12298..d2409d38017 100644 --- a/Examples/Algorithms/Traccc/CMakeLists.txt +++ b/Examples/Algorithms/Traccc/CMakeLists.txt @@ -14,4 +14,6 @@ target_link_libraries( ActsPluginDetray ) +acts_compile_headers(ActsExamplesTraccc GLOB "include/**/*.hpp") + install(TARGETS ActsExamplesTraccc LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/Examples/Algorithms/TrackFinding/CMakeLists.txt b/Examples/Algorithms/TrackFinding/CMakeLists.txt index 33b924e35aa..3f14723c2ca 100644 --- a/Examples/Algorithms/TrackFinding/CMakeLists.txt +++ b/Examples/Algorithms/TrackFinding/CMakeLists.txt @@ -20,14 +20,12 @@ target_include_directories( target_link_libraries( ActsExamplesTrackFinding - PUBLIC - ActsCore - ActsExamplesFramework - ActsExamplesIoJson - ActsExamplesMagneticField + PUBLIC ActsCore ActsExamplesFramework ActsExamplesMagneticField PRIVATE ROOT::Core ROOT::Geom ROOT::Graf ROOT::Hist ROOT::Gpad ) +acts_compile_headers(ActsExamplesTrackFinding GLOB "include/**/*.hpp") + # If Hashing examples are enabled, add them to the build if(ACTS_BUILD_EXAMPLES_HASHING) target_sources( diff --git a/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/HoughVectors.hpp b/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/HoughVectors.hpp deleted file mode 100644 index 3e0e54a41e1..00000000000 --- a/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/HoughVectors.hpp +++ /dev/null @@ -1,11 +0,0 @@ -// 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/. - -#pragma once - -#include "Acts/Seeding/HoughVectors.hpp" diff --git a/Examples/Algorithms/TrackFindingML/CMakeLists.txt b/Examples/Algorithms/TrackFindingML/CMakeLists.txt index 80f55f60579..f5680c16a03 100644 --- a/Examples/Algorithms/TrackFindingML/CMakeLists.txt +++ b/Examples/Algorithms/TrackFindingML/CMakeLists.txt @@ -15,6 +15,8 @@ target_link_libraries( PUBLIC ActsCore ActsPluginOnnx ActsExamplesFramework ) +acts_compile_headers(ActsExamplesTrackFindingML GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesTrackFindingML LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Algorithms/TrackFitting/CMakeLists.txt b/Examples/Algorithms/TrackFitting/CMakeLists.txt index a6a9a5afe3b..497e5bcec53 100644 --- a/Examples/Algorithms/TrackFitting/CMakeLists.txt +++ b/Examples/Algorithms/TrackFitting/CMakeLists.txt @@ -17,6 +17,8 @@ target_link_libraries( PUBLIC ActsCore ActsExamplesFramework ActsExamplesMagneticField ) +acts_compile_headers(ActsExamplesTrackFitting GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesTrackFitting LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Detectors/ContextualDetector/CMakeLists.txt b/Examples/Detectors/ContextualDetector/CMakeLists.txt index 550a4cb450a..f4ba2423bc2 100644 --- a/Examples/Detectors/ContextualDetector/CMakeLists.txt +++ b/Examples/Detectors/ContextualDetector/CMakeLists.txt @@ -20,6 +20,8 @@ target_link_libraries( ActsExamplesDetectorGeneric ) +acts_compile_headers(ActsExamplesDetectorContextual GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesDetectorContextual LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Detectors/GenericDetector/CMakeLists.txt b/Examples/Detectors/GenericDetector/CMakeLists.txt index 7260a22b128..cf3b334e611 100644 --- a/Examples/Detectors/GenericDetector/CMakeLists.txt +++ b/Examples/Detectors/GenericDetector/CMakeLists.txt @@ -6,6 +6,8 @@ add_library( src/GenericDetectorElement.cpp ) +acts_compile_headers(ActsExamplesDetectorGeneric GLOB "include/**/*.hpp") + target_include_directories( ActsExamplesDetectorGeneric PUBLIC $ diff --git a/Examples/Detectors/MagneticField/CMakeLists.txt b/Examples/Detectors/MagneticField/CMakeLists.txt index b8eb5f9c326..e9f3f929450 100644 --- a/Examples/Detectors/MagneticField/CMakeLists.txt +++ b/Examples/Detectors/MagneticField/CMakeLists.txt @@ -17,6 +17,8 @@ target_link_libraries( PRIVATE std::filesystem ) +acts_compile_headers(ActsExamplesMagneticField GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesMagneticField LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Detectors/TGeoDetector/CMakeLists.txt b/Examples/Detectors/TGeoDetector/CMakeLists.txt index 11fac0c179c..04d4c9ff74f 100644 --- a/Examples/Detectors/TGeoDetector/CMakeLists.txt +++ b/Examples/Detectors/TGeoDetector/CMakeLists.txt @@ -22,6 +22,8 @@ target_link_libraries( ActsExamplesITkModuleSplitting ) +acts_compile_headers(ActsExamplesDetectorTGeo GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesDetectorTGeo LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Framework/CMakeLists.txt b/Examples/Framework/CMakeLists.txt index 26e280b4dd3..995575def4b 100644 --- a/Examples/Framework/CMakeLists.txt +++ b/Examples/Framework/CMakeLists.txt @@ -58,6 +58,8 @@ if(NOT TBB_FOUND) endif() target_link_libraries(ActsExamplesFramework PUBLIC TBB::tbb) +acts_compile_headers(ActsExamplesFramework GLOB "include/**/*.hpp") + install( TARGETS ActsExamplesFramework LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} diff --git a/Examples/Io/Csv/src/CsvOutputData.hpp b/Examples/Io/Csv/src/CsvOutputData.hpp index 63fd8a23f0d..47638af8591 100644 --- a/Examples/Io/Csv/src/CsvOutputData.hpp +++ b/Examples/Io/Csv/src/CsvOutputData.hpp @@ -9,6 +9,7 @@ #pragma once #include +#include #include diff --git a/Examples/Io/Json/CMakeLists.txt b/Examples/Io/Json/CMakeLists.txt index 5461f5dd98d..02b0fde696f 100644 --- a/Examples/Io/Json/CMakeLists.txt +++ b/Examples/Io/Json/CMakeLists.txt @@ -18,6 +18,9 @@ target_link_libraries( ActsExamplesDigitization ActsExamplesFramework ActsExamplesMaterialMapping + ActsExamplesTrackFinding ) +acts_compile_headers(ActsExamplesIoJson GLOB "include/**/*.hpp") + install(TARGETS ActsExamplesIoJson LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/Examples/Io/Obj/CMakeLists.txt b/Examples/Io/Obj/CMakeLists.txt index bce7bf19e6b..ee87692c7cc 100644 --- a/Examples/Io/Obj/CMakeLists.txt +++ b/Examples/Io/Obj/CMakeLists.txt @@ -15,4 +15,6 @@ target_link_libraries( PUBLIC ActsCore ActsExamplesFramework Threads::Threads ) +acts_compile_headers(ActsExamplesIoObj GLOB "include/**/*.hpp") + install(TARGETS ActsExamplesIoObj LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/Examples/Io/Obj/include/ActsExamples/Io/Obj/ObjWriterOptions.hpp b/Examples/Io/Obj/include/ActsExamples/Io/Obj/ObjWriterOptions.hpp deleted file mode 100644 index f62a8b6a557..00000000000 --- a/Examples/Io/Obj/include/ActsExamples/Io/Obj/ObjWriterOptions.hpp +++ /dev/null @@ -1,97 +0,0 @@ -// 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/. - -#pragma once - -#include "Acts/Utilities/Logger.hpp" -#include "ActsExamples/Plugins/Obj/ObjTrackingGeometryWriter.hpp" -#include "ActsExamples/Utilities/Options.hpp" - -#include - -namespace ActsExamples { - -namespace Options { - -/// Common obj writing options -/// -/// @tparam aopt_t Type of the options object (from BOOST) -/// -/// @param opt The options object, where string based options are attached -void addObjWriterOptions(boost::program_options::options_description& opt) { - namespace po = boost::program_options; - opt.add_options()("obj-precision", po::value()->default_value(6), - "Floating number output precision.")( - "obj-scalor", po::value()->default_value(1.), - "Optional scaling from Acts units to output units.")( - "obj-container-view", - po::value>()->default_value({{0, 220, 220, 220, 0}}), - "View configuration of container volumes (vis/novis, r, g, b, trimesh).")( - "obj-volume-view", - po::value>()->default_value({{1, 220, 220, 0, 0}}), - "View configuration of navigation volumes (vis/novis, r, g, b, " - "trimesh).")( - "obj-layer-view", - po::value>()->default_value({{1, 100, 180, 240, 0}}), - "View configuration of layer structures (vis/novis, r, g, b, trimesh).")( - "obj-sensitive-view", - po::value>()->default_value({{1, 0, 180, 240, 0}}), - "View configuration of sensitive surfaces (vis/novis, r, g, b, " - "trimesh).")( - "obj-passive-view", - po::value>()->default_value({{1, 240, 280, 0, 0}}), - "View configuration of sensitive surfaces (vis/novis, r, g, " - "b, trimesh).")( - "obj-grid-view", - po::value>()->default_value({{1, 220, 0, 0, 0}}), - "View configuration of grid structures (vis/novis, r, g, b, trimesh).")( - "obj-grid-offset", po::value()->default_value(0.), - "View offset of grid values.")("obj-grid-thickness", - po::value()->default_value(0.5), - "Thickness of grid objects."); -} - -/// read the evgen options and return a Config file -ActsExamples::ObjTrackingGeometryWriter::Config -readObjTrackingGeometryWriterConfig( - const boost::program_options::variables_map& vm) { - namespace po = boost::program_options; - ActsExamples::ObjTrackingGeometryWriter::Config objTgConfig; - - objTgConfig.outputPrecision = vm["obj-precision"].template as(); - objTgConfig.outputScalor = vm["obj-scalor"].template as(); - - auto setView = [&](const std::string& vname, - Acts::ViewConfig& viewCfg) -> void { - auto cview = vm[vname].template as>(); - if (not cview.empty()) { - if (cview[0] == 0) { - viewCfg.visible = false; - } else if (cview.size() > 3) { - viewCfg.color = {cview[1], cview[2], cview[3]}; - if (cview.size() > 4 and cview[4] != 0) { - viewCfg.triangulate = true; - } - } - } - }; - - setView("obj-container-view", objTgConfig.containerView); - setView("obj-volume-view", objTgConfig.containerView); - setView("obj-sensitive-view", objTgConfig.sensitiveView); - setView("obj-passive-view", objTgConfig.passiveView); - setView("obj-grid-view", objTgConfig.gridView); - objTgConfig.gridView.offset = vm["obj-grid-offset"].template as(); - objTgConfig.gridView.lineThickness = - vm["obj-grid-thickness"].template as(); - - return objTgConfig; -} - -} // namespace Options -} // namespace ActsExamples diff --git a/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp b/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp index 79ccf2249a1..442b36968b4 100644 --- a/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp +++ b/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp @@ -13,6 +13,7 @@ #include "Acts/EventData/TrackContainer.hpp" #include "Acts/EventData/TrackStateProxy.hpp" #include "Acts/EventData/detail/DynamicColumn.hpp" +#include "Acts/EventData/detail/DynamicKeyIterator.hpp" #include "Acts/Plugins/Podio/PodioDynamicColumns.hpp" #include "Acts/Plugins/Podio/PodioUtil.hpp" #include "Acts/Utilities/Helpers.hpp" diff --git a/Tests/CommonHelpers/CMakeLists.txt b/Tests/CommonHelpers/CMakeLists.txt index 84417f3ddb4..c5c0a18649d 100644 --- a/Tests/CommonHelpers/CMakeLists.txt +++ b/Tests/CommonHelpers/CMakeLists.txt @@ -4,8 +4,8 @@ file(TO_NATIVE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/../Data" acts_test_data_dir) add_library( ActsTestsCommonHelpers SHARED - Acts/Tests/CommonHelpers/DataDirectory.cpp - Acts/Tests/CommonHelpers/CylindricalDetector.cpp + src/DataDirectory.cpp + src/CylindricalDetector.cpp ) target_compile_definitions( ActsTestsCommonHelpers @@ -15,10 +15,12 @@ target_compile_definitions( ) target_include_directories( ActsTestsCommonHelpers - PUBLIC $ + PUBLIC $ ) target_link_libraries( ActsTestsCommonHelpers - PUBLIC ActsCore + PUBLIC ActsCore ActsExamplesFramework PRIVATE std::filesystem ) + +acts_compile_headers(ActsTestsCommonHelpers GLOB "include/Acts/**/*.hpp") diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/Assertions.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/Assertions.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/Assertions.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/Assertions.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/BenchmarkTools.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/BenchmarkTools.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/BenchmarkTools.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/BenchmarkTools.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/CubicTrackingGeometry.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/CubicTrackingGeometry.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/CubicTrackingGeometry.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/CubicTrackingGeometry.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/CylindricalDetector.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/CylindricalDetector.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/CylindricalDetector.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/CylindricalDetector.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/CylindricalTrackingGeometry.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/CylindricalTrackingGeometry.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/CylindricalTrackingGeometry.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/CylindricalTrackingGeometry.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/DataDirectory.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/DataDirectory.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/DataDirectory.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/DataDirectory.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/DetectorElementStub.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/DetectorElementStub.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/DetectorElementStub.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/DetectorElementStub.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/FloatComparisons.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/FloatComparisons.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/FloatComparisons.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/FloatComparisons.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/LineSurfaceStub.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/LineSurfaceStub.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/LineSurfaceStub.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/LineSurfaceStub.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/MeasurementsCreator.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/MeasurementsCreator.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/MeasurementsCreator.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/MeasurementsCreator.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/PredefinedMaterials.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/PredefinedMaterials.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/PredefinedMaterials.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/PredefinedMaterials.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/TestSpacePoint.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/TestSpacePoint.hpp similarity index 98% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/TestSpacePoint.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/TestSpacePoint.hpp index b50451e0e7d..e3a2c60f1f6 100644 --- a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/TestSpacePoint.hpp +++ b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/TestSpacePoint.hpp @@ -11,6 +11,7 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Definitions/Common.hpp" #include "Acts/EventData/SourceLink.hpp" +#include "Acts/EventData/detail/TestSourceLink.hpp" #include #include diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/WhiteBoardUtilities.hpp b/Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/WhiteBoardUtilities.hpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/WhiteBoardUtilities.hpp rename to Tests/CommonHelpers/include/Acts/Tests/CommonHelpers/WhiteBoardUtilities.hpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/CylindricalDetector.cpp b/Tests/CommonHelpers/src/CylindricalDetector.cpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/CylindricalDetector.cpp rename to Tests/CommonHelpers/src/CylindricalDetector.cpp diff --git a/Tests/CommonHelpers/Acts/Tests/CommonHelpers/DataDirectory.cpp b/Tests/CommonHelpers/src/DataDirectory.cpp similarity index 100% rename from Tests/CommonHelpers/Acts/Tests/CommonHelpers/DataDirectory.cpp rename to Tests/CommonHelpers/src/DataDirectory.cpp diff --git a/cmake/ActsFunctions.cmake b/cmake/ActsFunctions.cmake new file mode 100644 index 00000000000..2619c29644e --- /dev/null +++ b/cmake/ActsFunctions.cmake @@ -0,0 +1,75 @@ +# This function adds a helper target called ${target}_HEADERS which has +# generated source files that include each hedaer that was given one-by-one. +# The generated target links against the one given in `target` and therefore +# has access to all includes. +# The generated target is not included in the default build, and is only meant +# to help tools like clangd discover compiler flags. +function(acts_compile_headers target) + set(options "") + set(oneValueArgs GLOB) + set(multiValueArgs "") + cmake_parse_arguments( + PARSE_ARGV + 0 + ARGS + "${options}" + "${oneValueArgs}" + "${multiValueArgs}" + ) + + if(NOT ACTS_COMPILE_HEADERS) + return() + endif() + + if(NOT "${ARGS_GLOB}" STREQUAL "") + if(NOT "${_headers}" STREQUAL "") + message(SEND_ERROR "Cannot use HEADERS and GLOB at the same time") + return() + endif() + + file( + GLOB_RECURSE _headers + RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} + ${ARGS_GLOB} + ) + endif() + + if("${_headers}" STREQUAL "") + message(SEND_ERROR "No headers specified") + return() + endif() + + set(_sources "") + + foreach(_file ${_headers}) + if(IS_ABSOLUTE "${_file}") + message(SEND_ERROR "Absolute paths are not allowed: ${_file}") + continue() + endif() + + set(_header_file "${CMAKE_CURRENT_SOURCE_DIR}/${_file}") + + if(NOT EXISTS "${_header_file}") + message(SEND_ERROR "File not found: ${_header_file}") + endif() + if(IS_DIRECTORY "${_header_file}") + message(SEND_ERROR "Path is a directory: ${_header_file}") + endif() + + get_filename_component(_header_file_name "${_file}" NAME_WLE) + get_filename_component(_header_directory "${_file}" DIRECTORY) + + set(_temporary_dir "${CMAKE_CURRENT_BINARY_DIR}/${_header_directory}") + + file(MAKE_DIRECTORY "${_temporary_dir}") + + set(_temporary_path "${_temporary_dir}/${_header_file_name}.cpp") + + file(WRITE "${_temporary_path}" "#include \"${_header_file}\"") + + list(APPEND _sources "${_temporary_path}") + endforeach() + + add_library(${target}_HEADERS SHARED EXCLUDE_FROM_ALL ${_sources}) + target_link_libraries(${target}_HEADERS PRIVATE ${target}) +endfunction() diff --git a/docs/getting_started.md b/docs/getting_started.md index 2b99227a1e7..75bcaef2534 100644 --- a/docs/getting_started.md +++ b/docs/getting_started.md @@ -305,7 +305,6 @@ components. | ACTS_BUILD_INTEGRATIONTESTS | Build integration tests
type: `bool`, default: `OFF` | | ACTS_BUILD_UNITTESTS | Build unit tests
type: `bool`, default: `OFF` | | ACTS_BUILD_EXAMPLES_UNITTESTS | Build unit tests
type: `bool`, default: `ACTS_BUILD_UNITTESTS AND ACTS_BUILD_EXAMPLES` | -| ACTS_BUILD_NONCOMPILE_TESTS | Build tests that check build failure
invariants
type: `bool`, default: `OFF` | | ACTS_RUN_CLANG_TIDY | Run clang-tidy static analysis
type: `bool`, default: `OFF` | | ACTS_BUILD_DOCS | Build documentation
type: `bool`, default: `OFF` | | ACTS_SETUP_BOOST | Explicitly set up Boost for the project
type: `bool`, default: `ON` | @@ -316,6 +315,7 @@ components. | ACTS_GPERF_INSTALL_DIR | Hint to help find gperf if profiling is
enabled
type: `string`, default: `""` | | ACTS_ENABLE_LOG_FAILURE_THRESHOLD | Enable failing on log messages with
level above certain threshold
type: `bool`, default: `OFF` | | ACTS_LOG_FAILURE_THRESHOLD | Log level above which an exception
should be automatically thrown. If
ACTS_ENABLE_LOG_FAILURE_THRESHOLD is set
and this is unset, this will enable a
runtime check of the log level.
type: `string`, default: `""` | +| ACTS_COMPILE_HEADERS | Generate targets to compile header files
type: `bool`, default: `ON` | All ACTS-specific options are disabled or empty by default and must be From 4e0bf58f1e28f0c7d76347dfa1b654cde6326cc4 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Thu, 13 Feb 2025 17:59:11 -0600 Subject: [PATCH 8/9] fix: require CUDA_STANDARD 20 in Plugins/Cuda (#4084) This PR requires CUDA_STANDARD 20 in the CUDA plugin, which is required since the use of the `numbers` header in https://github.com/acts-project/acts/pull/3781 (specifically through `Acts/Definitions/Units.hpp`. --- Plugins/Cuda/CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Plugins/Cuda/CMakeLists.txt b/Plugins/Cuda/CMakeLists.txt index 21adefee36a..4b9eaf71de9 100644 --- a/Plugins/Cuda/CMakeLists.txt +++ b/Plugins/Cuda/CMakeLists.txt @@ -47,7 +47,11 @@ target_include_directories( target_link_libraries(ActsPluginCuda2 PUBLIC ActsCore) set_target_properties( ActsPluginCuda2 - PROPERTIES CUDA_SEPARABLE_COMPILATION ON POSITION_INDEPENDENT_CODE ON + PROPERTIES + CUDA_STANDARD 20 + CUDA_STANDARD_REQUIRED ON + CUDA_SEPARABLE_COMPILATION ON + POSITION_INDEPENDENT_CODE ON ) # Install all CUDA plugins. From e4606c60445bbb6bc1cebef110009a3ba0e2e31e Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 14 Feb 2025 03:20:02 +0100 Subject: [PATCH 9/9] build: Use CONFIG mode for onnxruntime (#4086) Switch to using CMake's CONFIG mode for finding ONNX. This also deletes the find module we ship as of now. --- CMakeLists.txt | 2 +- Plugins/Onnx/CMakeLists.txt | 2 +- .../Acts/Plugins/Onnx/OnnxRuntimeBase.hpp | 2 +- .../Acts/Plugins/Onnx/SeedClassifier.hpp | 2 +- cmake/ActsCreatePackageConfig.cmake | 6 -- cmake/FindOnnxRuntime.cmake | 75 ------------------- 6 files changed, 4 insertions(+), 85 deletions(-) delete mode 100644 cmake/FindOnnxRuntime.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index a4c313f8784..5fb87075118 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -443,7 +443,7 @@ if(ACTS_BUILD_PLUGIN_EXATRKX) endif() endif() if(ACTS_BUILD_PLUGIN_ONNX OR ACTS_EXATRKX_ENABLE_ONNX) - find_package(OnnxRuntime ${_acts_onnxruntime_version} REQUIRED) + find_package(onnxruntime ${_acts_onnxruntime_version} CONFIG REQUIRED) endif() if(ACTS_BUILD_PLUGIN_EDM4HEP OR ACTS_BUILD_PLUGIN_PODIO) find_package(podio ${_acts_podio_version} CONFIG) diff --git a/Plugins/Onnx/CMakeLists.txt b/Plugins/Onnx/CMakeLists.txt index 3a07262ac71..2601c422479 100644 --- a/Plugins/Onnx/CMakeLists.txt +++ b/Plugins/Onnx/CMakeLists.txt @@ -18,7 +18,7 @@ target_include_directories( $ ) -target_link_libraries(ActsPluginOnnx PUBLIC ActsCore OnnxRuntime) +target_link_libraries(ActsPluginOnnx PUBLIC ActsCore onnxruntime::onnxruntime) install( TARGETS ActsPluginOnnx diff --git a/Plugins/Onnx/include/Acts/Plugins/Onnx/OnnxRuntimeBase.hpp b/Plugins/Onnx/include/Acts/Plugins/Onnx/OnnxRuntimeBase.hpp index 86476121187..f1ced286292 100644 --- a/Plugins/Onnx/include/Acts/Plugins/Onnx/OnnxRuntimeBase.hpp +++ b/Plugins/Onnx/include/Acts/Plugins/Onnx/OnnxRuntimeBase.hpp @@ -11,7 +11,7 @@ #include #include -#include +#include namespace Acts { diff --git a/Plugins/Onnx/include/Acts/Plugins/Onnx/SeedClassifier.hpp b/Plugins/Onnx/include/Acts/Plugins/Onnx/SeedClassifier.hpp index d72c48c3497..fd92f6e0ec0 100644 --- a/Plugins/Onnx/include/Acts/Plugins/Onnx/SeedClassifier.hpp +++ b/Plugins/Onnx/include/Acts/Plugins/Onnx/SeedClassifier.hpp @@ -12,7 +12,7 @@ #include -#include +#include namespace Acts { diff --git a/cmake/ActsCreatePackageConfig.cmake b/cmake/ActsCreatePackageConfig.cmake index f2aa6d0edfb..99f63fce86f 100644 --- a/cmake/ActsCreatePackageConfig.cmake +++ b/cmake/ActsCreatePackageConfig.cmake @@ -26,12 +26,6 @@ install( DESTINATION ${install_package_config_dir} ) -# install third party FindXXX.cmake files -install( - FILES ${CMAKE_CURRENT_LIST_DIR}/FindOnnxRuntime.cmake - DESTINATION ${install_package_config_dir}/Modules -) - # install target configs for all available components foreach(_component ${_components}) install( diff --git a/cmake/FindOnnxRuntime.cmake b/cmake/FindOnnxRuntime.cmake deleted file mode 100644 index 512d0e443c0..00000000000 --- a/cmake/FindOnnxRuntime.cmake +++ /dev/null @@ -1,75 +0,0 @@ -# Find the ONNX Runtime include directory and library. -# -# This module defines the `onnxruntime` imported target that encodes all -# necessary information in its target properties. - -find_library( - OnnxRuntime_LIBRARY - NAMES onnxruntime - PATHS ${onnxruntime_DIR} - PATH_SUFFIXES lib lib32 lib64 - DOC "The ONNXRuntime library" -) - -if(NOT OnnxRuntime_LIBRARY) - message(FATAL_ERROR "onnxruntime library not found") -else() - message(STATUS "Found OnnxRuntime library at ${OnnxRuntime_LIBRARY}") -endif() - -find_path( - OnnxRuntime_INCLUDE_DIR - NAMES onnxruntime_cxx_api.h - PATHS ${onnxruntime_DIR} - PATH_SUFFIXES include include/onnxruntime include/onnxruntime/core/session - DOC "The ONNXRuntime include directory" -) - -if(NOT OnnxRuntime_INCLUDE_DIR) - message(FATAL_ERROR "onnxruntime includes not found") -else() - file(READ ${OnnxRuntime_INCLUDE_DIR}/onnxruntime_c_api.h ver) - string(REGEX MATCH "ORT_API_VERSION ([0-9]*)" _ ${ver}) - set(OnnxRuntime_API_VERSION ${CMAKE_MATCH_1}) - message( - STATUS - "Found OnnxRuntime includes at ${OnnxRuntime_INCLUDE_DIR} (API version: ${OnnxRuntime_API_VERSION})" - ) -endif() - -string( - REPLACE - "." - ";" - OnnxRuntime_MIN_VERSION_LIST - ${_acts_onnxruntime_version} -) -list(GET OnnxRuntime_MIN_VERSION_LIST 1 OnnxRuntime_MIN_API_VERSION) -if("${OnnxRuntime_API_VERSION}" LESS ${OnnxRuntime_MIN_API_VERSION}) - message( - FATAL_ERROR - "OnnxRuntime API version ${OnnxRuntime_MIN_API_VERSION} or greater required" - ) -endif() - -include(FindPackageHandleStandardArgs) -find_package_handle_standard_args( - OnnxRuntime - REQUIRED_VARS OnnxRuntime_LIBRARY OnnxRuntime_INCLUDE_DIR -) - -add_library(OnnxRuntime SHARED IMPORTED) -set_property( - TARGET OnnxRuntime - PROPERTY IMPORTED_LOCATION ${OnnxRuntime_LIBRARY} -) -set_property( - TARGET OnnxRuntime - PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${OnnxRuntime_INCLUDE_DIR} -) -set_property( - TARGET OnnxRuntime - PROPERTY INTERFACE_SYSTEM_INCLUDE_DIRECTORIES ${OnnxRuntime_INCLUDE_DIR} -) - -mark_as_advanced(OnnxRuntime_FOUND OnnxRuntime_INCLUDE_DIR OnnxRuntime_LIBRARY)