Skip to content

Commit

Permalink
refactor!: Refactor direct navigator initialization (#3183)
Browse files Browse the repository at this point in the history 
After #3181 and #3182 we can properly initialize the `DirectNavigator` with the surface sequence without a special `Actor` modifying the sequence at the right moment. This is directly applied for the KF and GSF.

blocked by:
- #3181
- #3182
- #3190
  • Loading branch information
@andiwand
andiwand authored Jul 16, 2024
1 parent 3e4cb14 commit 918b464
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 148 deletions.
224 changes: 92 additions & 132 deletions Core/include/Acts/Propagator/DirectNavigator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "Acts/Geometry/TrackingVolume.hpp"
#include "Acts/Propagator/NavigatorOptions.hpp"
#include "Acts/Propagator/Propagator.hpp"
#include "Acts/Surfaces/BoundaryTolerance.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Utilities/Intersection.hpp"

Expand All @@ -34,69 +35,25 @@ class DirectNavigator {
public:
/// The sequentially crossed surfaces
using SurfaceSequence = std::vector<const Surface*>;
using SurfaceIter = std::vector<const Surface*>::iterator;
using SurfaceIter = SurfaceSequence::const_iterator;

struct Config {};

struct Options : public NavigatorPlainOptions {
/// The Surface sequence
SurfaceSequence surfaces;

// 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;

void setPlainOptions(const NavigatorPlainOptions& options) {
static_cast<NavigatorPlainOptions&>(*this) = options;
}
};

/// @brief Nested Actor struct, called Initializer
///
/// This is needed for the initialization of the surface sequence.
struct Initializer {
/// The Surface sequence
SurfaceSequence navSurfaces = {};

/// Actor result / state
struct this_result {
bool initialized = false;
};

using result_type = this_result;

/// Defaulting the constructor
Initializer() = default;

/// Actor operator call
/// @tparam statet Type of the full propagator state
/// @tparam stepper_t Type of the stepper
/// @tparam navigator_t Type of the navigator
///
/// @param state the entire propagator state
/// @param r the result of this Actor
template <typename propagator_state_t, typename stepper_t,
typename navigator_t>
void operator()(propagator_state_t& state, const stepper_t& /*stepper*/,
const navigator_t& /*navigator*/, result_type& r,
const Logger& /*logger*/) const {
// Only act once
if (!r.initialized) {
// Initialize the surface sequence
state.navigation.navSurfaces = navSurfaces;
state.navigation.navSurfaceIter = state.navigation.navSurfaces.begin();

// 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.options.startSurface) {
auto surfaceIter = std::find(state.navigation.navSurfaces.begin(),
state.navigation.navSurfaces.end(),
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;
}
}

r.initialized = true;
}
}
};

/// @brief Nested State struct
///
/// It acts as an internal state which is created for every
Expand All @@ -105,11 +62,8 @@ class DirectNavigator {
struct State {
Options options;

/// Externally provided surfaces - expected to be ordered along the path
SurfaceSequence navSurfaces = {};

/// Iterator the next surface
SurfaceIter navSurfaceIter = navSurfaces.begin();
/// Index of the next surface to try
std::size_t surfaceIndex = 0;

/// Navigation state - external interface: the current surface
const Surface* currentSurface = nullptr;
Expand All @@ -120,8 +74,9 @@ class DirectNavigator {
bool navigationBreak = false;
};

DirectNavigator(std::unique_ptr<const Logger> _logger =
getDefaultLogger("DirectNavigator", Logging::INFO))
explicit DirectNavigator(std::unique_ptr<const Logger> _logger =
getDefaultLogger("DirectNavigator",
Logging::INFO))
: m_logger{std::move(_logger)} {}

State makeState(const Options& options) const {
Expand Down Expand Up @@ -202,39 +157,15 @@ class DirectNavigator {

// Navigator target always resets the current surface
state.navigation.currentSurface = nullptr;

// Output the position in the sequence
ACTS_VERBOSE(std::distance(state.navigation.navSurfaceIter,
state.navigation.navSurfaces.end())
<< " out of " << state.navigation.navSurfaces.size()
ACTS_VERBOSE((state.navigation.options.surfaces.size() -
state.navigation.surfaceIndex)
<< " out of " << state.navigation.options.surfaces.size()
<< " surfaces remain to try.");

if (state.navigation.navSurfaceIter != state.navigation.navSurfaces.end()) {
// 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<double>::max();
const auto index =
chooseIntersection(
state.geoContext, surface, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
BoundaryTolerance::Infinite(), m_nearLimit, farLimit,
state.options.surfaceTolerance)
.index();
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, surface, index, state.options.direction,
BoundaryTolerance::Infinite(), state.options.surfaceTolerance,
*m_logger);
if (surfaceStatus == Intersection3D::Status::unreachable) {
ACTS_VERBOSE(
"Surface not reachable anymore, switching to next one in "
"sequence");
// Move the sequence to the next surface
++state.navigation.navSurfaceIter;
} else {
ACTS_VERBOSE("Navigation stepSize set to "
<< stepper.outputStepSize(state.stepping));
}
} else {
if (state.navigation.surfaceIndex >=
state.navigation.options.surfaces.size()) {
// Set the navigation break
state.navigation.navigationBreak = true;
// If no externally provided target is given, the target is reached
Expand All @@ -243,6 +174,34 @@ class DirectNavigator {
// Announce it then
ACTS_VERBOSE("No target Surface, job done.");
}
return;
}

// Establish & update the surface status
// TODO we do not know the intersection index - passing the closer one
const auto& surface =
*state.navigation.options.surfaces.at(state.navigation.surfaceIndex);
const double farLimit = std::numeric_limits<double>::max();
const auto index =
chooseIntersection(
state.geoContext, surface, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
BoundaryTolerance::Infinite(), state.navigation.options.nearLimit,
farLimit, state.options.surfaceTolerance)
.index();
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, surface, index, state.options.direction,
BoundaryTolerance::Infinite(), state.options.surfaceTolerance,
*m_logger);
if (surfaceStatus == Intersection3D::Status::unreachable) {
ACTS_VERBOSE(
"Surface not reachable anymore, switching to next one in "
"sequence");
// Move the sequence to the next surface
++state.navigation.surfaceIndex;
} else {
ACTS_VERBOSE("Navigation stepSize set to "
<< stepper.outputStepSize(state.stepping));
}
}

Expand All @@ -259,45 +218,52 @@ class DirectNavigator {

// Navigator post step always resets the current surface
state.navigation.currentSurface = nullptr;

// Output the position in the sequence
ACTS_VERBOSE(std::distance(state.navigation.navSurfaceIter,
state.navigation.navSurfaces.end())
<< " out of " << state.navigation.navSurfaces.size()
ACTS_VERBOSE((state.navigation.options.surfaces.size() -
state.navigation.surfaceIndex)
<< " out of " << state.navigation.options.surfaces.size()
<< " surfaces remain to try.");

// Check if we are on surface
if (state.navigation.navSurfaceIter != state.navigation.navSurfaces.end()) {
// 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<double>::max();
const auto index =
chooseIntersection(
state.geoContext, surface, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
BoundaryTolerance::Infinite(), m_nearLimit, farLimit,
state.options.surfaceTolerance)
.index();
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, surface, index, state.options.direction,
BoundaryTolerance::Infinite(), state.options.surfaceTolerance,
*m_logger);
if (surfaceStatus == Intersection3D::Status::onSurface) {
// Set the current surface
state.navigation.currentSurface = *state.navigation.navSurfaceIter;
ACTS_VERBOSE("Current surface set to "
<< state.navigation.currentSurface->geometryId());
// Move the sequence to the next surface
++state.navigation.navSurfaceIter;
if (state.navigation.navSurfaceIter !=
state.navigation.navSurfaces.end()) {
ACTS_VERBOSE("Next surface candidate is "
<< (*state.navigation.navSurfaceIter)->geometryId());
}
} else if (surfaceStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE("Next surface reachable at distance "
<< stepper.outputStepSize(state.stepping));
if (state.navigation.surfaceIndex >=
state.navigation.options.surfaces.size()) {
return;
}

// Establish the surface status
// TODO we do not know the intersection index - passing the closer one
const auto& surface =
*state.navigation.options.surfaces.at(state.navigation.surfaceIndex);
const double farLimit = std::numeric_limits<double>::max();
const auto index =
chooseIntersection(
state.geoContext, surface, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
BoundaryTolerance::Infinite(), state.navigation.options.nearLimit,
farLimit, state.options.surfaceTolerance)
.index();
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, surface, index, state.options.direction,
BoundaryTolerance::Infinite(), state.options.surfaceTolerance,
*m_logger);
if (surfaceStatus == Intersection3D::Status::onSurface) {
// Set the current surface
state.navigation.currentSurface =
state.navigation.options.surfaces.at(state.navigation.surfaceIndex);
ACTS_VERBOSE("Current surface set to "
<< state.navigation.currentSurface->geometryId());
// Move the sequence to the next surface
++state.navigation.surfaceIndex;
if (state.navigation.surfaceIndex >=
state.navigation.options.surfaces.size()) {
ACTS_VERBOSE("Next surface candidate is "
<< state.navigation.options.surfaces
.at(state.navigation.surfaceIndex)
->geometryId());
}
} else if (surfaceStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE("Next surface reachable at distance "
<< stepper.outputStepSize(state.stepping));
}
}

Expand All @@ -323,12 +289,6 @@ class DirectNavigator {
const Logger& logger() const { return *m_logger; }

std::unique_ptr<const Logger> 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
10 changes: 4 additions & 6 deletions Core/include/Acts/TrackFitting/GaussianSumFitter.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ struct GaussianSumFitter {

// Initialize the forward propagation with the DirectNavigator
auto fwdPropInitializer = [&sSequence, this](const auto& opts) {
using Actors = ActionList<GsfActor, DirectNavigator::Initializer>;
using Actors = ActionList<GsfActor>;
using Aborters = AbortList<NavigationBreakAborter>;
using PropagatorOptions =
typename propagator_t::template Options<Actors, Aborters>;
Expand All @@ -113,8 +113,7 @@ struct GaussianSumFitter {

propOptions.setPlainOptions(opts.propagatorPlainOptions);

propOptions.actionList.template get<DirectNavigator::Initializer>()
.navSurfaces = sSequence;
propOptions.navigation.surfaces = sSequence;
propOptions.actionList.template get<GsfActor>()
.m_cfg.bethe_heitler_approx = &m_betheHeitlerApproximation;

Expand All @@ -123,7 +122,7 @@ struct GaussianSumFitter {

// Initialize the backward propagation with the DirectNavigator
auto bwdPropInitializer = [&sSequence, this](const auto& opts) {
using Actors = ActionList<GsfActor, DirectNavigator::Initializer>;
using Actors = ActionList<GsfActor>;
using Aborters = AbortList<>;
using PropagatorOptions =
typename propagator_t::template Options<Actors, Aborters>;
Expand All @@ -136,8 +135,7 @@ struct GaussianSumFitter {

propOptions.setPlainOptions(opts.propagatorPlainOptions);

propOptions.actionList.template get<DirectNavigator::Initializer>()
.navSurfaces = std::move(backwardSequence);
propOptions.navigation.surfaces = backwardSequence;
propOptions.actionList.template get<GsfActor>()
.m_cfg.bethe_heitler_approx = &m_betheHeitlerApproximation;

Expand Down
6 changes: 2 additions & 4 deletions Core/include/Acts/TrackFitting/KalmanFitter.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,7 @@ class KalmanFitter {
using KalmanActor = Actor<parameters_t>;

using KalmanResult = typename KalmanActor::result_type;
using Actors = ActionList<DirectNavigator::Initializer, KalmanActor>;
using Actors = ActionList<KalmanActor>;
using Aborters = AbortList<KalmanAborter>;
using PropagatorOptions =
typename propagator_t::template Options<Actors, Aborters>;
Expand All @@ -1233,9 +1233,7 @@ class KalmanFitter {
kalmanActor.actorLogger = m_actorLogger.get();

// Set the surface sequence
auto& dInitializer = propagatorOptions.actionList
.template get<DirectNavigator::Initializer>();
dInitializer.navSurfaces = sSequence;
propagatorOptions.navigation.surfaces = sSequence;

return fit_impl<start_parameters_t, PropagatorOptions, KalmanResult,
track_container_t, holder_t>(sParameters, propagatorOptions,
Expand Down
7 changes: 2 additions & 5 deletions Tests/UnitTests/Core/Propagator/DirectNavigatorTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,14 @@ void runTest(const rpropagator_t& rprop, const dpropagator_t& dprop, double pT,
}

// Action list for direct navigator with its initializer
using DirectActionList = ActionList<DirectNavigator::Initializer,
MaterialInteractor, SurfaceCollector<>>;
using DirectActionList = ActionList<MaterialInteractor, SurfaceCollector<>>;

// Direct options definition
using DirectOptions =
typename dpropagator_t::template Options<DirectActionList, AbortList<>>;
DirectOptions dOptions(tgContext, mfContext);
// Set the surface sequence
auto& dInitializer =
dOptions.actionList.template get<DirectNavigator::Initializer>();
dInitializer.navSurfaces = surfaceSequence;
dOptions.navigation.surfaces = surfaceSequence;
// Surface collector configuration
auto& dCollector = dOptions.actionList.template get<SurfaceCollector<>>();
dCollector.selector.selectSensitive = true;
Expand Down
2 changes: 1 addition & 1 deletion docs/core/propagation.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ if( res.ok() ) {

ACTS comes with a couple of different navigator implementations:
- The standard navigator {class}`Acts::Navigator` which performs the full navigation in the volume/layer/surface hierarchy
- The {class}`Acts::DirectNavigator` which takes a sequence of surfaces and just navigates to one after the other. This sequence must be initialized with a special actor, the {struct}`Acts::DirectNavigator::Initializer`.
- The {class}`Acts::DirectNavigator` which takes a sequence of surfaces and just navigates to one after the other.
- The {class}`Acts::TryAllNavigator` which, as the name suggests, tries to intersect all available surfaces without acceleration structure and special assumptions. This navigator is meant for validation rather than production.
- The {class}`Acts::TryAllOverstepNavigator` which is similar to the {class}`Acts::TryAllNavigator`, but deliberately oversteps and then intersects surfaces which might have been missed by that step and targets them. This navigator is meant for validation rather than production.
- The {class}`Acts::Experimental::DetectorNavigator` which performs the full navigation in the gen2 geometry. Note that this is still experimental and should not be included in production code as it might be unstable and break with minor version updates of ACTS.
Expand Down

0 comments on commit 918b464

Please sign in to comment.