diff --git a/Core/include/Acts/Navigation/DetectorNavigator.hpp b/Core/include/Acts/Navigation/DetectorNavigator.hpp index 8ba5a1b4000..3f3261d3a5b 100644 --- a/Core/include/Acts/Navigation/DetectorNavigator.hpp +++ b/Core/include/Acts/Navigation/DetectorNavigator.hpp @@ -116,9 +116,9 @@ class DetectorNavigator { return state.navigationBreak; } - void initialize(State& state, const Vector3& position, - const Vector3& direction, - Direction propagationDirection) const { + [[nodiscard]] Result initialize(State& state, const Vector3& position, + const Vector3& direction, + Direction propagationDirection) const { (void)propagationDirection; ACTS_VERBOSE(volInfo(state) << posInfo(state, position) << "initialize"); @@ -140,6 +140,8 @@ class DetectorNavigator { throw std::invalid_argument("DetectorNavigator: no current volume found"); } updateCandidateSurfaces(state, position); + + return Result::success(); } NavigationTarget nextTarget(State& state, const Vector3& position, diff --git a/Core/include/Acts/Propagator/DirectNavigator.hpp b/Core/include/Acts/Propagator/DirectNavigator.hpp index 3c56589f80a..2e34e261316 100644 --- a/Core/include/Acts/Propagator/DirectNavigator.hpp +++ b/Core/include/Acts/Propagator/DirectNavigator.hpp @@ -20,6 +20,7 @@ #include "Acts/Surfaces/Surface.hpp" #include "Acts/Utilities/Intersection.hpp" #include "Acts/Utilities/Logger.hpp" +#include "Acts/Utilities/Result.hpp" #include #include @@ -171,9 +172,9 @@ class DirectNavigator { /// @param position The start position /// @param direction The start direction /// @param propagationDirection The propagation direction - void initialize(State& state, const Vector3& position, - const Vector3& direction, - Direction propagationDirection) const { + [[nodiscard]] Result initialize(State& state, const Vector3& position, + const Vector3& direction, + Direction propagationDirection) const { (void)position; (void)direction; @@ -214,6 +215,8 @@ class DirectNavigator { } state.navigationBreak = false; + + return Result::success(); } /// @brief Get the next target surface diff --git a/Core/include/Acts/Propagator/Navigator.hpp b/Core/include/Acts/Propagator/Navigator.hpp index 7e4954da0fd..47e34f18c61 100644 --- a/Core/include/Acts/Propagator/Navigator.hpp +++ b/Core/include/Acts/Propagator/Navigator.hpp @@ -13,6 +13,7 @@ #include "Acts/Geometry/TrackingGeometry.hpp" #include "Acts/Geometry/TrackingVolume.hpp" #include "Acts/Propagator/NavigationTarget.hpp" +#include "Acts/Propagator/NavigatorError.hpp" #include "Acts/Propagator/NavigatorOptions.hpp" #include "Acts/Propagator/NavigatorStatistics.hpp" #include "Acts/Surfaces/BoundaryTolerance.hpp" @@ -260,17 +261,19 @@ class Navigator { return state.navigationBreak; } - /// @brief Initialize the navigator + /// @brief Initialize the navigator state /// - /// This function initializes the navigator for a new propagation. + /// This function initializes the navigator state for a new propagation. /// /// @param state The navigation state /// @param position The start position /// @param direction The start direction /// @param propagationDirection The propagation direction - void initialize(State& state, const Vector3& position, - const Vector3& direction, - Direction propagationDirection) const { + /// + /// @return Indication if the initialization was successful + [[nodiscard]] Result initialize(State& state, const Vector3& position, + const Vector3& direction, + Direction propagationDirection) const { (void)propagationDirection; ACTS_VERBOSE(volInfo(state) << "Initialization."); @@ -326,9 +329,16 @@ class Navigator { if (state.currentVolume != nullptr) { ACTS_VERBOSE(volInfo(state) << "Start volume resolved " << state.currentVolume->geometryId()); - assert(state.currentVolume->inside(position, - state.options.surfaceTolerance) && - "We did not end up inside the volume."); + + if (!state.currentVolume->inside(position, + state.options.surfaceTolerance)) { + ACTS_DEBUG( + volInfo(state) + << "We did not end up inside the expected volume. position = " + << position.transpose()); + + return Result::failure(NavigatorError::NotInsideExpectedVolume); + } } if (state.currentLayer != nullptr) { ACTS_VERBOSE(volInfo(state) << "Start layer resolved " @@ -337,12 +347,21 @@ class Navigator { if (state.currentSurface != nullptr) { ACTS_VERBOSE(volInfo(state) << "Start surface resolved " << state.currentSurface->geometryId()); - assert(state.currentSurface->isOnSurface( - state.options.geoContext, position, direction, - BoundaryTolerance::Infinite(), - state.options.surfaceTolerance) && - "Stepper not on surface"); + + if (!state.currentSurface->isOnSurface( + state.options.geoContext, position, direction, + BoundaryTolerance::Infinite(), state.options.surfaceTolerance)) { + ACTS_DEBUG(volInfo(state) + << "We did not end up on the expected surface. surface = " + << state.currentSurface->geometryId() + << " position = " << position.transpose() + << " direction = " << direction.transpose()); + + return Result::failure(NavigatorError::NotOnExpectedSurface); + } } + + return Result::success(); } /// @brief Get the next target surface diff --git a/Core/include/Acts/Propagator/NavigatorError.hpp b/Core/include/Acts/Propagator/NavigatorError.hpp new file mode 100644 index 00000000000..0a89e0e6890 --- /dev/null +++ b/Core/include/Acts/Propagator/NavigatorError.hpp @@ -0,0 +1,30 @@ +// 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 +#include + +namespace Acts { + +enum class NavigatorError { + // ensure all values are non-zero + NotInsideExpectedVolume = 1, + NotOnExpectedSurface = 2, +}; + +std::error_code make_error_code(Acts::NavigatorError e); + +} // namespace Acts + +// register with STL +namespace std { +template <> +struct is_error_code_enum : std::true_type {}; +} // namespace std diff --git a/Core/include/Acts/Propagator/Propagator.hpp b/Core/include/Acts/Propagator/Propagator.hpp index ca94137f5ab..3c85c50e09e 100644 --- a/Core/include/Acts/Propagator/Propagator.hpp +++ b/Core/include/Acts/Propagator/Propagator.hpp @@ -279,44 +279,51 @@ class Propagator final /// This function creates the propagator state object from the initial track /// parameters and the propagation options. /// - /// @note This will also initialize the state - /// - /// @tparam parameters_t Type of initial track parameters to propagate /// @tparam propagator_options_t Type of the propagator options /// @tparam path_aborter_t The path aborter type to be added /// - /// @param [in] start Initial track parameters to propagate /// @param [in] options Propagation options /// /// @return Propagator state object - template - auto makeState(const parameters_t& start, - const propagator_options_t& options) const; + auto makeState(const propagator_options_t& options) const; /// @brief Builds the propagator state object /// /// This function creates the propagator state object from the initial track /// parameters, the target surface, and the propagation options. /// - /// @note This will also initialize the state - /// - /// @tparam parameters_t Type of initial track parameters to propagate /// @tparam propagator_options_t Type of the propagator options /// @tparam target_aborter_t The target aborter type to be added /// @tparam path_aborter_t The path aborter type to be added /// - /// @param [in] start Initial track parameters to propagate /// @param [in] target Target surface of to propagate to /// @param [in] options Propagation options /// /// @return Propagator state object - template - auto makeState(const parameters_t& start, const Surface& target, + auto makeState(const Surface& target, const propagator_options_t& options) const; + /// @brief Initialize the propagator state + /// + /// This function initializes the propagator state for a new propagation. + /// + /// @tparam propagator_state_t Type of the propagator state object + /// @tparam path_aborter_t The path aborter type to be added + /// + /// @param [in,out] state The propagator state object + /// @param [in] start Initial track parameters to propagate + /// + /// @return Indication if the initialization was successful + template + [[nodiscard]] Result initialize(propagator_state_t& state, + const parameters_t& start) const; + /// @brief Propagate track parameters /// /// This function performs the propagation of the track parameters according @@ -386,10 +393,6 @@ class Propagator final private: const Logger& logger() const { return *m_logger; } - template - void initialize(propagator_state_t& state, const parameters_t& start) const; - template void moveStateToResult(propagator_state_t& state, result_t& result) const; diff --git a/Core/include/Acts/Propagator/Propagator.ipp b/Core/include/Acts/Propagator/Propagator.ipp index 09b69be47ca..ef1a6383461 100644 --- a/Core/include/Acts/Propagator/Propagator.ipp +++ b/Core/include/Acts/Propagator/Propagator.ipp @@ -28,8 +28,8 @@ concept propagator_stepper_compatible_with = template template -auto Acts::Propagator::propagate(propagator_state_t& state) const - -> Result { +Acts::Result Acts::Propagator::propagate( + propagator_state_t& state) const { ACTS_VERBOSE("Entering propagation."); state.stage = PropagatorStage::prePropagation; @@ -196,7 +196,13 @@ auto Acts::Propagator::propagate(const parameters_t& start, static_assert(std::copy_constructible, "return track parameter type must be copy-constructible"); - auto state = makeState(start, options); + auto state = makeState(options); + + auto initRes = + initialize(state, start); + if (!initRes.ok()) { + return initRes.error(); + } // Perform the actual propagation auto propagationResult = propagate(state); @@ -217,8 +223,15 @@ auto Acts::Propagator::propagate( static_assert(BoundTrackParametersConcept, "Parameters do not fulfill bound parameters concept."); - auto state = makeState(start, target, options); + auto state = + makeState( + target, options); + + auto initRes = + initialize(state, start); + if (!initRes.ok()) { + return initRes.error(); + } // Perform the actual propagation auto propagationResult = propagate(state); @@ -227,86 +240,107 @@ auto Acts::Propagator::propagate( } template -template +template auto Acts::Propagator::makeState( - const parameters_t& start, const propagator_options_t& options) const { - static_assert(BoundTrackParametersConcept, - "Parameters do not fulfill bound parameters concept."); - + const propagator_options_t& options) const { // Type of track parameters produced by the propagation using ReturnParameterType = StepperCurvilinearTrackParameters; static_assert(std::copy_constructible, "return track parameter type must be copy-constructible"); - // Expand the abort list with a path aborter + // Expand the actor list with a path aborter path_aborter_t pathAborter; pathAborter.internalLimit = options.pathLimit; auto actorList = options.actorList.append(pathAborter); - // The expanded options (including path limit) + // Create the extended options and declare their type auto eOptions = options.extend(actorList); - eOptions.navigation.startSurface = &start.referenceSurface(); - eOptions.navigation.targetSurface = nullptr; + using OptionsType = decltype(eOptions); using StateType = actor_list_t_state_t; - // Initialize the internal propagator state - StateType state{eOptions, m_stepper.makeState(eOptions.stepping), - m_navigator.makeState(eOptions.navigation)}; static_assert( detail::propagator_stepper_compatible_with, "Step method of the Stepper is not compatible with the propagator " "state"); - initialize(state, start); + // Initialize the internal propagator state + StateType state{eOptions, m_stepper.makeState(eOptions.stepping), + m_navigator.makeState(eOptions.navigation)}; return state; } template -template +template auto Acts::Propagator::makeState( - const parameters_t& start, const Surface& target, - const propagator_options_t& options) const { - static_assert(BoundTrackParametersConcept, - "Parameters do not fulfill bound parameters concept."); - - // Type of provided options + const Surface& target, const propagator_options_t& options) const { + // Expand the actor list with a target and path aborter target_aborter_t targetAborter; targetAborter.surface = ⌖ path_aborter_t pathAborter; pathAborter.internalLimit = options.pathLimit; + auto actorList = options.actorList.append(targetAborter, pathAborter); // Create the extended options and declare their type auto eOptions = options.extend(actorList); - eOptions.navigation.startSurface = &start.referenceSurface(); eOptions.navigation.targetSurface = ⌖ - using OptionsType = decltype(eOptions); - // Initialize the internal propagator state + using OptionsType = decltype(eOptions); using StateType = actor_list_t_state_t; - StateType state{eOptions, m_stepper.makeState(eOptions.stepping), - m_navigator.makeState(eOptions.navigation)}; static_assert( detail::propagator_stepper_compatible_with, "Step method of the Stepper is not compatible with the propagator " "state"); - initialize(state, start); + // Initialize the internal propagator state + StateType state{eOptions, m_stepper.makeState(eOptions.stepping), + m_navigator.makeState(eOptions.navigation)}; return state; } +template +template +Acts::Result Acts::Propagator::initialize( + propagator_state_t& state, const parameters_t& start) const { + static_assert(BoundTrackParametersConcept, + "Parameters do not fulfill bound parameters concept."); + + m_stepper.initialize(state.stepping, start); + + state.position = m_stepper.position(state.stepping); + state.direction = + state.options.direction * m_stepper.direction(state.stepping); + + state.navigation.options.startSurface = &start.referenceSurface(); + + // Navigator initialize state call + auto navInitRes = + m_navigator.initialize(state.navigation, state.position, state.direction, + state.options.direction); + if (!navInitRes.ok()) { + return navInitRes.error(); + } + + // Apply the loop protection - it resets the internal path limit + detail::setupLoopProtection( + state, m_stepper, state.options.actorList.template get(), + false, logger()); + + return Result::success(); +} + template template auto Acts::Propagator::makeResult(propagator_state_t state, @@ -395,27 +429,6 @@ auto Acts::Propagator::makeResult( return Result::success(std::move(result)); } -template -template -void Acts::Propagator::initialize(propagator_state_t& state, - const parameters_t& start) const { - m_stepper.initialize(state.stepping, start); - - state.position = m_stepper.position(state.stepping); - state.direction = - state.options.direction * m_stepper.direction(state.stepping); - - // Navigator initialize state call - m_navigator.initialize(state.navigation, state.position, state.direction, - state.options.direction); - - // Apply the loop protection - it resets the internal path limit - detail::setupLoopProtection( - state, m_stepper, state.options.actorList.template get(), - false, logger()); -} - template template void Acts::Propagator::moveStateToResult(propagator_state_t& state, @@ -457,12 +470,12 @@ Acts::detail::BasePropagatorHelper::propagateToSurface( start, target, derivedOptions); } - if (res.ok()) { - // Without errors we can expect a valid endParameters when propagating to a - // target surface - assert((*res).endParameters); - return std::move((*res).endParameters.value()); - } else { + if (!res.ok()) { return res.error(); } + + // Without errors we can expect a valid endParameters when propagating to a + // target surface + assert((*res).endParameters); + return std::move((*res).endParameters.value()); } diff --git a/Core/include/Acts/Propagator/TryAllNavigator.hpp b/Core/include/Acts/Propagator/TryAllNavigator.hpp index 995e46df857..a01ba565f31 100644 --- a/Core/include/Acts/Propagator/TryAllNavigator.hpp +++ b/Core/include/Acts/Propagator/TryAllNavigator.hpp @@ -154,9 +154,9 @@ class TryAllNavigatorBase { /// @param position The starting position /// @param direction The starting direction /// @param propagationDirection The propagation direction - void initialize(State& state, const Vector3& position, - const Vector3& direction, - Direction propagationDirection) const { + [[nodiscard]] Result initialize(State& state, const Vector3& position, + const Vector3& direction, + Direction propagationDirection) const { (void)propagationDirection; ACTS_VERBOSE("initialize"); @@ -198,6 +198,8 @@ class TryAllNavigatorBase { ACTS_VERBOSE(volInfo(state) << "No start surface set."); } } + + return Result::success(); } protected: @@ -284,14 +286,19 @@ class TryAllNavigator : public TryAllNavigatorBase { /// @param position The starting position /// @param direction The starting direction /// @param propagationDirection The propagation direction - void initialize(State& state, const Vector3& position, - const Vector3& direction, - Direction propagationDirection) const { - TryAllNavigatorBase::initialize(state, position, direction, - propagationDirection); + [[nodiscard]] Result initialize(State& state, const Vector3& position, + const Vector3& direction, + Direction propagationDirection) const { + auto baseRes = TryAllNavigatorBase::initialize(state, position, direction, + propagationDirection); + if (!baseRes.ok()) { + return baseRes.error(); + } // Initialize navigation candidates for the start volume reinitializeCandidates(state); + + return Result::success(); } /// @brief Get the next target surface @@ -584,9 +591,6 @@ class TryAllOverstepNavigator : public TryAllNavigatorBase { State makeState(const Options& options) const { State state(options); - state.startSurface = options.startSurface; - state.targetSurface = options.targetSurface; - return state; } @@ -607,16 +611,21 @@ class TryAllOverstepNavigator : public TryAllNavigatorBase { /// @param position The starting position /// @param direction The starting direction /// @param propagationDirection The propagation direction - void initialize(State& state, const Vector3& position, - const Vector3& direction, - Direction propagationDirection) const { - TryAllNavigatorBase::initialize(state, position, direction, - propagationDirection); + [[nodiscard]] Result initialize(State& state, const Vector3& position, + const Vector3& direction, + Direction propagationDirection) const { + auto baseRes = TryAllNavigatorBase::initialize(state, position, direction, + propagationDirection); + if (!baseRes.ok()) { + return baseRes.error(); + } // Initialize navigation candidates for the start volume reinitializeCandidates(state); state.lastPosition.reset(); + + return Result::success(); } /// @brief Get the next target surface diff --git a/Core/include/Acts/Propagator/VoidNavigator.hpp b/Core/include/Acts/Propagator/VoidNavigator.hpp index d91b75450ec..66a4a0c875e 100644 --- a/Core/include/Acts/Propagator/VoidNavigator.hpp +++ b/Core/include/Acts/Propagator/VoidNavigator.hpp @@ -13,6 +13,7 @@ #include "Acts/Propagator/NavigationTarget.hpp" #include "Acts/Propagator/NavigatorOptions.hpp" #include "Acts/Propagator/NavigatorStatistics.hpp" +#include "Acts/Utilities/Result.hpp" namespace Acts { @@ -62,10 +63,10 @@ class VoidNavigator { bool navigationBreak(const State& /*state*/) const { return true; } - void initialize(State& /*state*/, const Vector3& /*position*/, - const Vector3& /*direction*/, - Direction /*propagationDirection*/) const { - return; + [[nodiscard]] Result initialize( + State& /*state*/, const Vector3& /*position*/, + const Vector3& /*direction*/, Direction /*propagationDirection*/) const { + return Result::success(); } NavigationTarget nextTarget(State& /*state*/, const Vector3& /*position*/, diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index 3b1d92d24b0..5acda71be82 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -377,13 +377,15 @@ class CombinatorialKalmanFilter { // Reset the navigation state // Set targetSurface to nullptr for forward filtering - auto navigationOptions = state.navigation.options; - navigationOptions.startSurface = ¤tState.referenceSurface(); - navigationOptions.targetSurface = nullptr; - state.navigation = navigator.makeState(navigationOptions); - navigator.initialize(state.navigation, stepper.position(state.stepping), - stepper.direction(state.stepping), - state.options.direction); + state.navigation.options.startSurface = ¤tState.referenceSurface(); + state.navigation.options.targetSurface = nullptr; + auto navInitRes = navigator.initialize( + state.navigation, stepper.position(state.stepping), + stepper.direction(state.stepping), state.options.direction); + if (!navInitRes.ok()) { + ACTS_ERROR("Navigation initialization failed: " << navInitRes.error()); + result.lastError = navInitRes.error(); + } // No Kalman filtering for the starting surface, but still need // to consider the material effects here @@ -871,9 +873,17 @@ class CombinatorialKalmanFilter { combKalmanActor.m_extensions = tfOptions.extensions; auto propState = - m_propagator.template makeState(initialParameters, - propOptions); + m_propagator + .template makeState( + propOptions); + + auto initResult = m_propagator.template initialize< + decltype(propState), start_parameters_t, StubPathLimitReached>( + propState, initialParameters); + if (!initResult.ok()) { + ACTS_ERROR("Propagation initialization failed: " << initResult.error()); + return initResult.error(); + } auto& r = propState diff --git a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp index fcada49abc2..2bf0b0b5978 100644 --- a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp +++ b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp @@ -264,7 +264,21 @@ struct GaussianSumFitter { params = sParameters; } - auto state = m_propagator.makeState(*params, fwdPropOptions); + auto state = m_propagator.makeState(fwdPropOptions); + + // Type deduction for propagation result to pass on errors + using OptionsType = decltype(fwdPropOptions); + using StateType = decltype(state); + using PropagationResultType = + decltype(m_propagator.propagate(std::declval())); + using ResultType = decltype(m_propagator.makeResult( + std::declval(), std::declval(), + std::declval(), false)); + + auto initRes = m_propagator.initialize(state, *params); + if (!initRes.ok()) { + return ResultType::failure(initRes.error()); + } auto& r = state.template get(); r.fittedStates = &trackContainer.trackStateContainer(); @@ -321,11 +335,23 @@ struct GaussianSumFitter { : sParameters.referenceSurface(); const auto& params = *fwdGsfResult.lastMeasurementState; - auto state = - m_propagator.template makeState( - params, target, bwdPropOptions); + auto state = m_propagator.template makeState( + target, bwdPropOptions); + + // Type deduction for propagation result to pass on errors + using OptionsType = decltype(bwdPropOptions); + using StateType = decltype(state); + using PropagationResultType = + decltype(m_propagator.propagate(std::declval())); + using ResultType = decltype(m_propagator.makeResult( + std::declval(), std::declval(), + target, std::declval())); + + auto initRes = m_propagator.initialize(state, params); + if (!initRes.ok()) { + return ResultType::failure(initRes.error()); + } assert( (fwdGsfResult.lastMeasurementTip != MultiTrajectoryTraits::kInvalid && diff --git a/Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp b/Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp index 5e46bc0dba3..72887f04842 100644 --- a/Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp +++ b/Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp @@ -14,25 +14,20 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/EventData/MeasurementHelpers.hpp" #include "Acts/EventData/MultiTrajectory.hpp" -#include "Acts/EventData/MultiTrajectoryHelpers.hpp" #include "Acts/EventData/SourceLink.hpp" -#include "Acts/EventData/TrackContainerFrontendConcept.hpp" #include "Acts/EventData/TrackParameters.hpp" #include "Acts/EventData/TrackProxyConcept.hpp" #include "Acts/EventData/VectorMultiTrajectory.hpp" #include "Acts/EventData/VectorTrackContainer.hpp" +#include "Acts/EventData/detail/CorrectedTransformationFreeToBound.hpp" #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/Geometry/TrackingVolume.hpp" #include "Acts/MagneticField/MagneticFieldContext.hpp" #include "Acts/Material/Interactions.hpp" -#include "Acts/Material/MaterialSlab.hpp" #include "Acts/Propagator/ActorList.hpp" -#include "Acts/Propagator/ConstrainedStep.hpp" #include "Acts/Propagator/DirectNavigator.hpp" -#include "Acts/Propagator/Navigator.hpp" -#include "Acts/Propagator/Propagator.hpp" +#include "Acts/Propagator/PropagatorOptions.hpp" #include "Acts/Propagator/StandardAborters.hpp" -#include "Acts/Propagator/StraightLineStepper.hpp" #include "Acts/Propagator/detail/PointwiseMaterialInteraction.hpp" #include "Acts/TrackFitting/GlobalChiSquareFitterError.hpp" #include "Acts/TrackFitting/detail/VoidFitterComponents.hpp" @@ -1256,7 +1251,15 @@ class Gx2Fitter { gx2fActor.scatteringMap = &scatteringMap; gx2fActor.parametersWithHypothesis = ¶ms; - auto propagatorState = m_propagator.makeState(params, propagatorOptions); + auto propagatorState = m_propagator.makeState(propagatorOptions); + + auto propagatorInitResult = + m_propagator.initialize(propagatorState, params); + if (!propagatorInitResult.ok()) { + ACTS_ERROR("Propagation initialization failed: " + << propagatorInitResult.error()); + return propagatorInitResult.error(); + } auto& r = propagatorState.template get>(); r.fittedStates = &trajectoryTempBackend; @@ -1421,7 +1424,15 @@ class Gx2Fitter { gx2fActor.scatteringMap = &scatteringMap; gx2fActor.parametersWithHypothesis = ¶ms; - auto propagatorState = m_propagator.makeState(params, propagatorOptions); + auto propagatorState = m_propagator.makeState(propagatorOptions); + + auto propagatorInitResult = + m_propagator.initialize(propagatorState, params); + if (!propagatorInitResult.ok()) { + ACTS_ERROR("Propagation initialization failed: " + << propagatorInitResult.error()); + return propagatorInitResult.error(); + } auto& r = propagatorState.template get>(); r.fittedStates = &trajectoryTempBackend; @@ -1569,7 +1580,15 @@ class Gx2Fitter { gx2fActor.scatteringMap = &scatteringMap; gx2fActor.parametersWithHypothesis = ¶ms; - auto propagatorState = m_propagator.makeState(params, propagatorOptions); + auto propagatorState = m_propagator.makeState(propagatorOptions); + + auto propagatorInitResult = + m_propagator.initialize(propagatorState, params); + if (!propagatorInitResult.ok()) { + ACTS_ERROR("Propagation initialization failed: " + << propagatorInitResult.error()); + return propagatorInitResult.error(); + } auto& r = propagatorState.template get>(); r.fittedStates = &trackContainer.trackStateContainer(); diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp index 50e4c91c138..bb940acbdb4 100644 --- a/Core/include/Acts/TrackFitting/KalmanFitter.hpp +++ b/Core/include/Acts/TrackFitting/KalmanFitter.hpp @@ -557,13 +557,14 @@ class KalmanFitter { // Reset navigation state // We do not need to specify a target here since this will be handled // separately in the KF actor - auto navigationOptions = state.navigation.options; - navigationOptions.startSurface = &st.referenceSurface(); - navigationOptions.targetSurface = nullptr; - state.navigation = navigator.makeState(navigationOptions); - navigator.initialize(state.navigation, stepper.position(state.stepping), - stepper.direction(state.stepping), - state.options.direction); + state.navigation.options.startSurface = &st.referenceSurface(); + state.navigation.options.targetSurface = nullptr; + auto navInitRes = navigator.initialize( + state.navigation, stepper.position(state.stepping), + stepper.direction(state.stepping), state.options.direction); + if (!navInitRes.ok()) { + return navInitRes.error(); + } // Update material effects for last measurement state in reversed // direction @@ -1045,9 +1046,12 @@ class KalmanFitter { navigationOptions.startSurface = &surface; navigationOptions.targetSurface = nullptr; state.navigation = navigator.makeState(navigationOptions); - navigator.initialize(state.navigation, stepper.position(state.stepping), - stepper.direction(state.stepping), - state.options.direction); + auto navInitRes = navigator.initialize( + state.navigation, stepper.position(state.stepping), + stepper.direction(state.stepping), state.options.direction); + if (!navInitRes.ok()) { + return navInitRes.error(); + } return Result::success(); } @@ -1235,8 +1239,15 @@ class KalmanFitter { const propagator_options_t& propagatorOptions, track_container_t& trackContainer) const -> Result { - auto propagatorState = - m_propagator.makeState(sParameters, propagatorOptions); + auto propagatorState = m_propagator.makeState(propagatorOptions); + + auto propagatorInitResult = + m_propagator.initialize(propagatorState, sParameters); + if (!propagatorInitResult.ok()) { + ACTS_ERROR("Propagation initialization failed: " + << propagatorInitResult.error()); + return propagatorInitResult.error(); + } auto& kalmanResult = propagatorState.template get>(); diff --git a/Core/include/Acts/Utilities/Result.hpp b/Core/include/Acts/Utilities/Result.hpp index c71836cc788..0b6f64dda31 100644 --- a/Core/include/Acts/Utilities/Result.hpp +++ b/Core/include/Acts/Utilities/Result.hpp @@ -413,8 +413,24 @@ class Result { /// @return Reference to the error E error() && noexcept { return std::move(m_opt.value()); } + void value() const { checkValueAccess(); } + private: std::optional m_opt; + + void checkValueAccess() const { + if (m_opt.has_value()) { + if constexpr (std::is_same_v) { + std::stringstream ss; + const auto& e = m_opt.value(); + ss << "Value called on error value: " << e.category().name() << ": " + << e.message() << " [" << e.value() << "]"; + throw std::runtime_error(ss.str()); + } else { + throw std::runtime_error("Value called on error value"); + } + } + } }; } // namespace Acts diff --git a/Core/src/Propagator/CMakeLists.txt b/Core/src/Propagator/CMakeLists.txt index b179a33e3c2..26df8fe76ef 100644 --- a/Core/src/Propagator/CMakeLists.txt +++ b/Core/src/Propagator/CMakeLists.txt @@ -3,6 +3,7 @@ target_sources( PRIVATE EigenStepperError.cpp MultiStepperError.cpp + NavigatorError.cpp SympyStepper.cpp PropagatorError.cpp StraightLineStepper.cpp diff --git a/Core/src/Propagator/NavigatorError.cpp b/Core/src/Propagator/NavigatorError.cpp new file mode 100644 index 00000000000..4c29e6882e5 --- /dev/null +++ b/Core/src/Propagator/NavigatorError.cpp @@ -0,0 +1,40 @@ +// 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/NavigatorError.hpp" + +#include + +namespace { + +class NavigatorErrorCategory : public std::error_category { + public: + // Return a short descriptive name for the category. + const char* name() const noexcept final { return "NavigatorError"; } + + // Return what each enum means in text. + std::string message(int c) const final { + using Acts::NavigatorError; + + switch (static_cast(c)) { + case NavigatorError::NotInsideExpectedVolume: + return "We did not end up inside the volume."; + case NavigatorError::NotOnExpectedSurface: + return "Stepper not on surface"; + default: + return "unknown"; + } + } +}; + +} // namespace + +std::error_code Acts::make_error_code(Acts::NavigatorError e) { + static NavigatorErrorCategory c; + return {static_cast(e), c}; +} diff --git a/Examples/Algorithms/Propagation/include/ActsExamples/Propagation/PropagatorInterface.hpp b/Examples/Algorithms/Propagation/include/ActsExamples/Propagation/PropagatorInterface.hpp index 7f113d47b56..8baa4a386d9 100644 --- a/Examples/Algorithms/Propagation/include/ActsExamples/Propagation/PropagatorInterface.hpp +++ b/Examples/Algorithms/Propagation/include/ActsExamples/Propagation/PropagatorInterface.hpp @@ -90,7 +90,12 @@ class ConcretePropagator : public PropagatorInterface { // Set a maximum step size options.stepping.maxStepSize = cfg.maxStepSize; - auto state = m_propagator.makeState(startParameters, options); + auto state = m_propagator.makeState(options); + + auto resultInit = m_propagator.initialize(state, startParameters); + if (!resultInit.ok()) { + return resultInit.error(); + } // Propagate using the propagator auto resultTmp = m_propagator.propagate(state); diff --git a/Tests/Benchmarks/StepperBenchmarkCommons.hpp b/Tests/Benchmarks/StepperBenchmarkCommons.hpp index 206f1479837..18cb7c01917 100644 --- a/Tests/Benchmarks/StepperBenchmarkCommons.hpp +++ b/Tests/Benchmarks/StepperBenchmarkCommons.hpp @@ -117,7 +117,12 @@ struct BenchmarkStepper { std::size_t numIters = 0; const auto propagationBenchResult = Acts::Test::microBenchmark( [&] { - auto state = propagator.makeState(pars, options); + auto state = propagator.makeState(options); + auto initRes = propagator.initialize(state, pars); + if (!initRes.ok()) { + ACTS_ERROR("initialization failed: " << initRes.error()); + return; + } auto tmp = propagator.propagate(state); auto r = propagator.makeResult(state, tmp, options, true).value(); if (totalPathLength == 0.) { @@ -129,7 +134,6 @@ struct BenchmarkStepper { numSteps += r.steps; numStepTrials += state.stepping.nStepTrials; ++numIters; - return r; }, 1, toys); diff --git a/Tests/UnitTests/Core/Navigation/DetectorNavigatorTests.cpp b/Tests/UnitTests/Core/Navigation/DetectorNavigatorTests.cpp index c69bd100ea9..f801939c418 100644 --- a/Tests/UnitTests/Core/Navigation/DetectorNavigatorTests.cpp +++ b/Tests/UnitTests/Core/Navigation/DetectorNavigatorTests.cpp @@ -97,7 +97,9 @@ BOOST_AUTO_TEST_CASE(DetectorNavigatorTestsInitialization) { Propagator propagator(stepper, navigator); - BOOST_CHECK_THROW(propagator.makeState(start, options), + auto state = propagator.makeState(options); + + BOOST_CHECK_THROW(propagator.initialize(state, start).value(), std::invalid_argument); } @@ -115,11 +117,16 @@ BOOST_AUTO_TEST_CASE(DetectorNavigatorTestsInitialization) { Acts::Experimental::DetectorNavigator> propagator(stepper, navigator); - auto state = propagator.makeState(start, options); + auto state = propagator.makeState(options); + + stepper.initialize(state.stepping, start); - navigator.initialize(state.navigation, stepper.position(state.stepping), - stepper.direction(state.stepping), - state.options.direction); + BOOST_CHECK(navigator + .initialize(state.navigation, + stepper.position(state.stepping), + stepper.direction(state.stepping), + state.options.direction) + .ok()); navigator.nextTarget(state.navigation, stepper.position(state.stepping), stepper.direction(state.stepping)); @@ -147,7 +154,9 @@ BOOST_AUTO_TEST_CASE(DetectorNavigatorTestsInitialization) { Acts::Experimental::DetectorNavigator> propagator(stepper, navigator); - BOOST_CHECK_THROW(propagator.makeState(startEoW, options), + auto state = propagator.makeState(options); + + BOOST_CHECK_THROW(propagator.initialize(state, startEoW).value(), std::invalid_argument); } @@ -162,11 +171,16 @@ BOOST_AUTO_TEST_CASE(DetectorNavigatorTestsInitialization) { Acts::Experimental::DetectorNavigator> propagator(stepper, navigator); - auto state = propagator.makeState(start, options); + auto state = propagator.makeState(options); + + stepper.initialize(state.stepping, start); - navigator.initialize(state.navigation, stepper.position(state.stepping), - stepper.direction(state.stepping), - state.options.direction); + BOOST_CHECK(navigator + .initialize(state.navigation, + stepper.position(state.stepping), + stepper.direction(state.stepping), + state.options.direction) + .ok()); auto initState = state.navigation; BOOST_CHECK_EQUAL(initState.currentDetector, detector.get()); @@ -175,7 +189,7 @@ BOOST_AUTO_TEST_CASE(DetectorNavigatorTestsInitialization) { detector->findDetectorVolume(geoContext, start.position())); BOOST_CHECK_EQUAL(initState.currentSurface, nullptr); BOOST_CHECK_EQUAL(initState.currentPortal, nullptr); - BOOST_CHECK_EQUAL(initState.surfaceCandidates.size(), 2u); + BOOST_CHECK_EQUAL(initState.surfaceCandidates.size(), 1u); } } diff --git a/Tests/UnitTests/Core/Propagator/NavigatorTests.cpp b/Tests/UnitTests/Core/Propagator/NavigatorTests.cpp index 5bf278d679c..5bc893bc802 100644 --- a/Tests/UnitTests/Core/Propagator/NavigatorTests.cpp +++ b/Tests/UnitTests/Core/Propagator/NavigatorTests.cpp @@ -190,7 +190,9 @@ BOOST_AUTO_TEST_CASE(Navigator_status_methods) { const Layer* startLay = startVol->associatedLayer(tgContext, position); state.options.startSurface = startSurf; state.options.targetSurface = startSurf; - navigator.initialize(state, position, direction, Direction::Forward()); + BOOST_CHECK( + navigator.initialize(state, position, direction, Direction::Forward()) + .ok()); BOOST_CHECK(testNavigatorStateVectors(state, 0u, 0u, 0u)); BOOST_CHECK(testNavigatorStatePointers(state, startVol, startLay, startSurf, startSurf, startVol, startSurf)); @@ -201,7 +203,9 @@ BOOST_AUTO_TEST_CASE(Navigator_status_methods) { position = Vector3::Zero(); startVol = tGeometry->lowestTrackingVolume(tgContext, position); startLay = startVol->associatedLayer(tgContext, position); - navigator.initialize(state, position, direction, Direction::Forward()); + BOOST_CHECK( + navigator.initialize(state, position, direction, Direction::Forward()) + .ok()); BOOST_CHECK(testNavigatorStateVectors(state, 0u, 0u, 0u)); BOOST_CHECK(testNavigatorStatePointers(state, startVol, startLay, nullptr, nullptr, startVol, nullptr)); @@ -209,7 +213,9 @@ BOOST_AUTO_TEST_CASE(Navigator_status_methods) { ACTS_INFO(" b) Initialise having a start surface"); state = navigator.makeState(options); state.options.startSurface = startSurf; - navigator.initialize(state, position, direction, Direction::Forward()); + BOOST_CHECK( + navigator.initialize(state, position, direction, Direction::Forward()) + .ok()); BOOST_CHECK(testNavigatorStateVectors(state, 0u, 0u, 0u)); BOOST_CHECK(testNavigatorStatePointers(state, startVol, startLay, startSurf, startSurf, startVol, nullptr)); @@ -217,7 +223,9 @@ BOOST_AUTO_TEST_CASE(Navigator_status_methods) { ACTS_INFO(" c) Initialise having a start volume"); state = navigator.makeState(options); state.startVolume = startVol; - navigator.initialize(state, position, direction, Direction::Forward()); + BOOST_CHECK( + navigator.initialize(state, position, direction, Direction::Forward()) + .ok()); BOOST_CHECK(testNavigatorStateVectors(state, 0u, 0u, 0u)); BOOST_CHECK(testNavigatorStatePointers(state, startVol, startLay, nullptr, nullptr, startVol, nullptr)); @@ -249,7 +257,9 @@ BOOST_AUTO_TEST_CASE(Navigator_target_methods) { // (1) Initialization navigation from start point // - this will call resolveLayers() as well // - and thus should call a return to the stepper - navigator.initialize(state, position, direction, Direction::Forward()); + BOOST_CHECK( + navigator.initialize(state, position, direction, Direction::Forward()) + .ok()); // Check that the currentVolume is set BOOST_CHECK_NE(state.currentVolume, nullptr); // Check that the currentVolume is the startVolume