From d64b82f97ec92c9a3d7ca13ef4cb52aa00d2c26a Mon Sep 17 00:00:00 2001 From: andiwand Date: Tue, 5 Dec 2023 11:00:27 +0100 Subject: [PATCH] pulled changes out of #2625 --- .../Acts/Geometry/ApproachDescriptor.hpp | 9 +- .../Acts/Geometry/BoundarySurfaceT.hpp | 12 +-- .../Geometry/GenericApproachDescriptor.hpp | 9 +- .../include/Acts/Geometry/NavigationLayer.hpp | 1 - .../Acts/Navigation/DetectorNavigator.hpp | 52 +++++------ .../Acts/Navigation/NavigationState.hpp | 10 +- .../Navigation/NavigationStateUpdaters.hpp | 13 ++- .../Acts/Propagator/DirectNavigator.hpp | 92 +++++++------------ Core/include/Acts/Propagator/Navigator.hpp | 54 ++--------- Core/include/Acts/Propagator/Propagator.hpp | 4 +- Core/include/Acts/Propagator/Propagator.ipp | 4 +- .../Acts/Propagator/StandardAborters.hpp | 47 ++++++---- .../Acts/Propagator/SurfaceCollector.hpp | 5 + ...agatorComponents.hpp => VoidNavigator.hpp} | 12 ++- .../Acts/Propagator/detail/SteppingHelper.hpp | 16 ++-- .../CombinatorialKalmanFilter.hpp | 13 +-- .../Acts/TrackFitting/KalmanFitter.hpp | 12 +-- Core/include/Acts/Utilities/Intersection.hpp | 33 +++---- Core/src/Detector/DetectorVolume.cpp | 2 +- .../Geometry/GenericApproachDescriptor.cpp | 8 +- Core/src/Geometry/Layer.cpp | 18 ++-- Core/src/Geometry/TrackingVolume.cpp | 13 ++- .../Fatras/src/FatrasSimulation.cpp | 23 +++-- .../src/IterativeVertexFinderAlgorithm.cpp | 7 +- .../Vertexing/src/VertexFitterAlgorithm.cpp | 4 +- .../PropagationDenseConstant.cpp | 10 +- .../Core/Detector/DetectorVolumeTests.cpp | 8 +- .../GenericApproachDescriptorTests.cpp | 14 ++- .../Navigation/MultiWireNavigationTests.cpp | 5 +- .../Vertexing/ImpactPointEstimatorTests.cpp | 4 +- 30 files changed, 236 insertions(+), 278 deletions(-) rename Core/include/Acts/Propagator/{detail/VoidPropagatorComponents.hpp => VoidNavigator.hpp} (93%) diff --git a/Core/include/Acts/Geometry/ApproachDescriptor.hpp b/Core/include/Acts/Geometry/ApproachDescriptor.hpp index 662cbc0faac..5f05a941672 100644 --- a/Core/include/Acts/Geometry/ApproachDescriptor.hpp +++ b/Core/include/Acts/Geometry/ApproachDescriptor.hpp @@ -43,17 +43,16 @@ class ApproachDescriptor { /// @param position is the position from start of the search /// @param direction is the direction at the start of the search /// @param bcheck is the boundary check directive - /// @param pLimit The path limit - /// @param oLimit The overstep limit - /// @param tolerance The surface tolerance + /// @param nearLimit The minimum distance for an intersection to be considered + /// @param farLimit The maximum distance for an intersection to be considered /// /// @return is a surface intersection virtual SurfaceIntersection approachSurface(const GeometryContext& gctx, const Vector3& position, const Vector3& direction, const BoundaryCheck& bcheck, - double pLimit, double oLimit, - double tolerance) const = 0; + double nearLimit, + double farLimit) const = 0; /// Get all the contained surfaces /// @return all contained surfaces of this approach descriptor diff --git a/Core/include/Acts/Geometry/BoundarySurfaceT.hpp b/Core/include/Acts/Geometry/BoundarySurfaceT.hpp index 09578def199..9179aec59f7 100644 --- a/Core/include/Acts/Geometry/BoundarySurfaceT.hpp +++ b/Core/include/Acts/Geometry/BoundarySurfaceT.hpp @@ -103,13 +103,12 @@ class BoundarySurfaceT { /// /// @param gctx The current geometry context object, e.g. alignment /// @param pos The global position on surface - /// @param mom The direction on the surface - /// @param dir is an additional direction corrective + /// @param dir The direction on the surface /// /// @return The attached volume at that position virtual const volume_t* attachedVolume(const GeometryContext& gctx, - const Vector3& pos, const Vector3& mom, - Direction dir) const; + const Vector3& pos, + const Vector3& dir) const; /// templated onBoundary method /// @@ -181,11 +180,10 @@ void BoundarySurfaceT::attachVolumeArray( template const volume_t* BoundarySurfaceT::attachedVolume( - const GeometryContext& gctx, const Vector3& pos, const Vector3& mom, - Direction dir) const { + const GeometryContext& gctx, const Vector3& pos, const Vector3& dir) const { const volume_t* attVolume = nullptr; // dot product with normal vector to distinguish inside/outside - if ((surfaceRepresentation().normal(gctx, pos)).dot(dir * mom) > 0.) { + if ((surfaceRepresentation().normal(gctx, pos)).dot(dir) > 0.) { attVolume = m_alongVolumeArray ? m_alongVolumeArray->object(pos).get() : m_alongVolume; } else { diff --git a/Core/include/Acts/Geometry/GenericApproachDescriptor.hpp b/Core/include/Acts/Geometry/GenericApproachDescriptor.hpp index 2c69d92d068..53dffaa1eaa 100644 --- a/Core/include/Acts/Geometry/GenericApproachDescriptor.hpp +++ b/Core/include/Acts/Geometry/GenericApproachDescriptor.hpp @@ -57,17 +57,16 @@ class GenericApproachDescriptor : public ApproachDescriptor { /// @param position The global position to start the approach from /// @param direction The momentum vector /// @param bcheck The boundary check prescription - /// @param pLimit The path limit - /// @param oLimit The overstep limit - /// @param tolerance The surface tolerance + /// @param nearLimit The minimum distance for an intersection to be considered + /// @param farLimit The maximum distance for an intersection to be considered /// /// @return : a @c SurfaceIntersection SurfaceIntersection approachSurface(const GeometryContext& gctx, const Vector3& position, const Vector3& direction, const BoundaryCheck& bcheck, - double pLimit, double oLimit, - double tolerance) const override; + double nearLimit, + double farLimit) const override; /// return all contained surfaces of this approach descriptor const std::vector& containedSurfaces() const override; diff --git a/Core/include/Acts/Geometry/NavigationLayer.hpp b/Core/include/Acts/Geometry/NavigationLayer.hpp index 57d020a4665..24f54653006 100644 --- a/Core/include/Acts/Geometry/NavigationLayer.hpp +++ b/Core/include/Acts/Geometry/NavigationLayer.hpp @@ -26,7 +26,6 @@ namespace Acts { /// Class to be used for gaps in Volumes as a navigational link. /// Navigation Layers have a surface representation, but should usually never be /// propagated to. - class NavigationLayer : public Layer { public: /// Factory Constructor - the surface representation is given by pointer diff --git a/Core/include/Acts/Navigation/DetectorNavigator.hpp b/Core/include/Acts/Navigation/DetectorNavigator.hpp index 1301017f8bc..f65bdb57b28 100644 --- a/Core/include/Acts/Navigation/DetectorNavigator.hpp +++ b/Core/include/Acts/Navigation/DetectorNavigator.hpp @@ -83,16 +83,6 @@ class DetectorNavigator { return result; } - void resetState(State& state, const GeometryContext& /*geoContext*/, - const Vector3& /*pos*/, const Vector3& /*dir*/, - const Surface* /*ssurface*/, - const Surface* /*tsurface*/) const { - // Reset everything first - state = State(); - - // TODO fill state - } - const Surface* currentSurface(const State& state) const { return state.currentSurface; } @@ -202,24 +192,24 @@ class DetectorNavigator { << posInfo(state, stepper) << "stepping through portal"); nState.surfaceCandidates.clear(); - nState.surfaceCandidate = nState.surfaceCandidates.cend(); + nState.surfaceCandidateIndex = 0; nState.currentPortal->updateDetectorVolume(state.geoContext, nState); initializeTarget(state, stepper); } - for (; nState.surfaceCandidate != nState.surfaceCandidates.cend(); - ++nState.surfaceCandidate) { + for (; nState.surfaceCandidateIndex != nState.surfaceCandidates.size(); + ++nState.surfaceCandidateIndex) { // Screen output how much is left to try ACTS_VERBOSE(volInfo(state) << posInfo(state, stepper) - << std::distance(nState.surfaceCandidate, - nState.surfaceCandidates.cend()) + << (nState.surfaceCandidates.size() - + nState.surfaceCandidateIndex) << " out of " << nState.surfaceCandidates.size() << " surfaces remain to try."); // Take the surface - const auto& c = *(nState.surfaceCandidate); + const auto& c = nState.surfaceCandidate(); const auto& surface = (c.surface != nullptr) ? (*c.surface) : (c.portal->surface()); // Screen output which surface you are on @@ -253,7 +243,7 @@ class DetectorNavigator { /// @param [in,out] state is the mutable propagator state object /// @param [in] stepper Stepper in use template - void postStep(propagator_state_t& state, const stepper_t& stepper) const { + bool postStep(propagator_state_t& state, const stepper_t& stepper) const { ACTS_VERBOSE(volInfo(state) << posInfo(state, stepper) << "Entering navigator::postStep."); @@ -263,43 +253,43 @@ class DetectorNavigator { if (inactive()) { ACTS_VERBOSE(volInfo(state) << posInfo(state, stepper) << "navigator inactive"); - return; + return true; } if (nState.currentDetector == nullptr) { initialize(state, stepper); - return; + return true; } - if (nState.surfaceCandidate == nState.surfaceCandidates.cend()) { + if (nState.surfaceCandidateIndex == nState.surfaceCandidates.size()) { ACTS_VERBOSE(volInfo(state) << posInfo(state, stepper) << "no surface candidates - waiting for target call"); - return; + return true; } const Portal* nextPortal = nullptr; const Surface* nextSurface = nullptr; bool isPortal = false; - bool boundaryCheck = nState.surfaceCandidate->boundaryCheck; + bool boundaryCheck = nState.surfaceCandidate().boundaryCheck; - if (nState.surfaceCandidate->surface != nullptr) { - nextSurface = nState.surfaceCandidate->surface; - } else if (nState.surfaceCandidate->portal != nullptr) { - nextPortal = nState.surfaceCandidate->portal; + if (nState.surfaceCandidate().surface != nullptr) { + nextSurface = nState.surfaceCandidate().surface; + } else if (nState.surfaceCandidate().portal != nullptr) { + nextPortal = nState.surfaceCandidate().portal; nextSurface = &nextPortal->surface(); isPortal = true; } else { ACTS_ERROR(volInfo(state) << posInfo(state, stepper) << "panic: not a surface not a portal - what is it?"); - return; + return true; } // TODO not sure about the boundary check auto surfaceStatus = stepper.updateSurfaceStatus( state.stepping, *nextSurface, - nState.surfaceCandidate->objectIntersection.index(), + nState.surfaceCandidate().objectIntersection.index(), state.options.direction, BoundaryCheck(boundaryCheck), state.options.surfaceTolerance, logger()); @@ -327,9 +317,11 @@ class DetectorNavigator { ACTS_VERBOSE(volInfo(state) << posInfo(state, stepper) << "current surface set to " << nState.currentSurface->geometryId()); - ++nState.surfaceCandidate; + ++nState.surfaceCandidateIndex; } } + + return true; } private: @@ -426,7 +418,7 @@ class DetectorNavigator { return pathToA < pathToB; }); // Set the surface candidate - nState.surfaceCandidate = nCandidates.begin(); + nState.surfaceCandidateIndex = 0; } template diff --git a/Core/include/Acts/Navigation/NavigationState.hpp b/Core/include/Acts/Navigation/NavigationState.hpp index ae72ce67657..1cd752d940a 100644 --- a/Core/include/Acts/Navigation/NavigationState.hpp +++ b/Core/include/Acts/Navigation/NavigationState.hpp @@ -9,12 +9,14 @@ #pragma once #include "Acts/Definitions/Algebra.hpp" +#include "Acts/Definitions/Units.hpp" #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/Surfaces/BoundaryCheck.hpp" #include "Acts/Utilities/Delegate.hpp" #include "Acts/Utilities/Intersection.hpp" #include +#include #include namespace Acts { @@ -80,16 +82,20 @@ struct NavigationState { /// That are the candidate surfaces to process SurfaceCandidates surfaceCandidates = {}; - SurfaceCandidates::const_iterator surfaceCandidate = surfaceCandidates.cend(); + std::size_t surfaceCandidateIndex = 0; /// Boundary directives for surfaces BoundaryCheck surfaceBoundaryCheck = BoundaryCheck(true); /// An overstep tolerance - ActsScalar overstepTolerance = -0.1; + ActsScalar overstepTolerance = -100 * UnitConstants::um; /// Auxiliary attached information std::any auxiliary; + + const SurfaceCandidate& surfaceCandidate() const { + return surfaceCandidates.at(surfaceCandidateIndex); + } }; } // namespace Experimental diff --git a/Core/include/Acts/Navigation/NavigationStateUpdaters.hpp b/Core/include/Acts/Navigation/NavigationStateUpdaters.hpp index 0658ccfe8de..3edfefb8c0a 100644 --- a/Core/include/Acts/Navigation/NavigationStateUpdaters.hpp +++ b/Core/include/Acts/Navigation/NavigationStateUpdaters.hpp @@ -12,6 +12,7 @@ #include "Acts/Definitions/Common.hpp" #include "Acts/Detector/Portal.hpp" #include "Acts/Navigation/NavigationDelegates.hpp" +#include "Acts/Navigation/NavigationState.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Utilities/BinningType.hpp" #include "Acts/Utilities/Enumerate.hpp" @@ -38,9 +39,10 @@ inline void updateCandidates(const GeometryContext& gctx, NavigationState& nState) { const auto& position = nState.position; const auto& direction = nState.direction; - auto& nCandidates = nState.surfaceCandidates; - for (auto& c : nCandidates) { + NavigationState::SurfaceCandidates nextSurfaceCandidates; + + for (NavigationState::SurfaceCandidate c : nState.surfaceCandidates) { // Get the surface representation: either native surfcae of portal const Surface& sRep = c.surface != nullptr ? *c.surface : c.portal->surface(); @@ -50,7 +52,14 @@ inline void updateCandidates(const GeometryContext& gctx, auto sIntersection = sRep.intersect(gctx, position, direction, c.boundaryCheck, s_onSurfaceTolerance); c.objectIntersection = sIntersection[c.objectIntersection.index()]; + + if (c.objectIntersection && + c.objectIntersection.pathLength() > nState.overstepTolerance) { + nextSurfaceCandidates.emplace_back(std::move(c)); + } } + + nState.surfaceCandidates = std::move(nextSurfaceCandidates); } /// @brief This sets a single object, e.g. single surface or single volume diff --git a/Core/include/Acts/Propagator/DirectNavigator.hpp b/Core/include/Acts/Propagator/DirectNavigator.hpp index c1b732f818f..4a815ef961d 100644 --- a/Core/include/Acts/Propagator/DirectNavigator.hpp +++ b/Core/include/Acts/Propagator/DirectNavigator.hpp @@ -1,6 +1,6 @@ // This file is part of the Acts project. // -// Copyright (C) 2019 CERN for the benefit of the Acts project +// Copyright (C) 2019-2023 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 @@ -24,13 +24,11 @@ namespace Acts { -/// DirectNavigator class +/// This is a fully guided navigator that progresses through a pre-given +/// sequence of surfaces. /// -/// This is a fully guided navigator that progresses through -/// a pre-given sequence of surfaces. -/// -/// This can either be used as a validation tool, for truth -/// tracking, or track refitting +/// This can either be used as a validation tool, for truth tracking, or track +/// refitting. class DirectNavigator { public: /// The sequentially crossed surfaces @@ -41,10 +39,9 @@ class DirectNavigator { getDefaultLogger("DirectNavigator", Logging::INFO)) : m_logger{std::move(_logger)} {} - /// Nested Actor struct, called Initializer + /// @brief Nested Actor struct, called Initializer /// - /// This is needed for the initialization of the - /// surface sequence + /// This is needed for the initialization of the surface sequence. struct Initializer { /// The Surface sequence SurfaceSequence navSurfaces = {}; @@ -94,14 +91,13 @@ class DirectNavigator { } }; - /// Nested State struct + /// @brief Nested State struct /// - /// It acts as an internal state which is - /// created for every propagation/extrapolation step - /// and keep thread-local navigation information + /// It acts as an internal state which is created for every + /// propagation/extrapolation step and keep thread-local navigation + /// information struct State { - /// Externally provided surfaces - expected to be ordered - /// along the path + /// Externally provided surfaces - expected to be ordered along the path SurfaceSequence navSurfaces = {}; /// Iterator the next surface @@ -138,27 +134,6 @@ class DirectNavigator { return result; } - /// Reset state - /// - /// @param state is the state to reset - /// @param ssurface is the new starting surface - /// @param tsurface is the target surface - void resetState(State& state, const GeometryContext& /*geoContext*/, - const Vector3& /*pos*/, const Vector3& /*dir*/, - const Surface* ssurface, const Surface* tsurface) const { - // Reset everything except the navSurfaces - auto navSurfaces = state.navSurfaces; - state = State(); - state.navSurfaces = navSurfaces; - - // Reset others - state.navSurfaceIter = - std::find(state.navSurfaces.begin(), state.navSurfaces.end(), ssurface); - state.startSurface = ssurface; - state.currentSurface = ssurface; - state.targetSurface = tsurface; - } - const Surface* currentSurface(const State& state) const { return state.currentSurface; } @@ -215,8 +190,7 @@ class DirectNavigator { void initialize(propagator_state_t& state, const stepper_t& stepper) const { (void)stepper; - // Call the navigation helper prior to actual navigation - ACTS_VERBOSE(volInfo(state) << "Initialization."); + ACTS_VERBOSE(volInfo(state) << "initialize"); // We set the current surface to the start surface state.navigation.currentSurface = state.navigation.startSurface; @@ -236,8 +210,7 @@ class DirectNavigator { /// @param [in] stepper Stepper in use template void preStep(propagator_state_t& state, const stepper_t& stepper) const { - // Screen output - ACTS_VERBOSE("Entering navigator::target."); + ACTS_VERBOSE(volInfo(state) << "pre step"); // Navigator target always resets the current surface state.navigation.currentSurface = nullptr; @@ -251,12 +224,12 @@ class DirectNavigator { // Establish & update the surface status // TODO we do not know the intersection index - passing the closer one const auto& surface = **state.navigation.navSurfaceIter; + const double farLimit = std::numeric_limits::max(); const auto index = chooseIntersection( state.geoContext, surface, stepper.position(state.stepping), state.options.direction * stepper.direction(state.stepping), - BoundaryCheck(false), std::numeric_limits::max(), - stepper.overstepLimit(state.stepping), + BoundaryCheck(false), m_nearLimit, farLimit, state.options.surfaceTolerance) .index(); auto surfaceStatus = stepper.updateSurfaceStatus( @@ -292,9 +265,8 @@ class DirectNavigator { /// @param [in,out] state is the mutable propagator state object /// @param [in] stepper Stepper in use template - void postStep(propagator_state_t& state, const stepper_t& stepper) const { - // Screen output - ACTS_VERBOSE("Entering navigator::postStep."); + bool postStep(propagator_state_t& state, const stepper_t& stepper) const { + ACTS_VERBOSE(volInfo(state) << "post step"); // Navigator post step always resets the current surface state.navigation.currentSurface = nullptr; @@ -309,12 +281,12 @@ class DirectNavigator { // Establish the surface status // TODO we do not know the intersection index - passing the closer one const auto& surface = **state.navigation.navSurfaceIter; + const double farLimit = std::numeric_limits::max(); const auto index = chooseIntersection( state.geoContext, surface, stepper.position(state.stepping), state.options.direction * stepper.direction(state.stepping), - BoundaryCheck(false), std::numeric_limits::max(), - stepper.overstepLimit(state.stepping), + BoundaryCheck(false), m_nearLimit, farLimit, state.options.surfaceTolerance) .index(); auto surfaceStatus = stepper.updateSurfaceStatus( @@ -337,29 +309,29 @@ class DirectNavigator { << stepper.outputStepSize(state.stepping)); } } + + return true; } private: template std::string volInfo(const propagator_state_t& state) const { - return (state.navigation.currentVolume + 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, - const BoundaryCheck& bcheck, - double pLimit, double oLimit, - double tolerance) const { + ObjectIntersection chooseIntersection( + const GeometryContext& gctx, const Surface& surface, + const Vector3& position, const Vector3& direction, + const BoundaryCheck& bcheck, double nearLimit, double farLimit, + double tolerance) const { auto intersections = surface.intersect(gctx, position, direction, bcheck, tolerance); for (auto& intersection : intersections.split()) { - if (detail::checkIntersection(intersection, pLimit, oLimit, tolerance, + if (detail::checkIntersection(intersection, nearLimit, farLimit, logger())) { return intersection; } @@ -371,6 +343,12 @@ class DirectNavigator { const Logger& logger() const { return *m_logger; } std::unique_ptr m_logger; + + // TODO https://github.com/acts-project/acts/issues/2738 + /// Distance limit to discard intersections "behind us" + /// @note this is only necessary because some surfaces have more than one + /// intersection + double m_nearLimit = -100 * UnitConstants::um; }; } // namespace Acts diff --git a/Core/include/Acts/Propagator/Navigator.hpp b/Core/include/Acts/Propagator/Navigator.hpp index 46126777d01..7d3570436ec 100644 --- a/Core/include/Acts/Propagator/Navigator.hpp +++ b/Core/include/Acts/Propagator/Navigator.hpp @@ -211,44 +211,6 @@ class Navigator { return result; } - /// Reset state - /// - /// @param state is the state - /// @param geoContext is the geometry context - /// @param pos is the global position - /// @param dir is the direction of navigation - /// @param ssurface is the new starting surface - /// @param tsurface is the target surface - void resetState(State& state, const GeometryContext& geoContext, - const Vector3& pos, const Vector3& dir, - const Surface* ssurface, const Surface* tsurface) const { - // Reset everything first - state = State(); - - // Set the start, current and target objects - state.startSurface = ssurface; - if (ssurface->associatedLayer() != nullptr) { - state.startLayer = ssurface->associatedLayer(); - } - if (state.startLayer->trackingVolume() != nullptr) { - state.startVolume = state.startLayer->trackingVolume(); - } - state.currentSurface = state.startSurface; - state.currentVolume = state.startVolume; - state.targetSurface = tsurface; - - // Get the compatible layers (including the current layer) - NavigationOptions navOpts; - navOpts.resolveSensitive = true; - navOpts.resolveMaterial = true; - navOpts.resolvePassive = true; - state.navLayers = - state.currentVolume->compatibleLayers(geoContext, pos, dir, navOpts); - - // Set the index to the first - state.navLayerIndex = 0; - } - const Surface* currentSurface(const State& state) const { return state.currentSurface; } @@ -447,10 +409,10 @@ class Navigator { /// @param [in,out] state is the mutable propagator state object /// @param [in] stepper Stepper in use template - void postStep(propagator_state_t& state, const stepper_t& stepper) const { + bool postStep(propagator_state_t& state, const stepper_t& stepper) const { // Check if the navigator is inactive if (inactive(state, stepper)) { - return; + return true; } // Set the navigation stage @@ -480,11 +442,11 @@ class Navigator { state.navigation.startLayer) { // this was the start layer, switch to layer target next state.navigation.navigationStage = Stage::layerTarget; - return; + return true; } else { // no layers, go to boundary state.navigation.navigationStage = Stage::boundaryTarget; - return; + return true; } } } @@ -500,7 +462,7 @@ class Navigator { if (resolveSurfaces(state, stepper)) { // Set the navigation stage back to surface handling state.navigation.navigationStage = Stage::surfaceTarget; - return; + return true; } } else { // Set the navigation stage to layer target @@ -528,7 +490,7 @@ class Navigator { auto boundary = state.navigation.navBoundary().object(); state.navigation.currentVolume = boundary->attachedVolume( state.geoContext, stepper.position(state.stepping), - stepper.direction(state.stepping), state.options.direction); + state.options.direction * stepper.direction(state.stepping)); // No volume anymore : end of known world if (!state.navigation.currentVolume) { ACTS_VERBOSE( @@ -536,7 +498,7 @@ class Navigator { << "No more volume to progress to, stopping navigation."); // Navigation break & release navigation stepping state.navigation.navigationBreak = true; - return; + return true; } else { ACTS_VERBOSE(volInfo(state) << "Volume updated."); // Forget the boundary information @@ -565,6 +527,8 @@ class Navigator { ACTS_VERBOSE(volInfo(state) << "Status could not be determined - good luck."); } + + return true; } private: diff --git a/Core/include/Acts/Propagator/Propagator.hpp b/Core/include/Acts/Propagator/Propagator.hpp index a3c914a8cc0..8eee3fcf2db 100644 --- a/Core/include/Acts/Propagator/Propagator.hpp +++ b/Core/include/Acts/Propagator/Propagator.hpp @@ -23,8 +23,8 @@ #include "Acts/Propagator/ActionList.hpp" #include "Acts/Propagator/StandardAborters.hpp" #include "Acts/Propagator/StepperConcept.hpp" +#include "Acts/Propagator/VoidNavigator.hpp" #include "Acts/Propagator/detail/ParameterTraits.hpp" -#include "Acts/Propagator/detail/VoidPropagatorComponents.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/Result.hpp" @@ -203,7 +203,7 @@ struct PropagatorOptions : public PropagatorPlainOptions { /// - a type mapping for: (initial track parameter type and destination /// surface type) -> type of internal state object /// -template +template class Propagator final { /// Re-define bound track parameters dependent on the stepper using StepperBoundTrackParameters = diff --git a/Core/include/Acts/Propagator/Propagator.ipp b/Core/include/Acts/Propagator/Propagator.ipp index e94bf2b9bba..7319cbfb22b 100644 --- a/Core/include/Acts/Propagator/Propagator.ipp +++ b/Core/include/Acts/Propagator/Propagator.ipp @@ -65,7 +65,9 @@ auto Acts::Propagator::propagate_impl(propagator_state_t& state, // Post-stepping: // navigator post step call - action list - aborter list state.stage = PropagatorStage::postStep; - m_navigator.postStep(state, m_stepper); + if (!m_navigator.postStep(state, m_stepper)) { + continue; + } state.options.actionList(state, m_stepper, m_navigator, result, logger()); if (state.options.abortList(state, m_stepper, m_navigator, result, logger())) { diff --git a/Core/include/Acts/Propagator/StandardAborters.hpp b/Core/include/Acts/Propagator/StandardAborters.hpp index 7c93b79198e..9f914b50758 100644 --- a/Core/include/Acts/Propagator/StandardAborters.hpp +++ b/Core/include/Acts/Propagator/StandardAborters.hpp @@ -1,6 +1,6 @@ // This file is part of the Acts project. // -// Copyright (C) 2017-2018 CERN for the benefit of the Acts project +// Copyright (C) 2017-2023 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 @@ -67,15 +67,20 @@ struct PathLimitReached { } }; -/// This is the condition that the Surface has been reached -/// it then triggers an propagation abort of the propagation +/// This is the condition that the Surface has been reached it then triggers an +/// propagation abort struct SurfaceReached { const Surface* surface = nullptr; BoundaryCheck boundaryCheck = BoundaryCheck(true); - std::optional overstepLimit; + + // TODO https://github.com/acts-project/acts/issues/2738 + /// Distance limit to discard intersections "behind us" + /// @note this is only necessary because some surfaces have more than one + /// intersection + double nearLimit = -100 * UnitConstants::um; SurfaceReached() = default; - SurfaceReached(double oLimit) : overstepLimit(oLimit) {} + SurfaceReached(double nLimit) : nearLimit(nLimit) {} /// boolean operator for abort condition without using the result /// @@ -101,10 +106,11 @@ struct SurfaceReached { return true; } - const double pLimit = - state.stepping.stepSize.value(ConstrainedStep::aborter); - const double oLimit = - overstepLimit.value_or(stepper.overstepLimit(state.stepping)); + // not using the stepper overstep limit here because it does not always work + // for perigee surfaces + // note: the near limit is necessary for surfaces with more than one + // intersections in order to discard the ones which are behind us + const double farLimit = std::numeric_limits::max(); const double tolerance = state.options.surfaceTolerance; const auto sIntersection = surface->intersect( @@ -113,33 +119,40 @@ struct SurfaceReached { boundaryCheck, tolerance); const auto closest = sIntersection.closest(); + bool reached = false; + if (closest.status() == Intersection3D::Status::onSurface) { const double distance = closest.pathLength(); ACTS_VERBOSE( "SurfaceReached aborter | " "Target surface reached at distance (tolerance) " << distance << " (" << tolerance << ")"); - return true; + reached = true; } + bool intersectionFound = false; + for (const auto& intersection : sIntersection.split()) { if (intersection && - detail::checkIntersection(intersection.intersection(), pLimit, oLimit, - tolerance, logger)) { + detail::checkIntersection(intersection.intersection(), nearLimit, + farLimit, logger)) { stepper.updateStepSize(state.stepping, intersection.pathLength(), ConstrainedStep::aborter, false); ACTS_VERBOSE( "SurfaceReached aborter | " "Target stepSize (surface) updated to " << stepper.outputStepSize(state.stepping)); - return false; + intersectionFound = true; + break; } } - ACTS_VERBOSE( - "SurfaceReached aborter | " - "Target intersection not found. Maybe next time?"); - return false; + if (!intersectionFound) { + ACTS_VERBOSE( + "SurfaceReached aborter | " + "Target intersection not found. Maybe next time?"); + } + return reached; } }; diff --git a/Core/include/Acts/Propagator/SurfaceCollector.hpp b/Core/include/Acts/Propagator/SurfaceCollector.hpp index 130fad7ebef..4f996019233 100644 --- a/Core/include/Acts/Propagator/SurfaceCollector.hpp +++ b/Core/include/Acts/Propagator/SurfaceCollector.hpp @@ -8,6 +8,7 @@ #pragma once +#include "Acts/Propagator/Propagator.hpp" #include "Acts/Surfaces/Surface.hpp" #include @@ -94,6 +95,10 @@ struct SurfaceCollector { void operator()(propagator_state_t& state, const stepper_t& stepper, const navigator_t& navigator, result_type& result, const Logger& logger) const { + if (state.stage == PropagatorStage::postPropagation) { + return; + } + auto currentSurface = navigator.currentSurface(state.navigation); // The current surface has been assigned by the navigator diff --git a/Core/include/Acts/Propagator/detail/VoidPropagatorComponents.hpp b/Core/include/Acts/Propagator/VoidNavigator.hpp similarity index 93% rename from Core/include/Acts/Propagator/detail/VoidPropagatorComponents.hpp rename to Core/include/Acts/Propagator/VoidNavigator.hpp index f8f44f275a6..a7d0f5fce2a 100644 --- a/Core/include/Acts/Propagator/detail/VoidPropagatorComponents.hpp +++ b/Core/include/Acts/Propagator/VoidNavigator.hpp @@ -1,6 +1,6 @@ // This file is part of the Acts project. // -// Copyright (C) 2018 CERN for the benefit of the Acts project +// Copyright (C) 2018-2023 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 @@ -9,7 +9,8 @@ #pragma once namespace Acts { -namespace detail { + +class Surface; /// @brief The void navigator struct as a default navigator /// @@ -103,9 +104,10 @@ struct VoidNavigator { /// /// Empty call, compiler should optimise that template - void postStep(propagator_state_t& /*state*/, - const stepper_t& /*stepper*/) const {} + bool postStep(propagator_state_t& /*state*/, + const stepper_t& /*stepper*/) const { + return true; + } }; -} // namespace detail } // namespace Acts diff --git a/Core/include/Acts/Propagator/detail/SteppingHelper.hpp b/Core/include/Acts/Propagator/detail/SteppingHelper.hpp index 359233ad033..7f8c5ebe9dc 100644 --- a/Core/include/Acts/Propagator/detail/SteppingHelper.hpp +++ b/Core/include/Acts/Propagator/detail/SteppingHelper.hpp @@ -17,6 +17,8 @@ #include "Acts/Utilities/Intersection.hpp" #include "Acts/Utilities/Logger.hpp" +#include + namespace Acts { namespace detail { @@ -36,8 +38,8 @@ Acts::Intersection3D::Status updateSingleSurfaceStatus( const Surface& surface, std::uint8_t index, Direction navDir, const BoundaryCheck& bcheck, ActsScalar surfaceTolerance, const Logger& logger) { - ACTS_VERBOSE( - "Update single surface status for surface: " << surface.geometryId()); + ACTS_VERBOSE("Update single surface status for surface: " + << surface.geometryId() << " index " << (int)index); auto sIntersection = surface.intersect( state.geoContext, stepper.position(state), @@ -51,13 +53,11 @@ Acts::Intersection3D::Status updateSingleSurfaceStatus( return Intersection3D::Status::onSurface; } - // Path and overstep limit checking - const double pLimit = state.stepSize.value(ConstrainedStep::aborter); - const double oLimit = stepper.overstepLimit(state); + const double nearLimit = std::numeric_limits::lowest(); + const double farLimit = std::numeric_limits::max(); - if (sIntersection && - detail::checkIntersection(sIntersection.intersection(), pLimit, oLimit, - surfaceTolerance, logger)) { + if (sIntersection && detail::checkIntersection(sIntersection.intersection(), + nearLimit, farLimit, logger)) { ACTS_VERBOSE("Surface is reachable"); stepper.updateStepSize(state, sIntersection.pathLength(), ConstrainedStep::actor); diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index cab69814517..2fc392ab24b 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -634,10 +634,9 @@ class CombinatorialKalmanFilter { // Reset the navigation state // Set targetSurface to nullptr for forward filtering; it's only needed // after smoothing - navigator.resetState( - state.navigation, state.geoContext, stepper.position(state.stepping), - state.options.direction * stepper.direction(state.stepping), - ¤tState.referenceSurface(), nullptr); + state.navigation = + navigator.makeState(¤tState.referenceSurface(), nullptr); + navigator.initialize(state, stepper); // No Kalman filtering for the starting surface, but still need // to consider the material effects here @@ -1310,10 +1309,8 @@ class CombinatorialKalmanFilter { // Reset the navigation state to enable propagation towards the target // surface // Set targetSurface to nullptr as it is handled manually in the actor - navigator.resetState( - state.navigation, state.geoContext, stepper.position(state.stepping), - state.options.direction * stepper.direction(state.stepping), &surface, - nullptr); + state.navigation = navigator.makeState(&surface, nullptr); + navigator.initialize(state, stepper); detail::setupLoopProtection(state, stepper, result.pathLimitReached, true, logger()); diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp index 993215ea39a..71589eb12a5 100644 --- a/Core/include/Acts/TrackFitting/KalmanFitter.hpp +++ b/Core/include/Acts/TrackFitting/KalmanFitter.hpp @@ -569,10 +569,8 @@ class KalmanFitter { // Reset navigation state // We do not need to specify a target here since this will be handled // separately in the KF actor - navigator.resetState( - state.navigation, state.geoContext, stepper.position(state.stepping), - state.options.direction * stepper.direction(state.stepping), - &st.referenceSurface(), nullptr); + state.navigation = navigator.makeState(&st.referenceSurface(), nullptr); + navigator.initialize(state, stepper); // Update material effects for last measurement state in reversed // direction @@ -1048,10 +1046,8 @@ 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 - navigator.resetState( - state.navigation, state.geoContext, stepper.position(state.stepping), - state.options.direction * stepper.direction(state.stepping), &surface, - nullptr); + state.navigation = navigator.makeState(&surface, nullptr); + navigator.initialize(state, stepper); return Result::success(); } diff --git a/Core/include/Acts/Utilities/Intersection.hpp b/Core/include/Acts/Utilities/Intersection.hpp index a4ef4990d52..d6ccbfc3a8e 100644 --- a/Core/include/Acts/Utilities/Intersection.hpp +++ b/Core/include/Acts/Utilities/Intersection.hpp @@ -281,26 +281,25 @@ namespace detail { /// path-limit and overstep-limit /// /// @tparam intersection_t Type of the intersection object -/// @tparam logger_t The logger type, which defaults to std::false_type to -/// prevent the generation of logging code +/// @tparam logger_t The logger type, which defaults to std::false_type +/// to prevent the generation of logging code /// /// @param intersection The intersection to check -/// @param pLimit The path-limit -/// @param oLimit The overstep-limit -/// @param tolerance The tolerance that is applied to the path-limit criterion +/// @param nearLimit The minimum distance for an intersection to be considered +/// @param farLimit The maximum distance for an intersection to be considered /// @param logger A optionally supplied logger which prints out a lot of infos -/// at VERBOSE level +/// at VERBOSE level template -bool checkIntersection(const intersection_t& intersection, double pLimit, - double oLimit, double tolerance, +bool checkIntersection(const intersection_t& intersection, double nearLimit, + double farLimit, const Logger& logger = getDummyLogger()) { - const double cLimit = intersection.pathLength(); + const double distance = intersection.pathLength(); - ACTS_VERBOSE(" -> pLimit, oLimit, cLimit: " << pLimit << ", " << oLimit - << ", " << cLimit); + ACTS_VERBOSE(" -> near limit, far limit, distance: " + << nearLimit << ", " << farLimit << ", " << distance); - const bool coCriterion = cLimit > oLimit; - const bool cpCriterion = std::abs(cLimit) < std::abs(pLimit) + tolerance; + const bool coCriterion = distance > nearLimit; + const bool cpCriterion = distance < farLimit; const bool accept = coCriterion && cpCriterion; @@ -310,13 +309,11 @@ bool checkIntersection(const intersection_t& intersection, double pLimit, ACTS_VERBOSE("Intersection is OUTSIDE limit because: "); if (!coCriterion) { ACTS_VERBOSE("- intersection path length " - << cLimit << " <= overstep limit " << oLimit); + << distance << " <= near limit " << nearLimit); } if (!cpCriterion) { - ACTS_VERBOSE("- intersection path length " - << std::abs(cLimit) << " is over the path limit " - << (std::abs(pLimit) + tolerance) - << " (including tolerance of " << tolerance << ")"); + ACTS_VERBOSE("- intersection path length " << distance << " >= far limit " + << farLimit); } } diff --git a/Core/src/Detector/DetectorVolume.cpp b/Core/src/Detector/DetectorVolume.cpp index 2b49b3476a0..c233f36ae3b 100644 --- a/Core/src/Detector/DetectorVolume.cpp +++ b/Core/src/Detector/DetectorVolume.cpp @@ -240,7 +240,7 @@ void Acts::Experimental::DetectorVolume::updateNavigationState( const GeometryContext& gctx, NavigationState& nState) const { nState.currentVolume = this; m_surfaceCandidatesUpdater(gctx, nState); - nState.surfaceCandidate = nState.surfaceCandidates.begin(); + nState.surfaceCandidateIndex = 0; } void Acts::Experimental::DetectorVolume::assignSurfaceCandidatesUpdater( diff --git a/Core/src/Geometry/GenericApproachDescriptor.cpp b/Core/src/Geometry/GenericApproachDescriptor.cpp index 9307cde34d6..a6f5eca3ade 100644 --- a/Core/src/Geometry/GenericApproachDescriptor.cpp +++ b/Core/src/Geometry/GenericApproachDescriptor.cpp @@ -25,16 +25,16 @@ void Acts::GenericApproachDescriptor::registerLayer(const Layer& lay) { Acts::SurfaceIntersection Acts::GenericApproachDescriptor::approachSurface( const GeometryContext& gctx, const Vector3& position, - const Vector3& direction, const BoundaryCheck& bcheck, double pLimit, - double oLimit, double tolerance) const { + const Vector3& direction, const BoundaryCheck& bcheck, double nearLimit, + double farLimit) const { // almost always 2 - boost::container::small_vector sIntersections; + boost::container::small_vector sIntersections; sIntersections.reserve(m_surfaceCache.size()); for (const auto& sf : m_surfaceCache) { auto sfIntersection = sf->intersect(gctx, position, direction, bcheck); for (const auto& intersection : sfIntersection.split()) { if (intersection && - detail::checkIntersection(intersection, pLimit, oLimit, tolerance)) { + detail::checkIntersection(intersection, nearLimit, farLimit)) { sIntersections.push_back(intersection); } } diff --git a/Core/src/Geometry/Layer.cpp b/Core/src/Geometry/Layer.cpp index 705188d3dae..2d8ae499caf 100644 --- a/Core/src/Geometry/Layer.cpp +++ b/Core/src/Geometry/Layer.cpp @@ -187,9 +187,8 @@ Acts::Layer::compatibleSurfaces( sf.intersect(gctx, position, direction, BoundaryCheck(boundaryCheck)); for (const auto& sfi : sfmi.split()) { // check if intersection is valid and pathLimit has not been exceeded - if (sfi && - detail::checkIntersection(sfi.intersection(), pathLimit, - overstepLimit, s_onSurfaceTolerance)) { + if (sfi && detail::checkIntersection(sfi.intersection(), overstepLimit, + pathLimit)) { sIntersections.push_back(sfi); } } @@ -272,17 +271,15 @@ Acts::SurfaceIntersection Acts::Layer::surfaceOnApproach( (surfaceRepresentation().surfaceMaterial() != nullptr)); // The Limits: current path & overstepping - double pLimit = options.pathLimit; - double oLimit = options.overstepLimit; - // TODO this should be configurable - double tolerance = s_onSurfaceTolerance; + double nearLimit = options.pathLimit; + double farLimit = options.overstepLimit; // Helper function to find valid intersection auto findValidIntersection = [&](const SurfaceMultiIntersection& sfmi) -> SurfaceIntersection { for (const auto& sfi : sfmi.split()) { - if (sfi && detail::checkIntersection(sfi.intersection(), pLimit, oLimit, - tolerance)) { + if (sfi && + detail::checkIntersection(sfi.intersection(), nearLimit, farLimit)) { return sfi; } } @@ -294,8 +291,7 @@ Acts::SurfaceIntersection Acts::Layer::surfaceOnApproach( // Approach descriptor present and resolving is necessary if (m_approachDescriptor && (resolvePS || resolveMS)) { SurfaceIntersection aSurface = m_approachDescriptor->approachSurface( - gctx, position, direction, options.boundaryCheck, pLimit, oLimit, - tolerance); + gctx, position, direction, options.boundaryCheck, nearLimit, farLimit); return aSurface; } diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index af9f111446e..5f4a1d44901 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -482,8 +482,8 @@ Acts::TrackingVolume::compatibleBoundaries( boost::container::small_vector bIntersections; // The Limits: current, path & overstepping - double pLimit = options.pathLimit; - double oLimit = 0; + double nearLimit = 0; + double farLimit = options.pathLimit; // Helper function to test intersection auto checkIntersection = @@ -496,14 +496,14 @@ Acts::TrackingVolume::compatibleBoundaries( if (options.forceIntersectBoundaries) { const bool coCriterion = - std::abs(sIntersection.pathLength()) < std::abs(oLimit); + std::abs(sIntersection.pathLength()) < std::abs(nearLimit); ACTS_VERBOSE("Forcing intersection with surface " << bSurface->surfaceRepresentation().geometryId()); if (coCriterion) { ACTS_VERBOSE("Intersection forced successfully "); ACTS_VERBOSE("- intersection path length " << std::abs(sIntersection.pathLength()) - << " < overstep limit " << std::abs(oLimit)); + << " < overstep limit " << std::abs(nearLimit)); return BoundaryIntersection(sIntersection.intersection(), bSurface, sIntersection.object(), sIntersection.index()); @@ -511,15 +511,14 @@ Acts::TrackingVolume::compatibleBoundaries( ACTS_VERBOSE("Can't force intersection: "); ACTS_VERBOSE("- intersection path length " << std::abs(sIntersection.pathLength()) - << " > overstep limit " << std::abs(oLimit)); + << " > overstep limit " << std::abs(nearLimit)); } ACTS_VERBOSE("Check intersection with surface " << bSurface->surfaceRepresentation().geometryId()); if (detail::checkIntersection( - sIntersection.intersection(), pLimit, oLimit, - s_onSurfaceTolerance, logger)) { + sIntersection.intersection(), nearLimit, farLimit, logger)) { return BoundaryIntersection(sIntersection.intersection(), bSurface, sIntersection.object(), sIntersection.index()); diff --git a/Examples/Algorithms/Fatras/src/FatrasSimulation.cpp b/Examples/Algorithms/Fatras/src/FatrasSimulation.cpp index e7857a0ed1b..a65953f1bc8 100644 --- a/Examples/Algorithms/Fatras/src/FatrasSimulation.cpp +++ b/Examples/Algorithms/Fatras/src/FatrasSimulation.cpp @@ -120,15 +120,20 @@ struct FatrasSimulationT final : ActsExamples::detail::FatrasSimulation { FatrasSimulationT(const ActsExamples::FatrasSimulation::Config &cfg, Acts::Logging::Level lvl) - : simulation( - ChargedSimulation( - ChargedPropagator(ChargedStepper(cfg.magneticField), - Acts::Navigator{{cfg.trackingGeometry}}), - Acts::getDefaultLogger("Simulation", lvl)), - NeutralSimulation( - NeutralPropagator(NeutralStepper(), - Acts::Navigator{{cfg.trackingGeometry}}), - Acts::getDefaultLogger("Simulation", lvl))) { + : simulation(ChargedSimulation( + ChargedPropagator( + ChargedStepper(cfg.magneticField), + Acts::Navigator{{cfg.trackingGeometry}, + Acts::getDefaultLogger("nav", lvl)}, + Acts::getDefaultLogger("prop", lvl)), + Acts::getDefaultLogger("Simulation", lvl)), + NeutralSimulation( + NeutralPropagator( + NeutralStepper(), + Acts::Navigator{{cfg.trackingGeometry}, + Acts::getDefaultLogger("nav", lvl)}, + Acts::getDefaultLogger("prop", lvl)), + Acts::getDefaultLogger("Simulation", lvl))) { using namespace ActsFatras; using namespace ActsFatras::detail; // apply the configuration diff --git a/Examples/Algorithms/Vertexing/src/IterativeVertexFinderAlgorithm.cpp b/Examples/Algorithms/Vertexing/src/IterativeVertexFinderAlgorithm.cpp index 0052381886e..c432da57910 100644 --- a/Examples/Algorithms/Vertexing/src/IterativeVertexFinderAlgorithm.cpp +++ b/Examples/Algorithms/Vertexing/src/IterativeVertexFinderAlgorithm.cpp @@ -10,7 +10,7 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Propagator/EigenStepper.hpp" -#include "Acts/Propagator/detail/VoidPropagatorComponents.hpp" +#include "Acts/Propagator/VoidNavigator.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/Result.hpp" #include "Acts/Vertexing/IterativeVertexFinder.hpp" @@ -71,9 +71,8 @@ ActsExamples::ProcessCode ActsExamples::IterativeVertexFinderAlgorithm::execute( Acts::EigenStepper<> stepper(m_cfg.bField); // Set up propagator with void navigator - auto propagator = - std::make_shared(stepper, Acts::detail::VoidNavigator{}, - logger().cloneWithSuffix("Propagator")); + auto propagator = std::make_shared( + stepper, Acts::VoidNavigator{}, logger().cloneWithSuffix("Propagator")); // Setup the vertex fitter Fitter::Config vertexFitterCfg; Fitter vertexFitter(vertexFitterCfg, diff --git a/Examples/Algorithms/Vertexing/src/VertexFitterAlgorithm.cpp b/Examples/Algorithms/Vertexing/src/VertexFitterAlgorithm.cpp index 7889a8f0ef6..05ea2f092f2 100644 --- a/Examples/Algorithms/Vertexing/src/VertexFitterAlgorithm.cpp +++ b/Examples/Algorithms/Vertexing/src/VertexFitterAlgorithm.cpp @@ -11,7 +11,7 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/MagneticField/MagneticFieldProvider.hpp" #include "Acts/Propagator/EigenStepper.hpp" -#include "Acts/Propagator/detail/VoidPropagatorComponents.hpp" +#include "Acts/Propagator/VoidNavigator.hpp" #include "Acts/Utilities/Result.hpp" #include "Acts/Vertexing/Vertex.hpp" #include "ActsExamples/EventData/ProtoVertex.hpp" @@ -45,7 +45,7 @@ ActsExamples::ProcessCode ActsExamples::VertexFitterAlgorithm::execute( // Setup the propagator with void navigator auto propagator = std::make_shared( - stepper, Acts::detail::VoidNavigator{}, logger().cloneWithSuffix("Prop")); + stepper, Acts::VoidNavigator{}, logger().cloneWithSuffix("Prop")); PropagatorOptions propagatorOpts(ctx.geoContext, ctx.magFieldContext); // Setup the vertex fitter VertexFitter::Config vertexFitterCfg; diff --git a/Tests/IntegrationTests/PropagationDenseConstant.cpp b/Tests/IntegrationTests/PropagationDenseConstant.cpp index db901b4f801..b5a9eee554d 100644 --- a/Tests/IntegrationTests/PropagationDenseConstant.cpp +++ b/Tests/IntegrationTests/PropagationDenseConstant.cpp @@ -80,7 +80,7 @@ inline Propagator makePropagator(double bz) { auto magField = std::make_shared(Acts::Vector3(0.0, 0.0, bz)); Stepper stepper(std::move(magField)); - auto logger = getDefaultLogger("Dense", Logging::INFO); + auto logger = getDefaultLogger("Nominal", Logging::INFO); return Propagator( std::move(stepper), Acts::Navigator{{makeDetector()}, logger->cloneWithSuffix("Nav")}, @@ -92,8 +92,12 @@ inline RiddersPropagator makeRiddersPropagator(double bz) { auto magField = std::make_shared(Acts::Vector3(0.0, 0.0, bz)); Stepper stepper(std::move(magField)); - return RiddersPropagator(std::move(stepper), - Acts::Navigator{{makeDetector()}}); + auto logger = getDefaultLogger("Ridder", Logging::INFO); + auto propagator = Propagator( + std::move(stepper), + Acts::Navigator{{makeDetector()}, logger->cloneWithSuffix("Nav")}, + logger->cloneWithSuffix("Prop")); + return RiddersPropagator(std::move(propagator)); } } // namespace diff --git a/Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp b/Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp index a0c875ec6e7..db4ace07e00 100644 --- a/Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp +++ b/Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp @@ -131,8 +131,7 @@ BOOST_AUTO_TEST_CASE(UpdatePortal) { auto cylinderSurface = Acts::Surface::makeShared(nominal, 10., 100.); - auto cylinderPortal = - std::make_shared(cylinderSurface); + auto cylinderPortal = Acts::Experimental::Portal::makeShared(cylinderSurface); fullCylinderVolume->updatePortal(cylinderPortal, 2u); @@ -180,8 +179,9 @@ BOOST_AUTO_TEST_CASE(CuboidWithCuboid) { outerBox->updateNavigationState(tContext, nState); - // We should have 12 candidates, 6 inner, 6 outer portals - BOOST_CHECK_EQUAL(nState.surfaceCandidates.size(), 12u); + // We should have 12 candidates, 6 inner, 6 outer portals but only 3 are + // reachable + BOOST_CHECK_EQUAL(nState.surfaceCandidates.size(), 3u); } BOOST_AUTO_TEST_CASE(CylinderWithSurfacesTestExtractors) { diff --git a/Tests/UnitTests/Core/Geometry/GenericApproachDescriptorTests.cpp b/Tests/UnitTests/Core/Geometry/GenericApproachDescriptorTests.cpp index 154831b2e12..daa7a2944c3 100644 --- a/Tests/UnitTests/Core/Geometry/GenericApproachDescriptorTests.cpp +++ b/Tests/UnitTests/Core/Geometry/GenericApproachDescriptorTests.cpp @@ -66,9 +66,8 @@ BOOST_AUTO_TEST_CASE(GenericApproachDescriptorProperties) { }; Vector3 zDir{0., 0., 1.}; BoundaryCheck bcheck{true}; - double pLimit = std::numeric_limits::max(); - double oLimit = -100 * UnitConstants::um; - double tolerance = s_onSurfaceTolerance; + double nearLimit = -100 * UnitConstants::um; + double farLimit = std::numeric_limits::max(); // std::vector> someSurfaces{ Surface::makeShared(), Surface::makeShared()}; @@ -78,7 +77,7 @@ BOOST_AUTO_TEST_CASE(GenericApproachDescriptorProperties) { BOOST_CHECK_NO_THROW(approachDescriptor.registerLayer(aLayer)); // approachSurface SurfaceIntersection surfIntersection = approachDescriptor.approachSurface( - tgContext, origin, zDir, bcheck, pLimit, oLimit, tolerance); + tgContext, origin, zDir, bcheck, nearLimit, farLimit); double expectedIntersection = 20.0; // property of SurfaceStub CHECK_CLOSE_REL(surfIntersection.pathLength(), expectedIntersection, 1e-6); // containedSurfaces() @@ -98,9 +97,8 @@ BOOST_AUTO_TEST_CASE(GenericApproachNoOverstepping) { Vector3 origin{0., -0.5, 1.}; Vector3 direction{0., 1., 0.}; BoundaryCheck bcheck{true}; - double pLimit = std::numeric_limits::max(); - double oLimit = -100 * UnitConstants::um; - double tolerance = s_onSurfaceTolerance; + double nearLimit = -100 * UnitConstants::um; + double farLimit = std::numeric_limits::max(); auto conCyl = Surface::makeShared(Transform3::Identity(), 10., 20.); @@ -110,7 +108,7 @@ BOOST_AUTO_TEST_CASE(GenericApproachNoOverstepping) { GenericApproachDescriptor gad(approachSurface); auto sfIntersection = gad.approachSurface( - GeometryContext(), origin, direction, bcheck, pLimit, oLimit, tolerance); + GeometryContext(), origin, direction, bcheck, nearLimit, farLimit); // No overstepping allowed, the preferred solution should be the forward one CHECK_CLOSE_ABS(sfIntersection.pathLength(), 10.5, s_epsilon); diff --git a/Tests/UnitTests/Core/Navigation/MultiWireNavigationTests.cpp b/Tests/UnitTests/Core/Navigation/MultiWireNavigationTests.cpp index 9b2b8e048ef..84712cfb47b 100644 --- a/Tests/UnitTests/Core/Navigation/MultiWireNavigationTests.cpp +++ b/Tests/UnitTests/Core/Navigation/MultiWireNavigationTests.cpp @@ -92,8 +92,9 @@ BOOST_AUTO_TEST_CASE(Navigation_in_Indexed_Surfaces) { nState.currentVolume = volumes.front().get(); nState.currentVolume->updateNavigationState(tContext, nState); - // check the surface candidates after update (12 surfaces + 6 portals) - BOOST_CHECK_EQUAL(nState.surfaceCandidates.size(), 18u); + // check the surface candidates after update (12 surfaces + 6 portals but only + // 5 are reachable) + BOOST_CHECK_EQUAL(nState.surfaceCandidates.size(), 5u); } BOOST_AUTO_TEST_SUITE_END() diff --git a/Tests/UnitTests/Core/Vertexing/ImpactPointEstimatorTests.cpp b/Tests/UnitTests/Core/Vertexing/ImpactPointEstimatorTests.cpp index 4020d767a5e..cd983d348e2 100644 --- a/Tests/UnitTests/Core/Vertexing/ImpactPointEstimatorTests.cpp +++ b/Tests/UnitTests/Core/Vertexing/ImpactPointEstimatorTests.cpp @@ -26,7 +26,7 @@ #include "Acts/Propagator/EigenStepper.hpp" #include "Acts/Propagator/Propagator.hpp" #include "Acts/Propagator/StraightLineStepper.hpp" -#include "Acts/Propagator/detail/VoidPropagatorComponents.hpp" +#include "Acts/Propagator/VoidNavigator.hpp" #include "Acts/Surfaces/PerigeeSurface.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Tests/CommonHelpers/FloatComparisons.hpp" @@ -98,7 +98,7 @@ Estimator makeEstimator(double bZ) { Stepper stepper(field); Estimator::Config cfg(field, std::make_shared( - std::move(stepper), detail::VoidNavigator(), + std::move(stepper), VoidNavigator(), getDefaultLogger("Prop", Logging::Level::WARNING))); return Estimator(cfg); }