From a3c9a1a8aaab5efb93b9fc16307016e5e46b557a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 10 May 2024 13:47:33 +0200 Subject: [PATCH] refactor!: Refactor navigator options --- .../Acts/Navigation/DetectorNavigator.hpp | 20 ++--- .../Acts/Propagator/DirectNavigator.hpp | 82 ++++++------------- Core/include/Acts/Propagator/Navigator.hpp | 12 +-- .../Acts/Propagator/NavigatorOptions.hpp | 7 +- Core/include/Acts/Propagator/Propagator.ipp | 10 ++- .../include/Acts/Propagator/VoidNavigator.hpp | 55 ++++--------- .../CombinatorialKalmanFilter.hpp | 6 +- .../Acts/TrackFitting/KalmanFitter.hpp | 10 ++- 8 files changed, 80 insertions(+), 122 deletions(-) diff --git a/Core/include/Acts/Navigation/DetectorNavigator.hpp b/Core/include/Acts/Navigation/DetectorNavigator.hpp index 0b2433c8982..29d80b01275 100644 --- a/Core/include/Acts/Navigation/DetectorNavigator.hpp +++ b/Core/include/Acts/Navigation/DetectorNavigator.hpp @@ -59,12 +59,10 @@ class DetectorNavigator { /// created for every propagation/extrapolation step /// and keep thread-local navigation information struct State : public NavigationState { - /// Navigation state - external state: the start surface - const Surface* startSurface = nullptr; + Options options; + /// Navigation state - external state: the current surface const Surface* currentSurface = nullptr; - /// Navigation state - external state: the target surface - const Surface* targetSurface = nullptr; /// Indicator if the target is reached bool targetReached = false; /// Navigation state : a break has been detected @@ -81,12 +79,10 @@ class DetectorNavigator { Logging::Level::INFO)) : m_cfg{cfg}, m_logger{std::move(_logger)} {} - State makeState(const Surface* startSurface, - const Surface* targetSurface) const { - State result; - result.startSurface = startSurface; - result.targetSurface = targetSurface; - return result; + State makeState(const Options& options) const { + State state; + state.options = options; + return state; } const Surface* currentSurface(const State& state) const { @@ -102,11 +98,11 @@ class DetectorNavigator { } const Surface* startSurface(const State& state) const { - return state.startSurface; + return state.options.startSurface; } const Surface* targetSurface(const State& state) const { - return state.targetSurface; + return state.options.targetSurface; } bool targetReached(const State& state) const { return state.targetReached; } diff --git a/Core/include/Acts/Propagator/DirectNavigator.hpp b/Core/include/Acts/Propagator/DirectNavigator.hpp index ad0a3ac5365..1f6a5772639 100644 --- a/Core/include/Acts/Propagator/DirectNavigator.hpp +++ b/Core/include/Acts/Propagator/DirectNavigator.hpp @@ -38,6 +38,12 @@ class DirectNavigator { struct Config {}; + struct Options : public NavigatorPlainOptions { + void setPlainOptions(const NavigatorPlainOptions& options) { + static_cast(*this) = options; + } + }; + /// @brief Nested Actor struct, called Initializer /// /// This is needed for the initialization of the surface sequence. @@ -76,10 +82,10 @@ class DirectNavigator { // In case the start surface is in the list of nav surfaces // we need to correct the iterator to point to the next surface // in the vector - if (state.navigation.startSurface) { + if (state.navigation.options.startSurface) { auto surfaceIter = std::find(state.navigation.navSurfaces.begin(), state.navigation.navSurfaces.end(), - state.navigation.startSurface); + state.navigation.options.startSurface); // if current surface in the list, point to the next surface if (surfaceIter != state.navigation.navSurfaces.end()) { state.navigation.navSurfaceIter = ++surfaceIter; @@ -97,28 +103,16 @@ class DirectNavigator { /// propagation/extrapolation step and keep thread-local navigation /// information struct State { + Options options; + /// Externally provided surfaces - expected to be ordered along the path SurfaceSequence navSurfaces = {}; /// Iterator the next surface SurfaceIter navSurfaceIter = navSurfaces.begin(); - /// Navigation state - external interface: the start surface - const Surface* startSurface = nullptr; /// Navigation state - external interface: the current surface const Surface* currentSurface = nullptr; - /// Navigation state - external interface: the target surface - const Surface* targetSurface = nullptr; - /// Navigation state - starting layer - const Layer* startLayer = nullptr; - /// Navigation state - target layer - const Layer* targetLayer = nullptr; - /// Navigation state: the start volume - const TrackingVolume* startVolume = nullptr; - /// Navigation state: the current volume - const TrackingVolume* currentVolume = nullptr; - /// Navigation state: the target volume - const TrackingVolume* targetVolume = nullptr; /// Navigation state - external interface: target is reached bool targetReached = false; @@ -126,52 +120,39 @@ class DirectNavigator { bool navigationBreak = false; }; - struct Options : public NavigatorPlainOptions { - void setPlainOptions(const NavigatorPlainOptions& options) { - static_cast(*this) = options; - } - }; - DirectNavigator(std::unique_ptr _logger = getDefaultLogger("DirectNavigator", Logging::INFO)) : m_logger{std::move(_logger)} {} - State makeState(const Surface* startSurface, - const Surface* targetSurface) const { - State result; - result.startSurface = startSurface; - result.targetSurface = targetSurface; - return result; + State makeState(const Options& options) const { + State state; + state.options = options; + return state; } const Surface* currentSurface(const State& state) const { return state.currentSurface; } - const TrackingVolume* currentVolume(const State& state) const { - return state.currentVolume; + const TrackingVolume* currentVolume(const State& /*state*/) const { + return nullptr; } - const IVolumeMaterial* currentVolumeMaterial(const State& state) const { - if (state.currentVolume == nullptr) { - return nullptr; - } - return state.currentVolume->volumeMaterial(); + const IVolumeMaterial* currentVolumeMaterial(const State& /*state*/) const { + return nullptr; } const Surface* startSurface(const State& state) const { - return state.startSurface; + return state.options.startSurface; } const Surface* targetSurface(const State& state) const { - return state.targetSurface; + return state.options.targetSurface; } bool targetReached(const State& state) const { return state.targetReached; } - bool endOfWorldReached(State& state) const { - return state.currentVolume == nullptr; - } + bool endOfWorldReached(State& /*state*/) const { return false; } bool navigationBreak(const State& state) const { return state.navigationBreak; @@ -198,13 +179,12 @@ class DirectNavigator { template void initialize(propagator_state_t& state, const stepper_t& /*stepper*/) const { - ACTS_VERBOSE(volInfo(state) << "initialize"); + ACTS_VERBOSE("initialize"); // We set the current surface to the start surface - state.navigation.currentSurface = state.navigation.startSurface; + state.navigation.currentSurface = state.navigation.options.startSurface; if (state.navigation.currentSurface) { - ACTS_VERBOSE(volInfo(state) - << "Current surface set to start surface " + ACTS_VERBOSE("Current surface set to start surface " << state.navigation.currentSurface->geometryId()); } } @@ -218,7 +198,7 @@ class DirectNavigator { /// @param [in] stepper Stepper in use template void preStep(propagator_state_t& state, const stepper_t& stepper) const { - ACTS_VERBOSE(volInfo(state) << "pre step"); + ACTS_VERBOSE("pre step"); // Navigator target always resets the current surface state.navigation.currentSurface = nullptr; @@ -257,7 +237,7 @@ class DirectNavigator { // Set the navigation break state.navigation.navigationBreak = true; // If no externally provided target is given, the target is reached - if (state.navigation.targetSurface == nullptr) { + if (state.navigation.options.targetSurface == nullptr) { state.navigation.targetReached = true; // Announce it then ACTS_VERBOSE("No target Surface, job done."); @@ -274,7 +254,7 @@ class DirectNavigator { /// @param [in] stepper Stepper in use template void postStep(propagator_state_t& state, const stepper_t& stepper) const { - ACTS_VERBOSE(volInfo(state) << "post step"); + ACTS_VERBOSE("post step"); // Navigator post step always resets the current surface state.navigation.currentSurface = nullptr; @@ -320,14 +300,6 @@ class DirectNavigator { } private: - template - std::string volInfo(const propagator_state_t& state) const { - return (state.navigation.currentVolume != nullptr - ? state.navigation.currentVolume->volumeName() - : "No Volume") + - " | "; - } - ObjectIntersection chooseIntersection( const GeometryContext& gctx, const Surface& surface, const Vector3& position, const Vector3& direction, diff --git a/Core/include/Acts/Propagator/Navigator.hpp b/Core/include/Acts/Propagator/Navigator.hpp index 1f125b88b86..27a4c9148b9 100644 --- a/Core/include/Acts/Propagator/Navigator.hpp +++ b/Core/include/Acts/Propagator/Navigator.hpp @@ -127,6 +127,8 @@ class Navigator { /// It acts as an internal state which is created for every propagation and /// meant to keep thread-local navigation information. struct State { + Options options; + // Navigation on surface level /// the vector of navigation surfaces to work through NavigationSurfaces navSurfaces = {}; @@ -196,12 +198,10 @@ class Navigator { getDefaultLogger("Navigator", Logging::Level::INFO)) : m_cfg{std::move(cfg)}, m_logger{std::move(_logger)} {} - State makeState(const Surface* startSurface, - const Surface* targetSurface) const { - State result; - result.startSurface = startSurface; - result.targetSurface = targetSurface; - return result; + State makeState(const Options& options) const { + State state; + state.options = options; + return state; } const Surface* currentSurface(const State& state) const { diff --git a/Core/include/Acts/Propagator/NavigatorOptions.hpp b/Core/include/Acts/Propagator/NavigatorOptions.hpp index 266fd4adb20..55928e6eb11 100644 --- a/Core/include/Acts/Propagator/NavigatorOptions.hpp +++ b/Core/include/Acts/Propagator/NavigatorOptions.hpp @@ -10,6 +10,11 @@ namespace Acts { -struct NavigatorPlainOptions {}; +class Surface; + +struct NavigatorPlainOptions { + const Surface *startSurface{}; + const Surface *targetSurface{}; +}; } // namespace Acts diff --git a/Core/include/Acts/Propagator/Propagator.ipp b/Core/include/Acts/Propagator/Propagator.ipp index a2d3428fef3..0ae01df6194 100644 --- a/Core/include/Acts/Propagator/Propagator.ipp +++ b/Core/include/Acts/Propagator/Propagator.ipp @@ -161,16 +161,18 @@ auto Acts::Propagator::makeState( // The expanded options (including path limit) auto eOptions = options.extend(abortList); + eOptions.navigation.startSurface = &start.referenceSurface(); + eOptions.navigation.targetSurface = nullptr; using OptionsType = decltype(eOptions); - // Initialize the internal propagator state using StateType = action_list_t_state_t; + // Initialize the internal propagator state StateType state{ eOptions, m_stepper.makeState(eOptions.geoContext, eOptions.magFieldContext, start, eOptions.stepping.maxStepSize), - m_navigator.makeState(&start.referenceSurface(), nullptr)}; + m_navigator.makeState(eOptions.navigation)}; static_assert( Concepts::has_method, Concepts::Stepper::step_t, @@ -204,6 +206,8 @@ auto Acts::Propagator::makeState( // Create the extended options and declare their type auto eOptions = options.extend(abortList); + eOptions.navigation.startSurface = &start.referenceSurface(); + eOptions.navigation.targetSurface = ⌖ using OptionsType = decltype(eOptions); // Initialize the internal propagator state @@ -214,7 +218,7 @@ auto Acts::Propagator::makeState( eOptions, m_stepper.makeState(eOptions.geoContext, eOptions.magFieldContext, start, eOptions.stepping.maxStepSize), - m_navigator.makeState(&start.referenceSurface(), &target)}; + m_navigator.makeState(eOptions.navigation)}; static_assert( Concepts::has_method, Concepts::Stepper::step_t, diff --git a/Core/include/Acts/Propagator/VoidNavigator.hpp b/Core/include/Acts/Propagator/VoidNavigator.hpp index 5b2c4516982..fe5ed967157 100644 --- a/Core/include/Acts/Propagator/VoidNavigator.hpp +++ b/Core/include/Acts/Propagator/VoidNavigator.hpp @@ -29,59 +29,32 @@ struct VoidNavigator { /// @brief Nested State struct, minimal requirement struct State { - /// Navigation state - external state: the start surface - const Surface* startSurface = nullptr; - - /// Navigation state - external state: the current surface - const Surface* currentSurface = nullptr; - - /// Navigation state - external state: the target surface - const Surface* targetSurface = nullptr; - - /// Indicator if the target is reached - bool targetReached = false; - - /// Navigation state : a break has been detected - bool navigationBreak = false; + Options options; }; - State makeState(const Surface* startSurface, - const Surface* targetSurface) const { - State result; - result.startSurface = startSurface; - result.targetSurface = targetSurface; - return result; + State makeState(const Options& options) const { + State state; + state.options = options; + return state; } - const Surface* currentSurface(const State& state) const { - return state.currentSurface; + const Surface* currentSurface(const State& /*state*/) const { + return nullptr; } - const Surface* startSurface(const State& state) const { - return state.startSurface; - } + const Surface* startSurface(const State& /*state*/) const { return nullptr; } - const Surface* targetSurface(const State& state) const { - return state.targetSurface; - } + const Surface* targetSurface(const State& /*state*/) const { return nullptr; } - bool targetReached(const State& state) const { return state.targetReached; } + bool targetReached(const State& /*state*/) const { return false; } - bool navigationBreak(const State& state) const { - return state.navigationBreak; - } + bool navigationBreak(const State& /*state*/) const { return false; } - void currentSurface(State& state, const Surface* surface) const { - state.currentSurface = surface; - } + void currentSurface(State& /*state*/, const Surface* /*surface*/) const {} - void targetReached(State& state, bool targetReached) const { - state.targetReached = targetReached; - } + void targetReached(State& /*state*/, bool /*targetReached*/) const {} - void navigationBreak(State& state, bool navigationBreak) const { - state.navigationBreak = navigationBreak; - } + void navigationBreak(State& /*state*/, bool /*navigationBreak*/) const {} /// Navigation call - void /// diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index 29f5223c8d2..97992902f2b 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -477,8 +477,10 @@ class CombinatorialKalmanFilter { // Reset the navigation state // Set targetSurface to nullptr for forward filtering; it's only needed // after smoothing - state.navigation = - navigator.makeState(¤tState.referenceSurface(), nullptr); + auto navigationOptions = state.navigation.options; + navigationOptions.startSurface = ¤tState.referenceSurface(); + navigationOptions.targetSurface = nullptr; + state.navigation = navigator.makeState(navigationOptions); navigator.initialize(state, stepper); // No Kalman filtering for the starting surface, but still need diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp index a9980073087..b1f7852ab2c 100644 --- a/Core/include/Acts/TrackFitting/KalmanFitter.hpp +++ b/Core/include/Acts/TrackFitting/KalmanFitter.hpp @@ -562,7 +562,10 @@ class KalmanFitter { // Reset navigation state // We do not need to specify a target here since this will be handled // separately in the KF actor - state.navigation = navigator.makeState(&st.referenceSurface(), nullptr); + auto navigationOptions = state.navigation.options; + navigationOptions.startSurface = &st.referenceSurface(); + navigationOptions.targetSurface = nullptr; + state.navigation = navigator.makeState(navigationOptions); navigator.initialize(state, stepper); // Update material effects for last measurement state in reversed @@ -1039,7 +1042,10 @@ class KalmanFitter { // Reset the navigation state to enable propagation towards the target // surface // Set targetSurface to nullptr as it is handled manually in the actor - state.navigation = navigator.makeState(&surface, nullptr); + auto navigationOptions = state.navigation.options; + navigationOptions.startSurface = &surface; + navigationOptions.targetSurface = nullptr; + state.navigation = navigator.makeState(navigationOptions); navigator.initialize(state, stepper); return Result::success();