Skip to content

Commit

Permalink
refactor!: Rename and rearrange propagator options (#2620)
Browse files Browse the repository at this point in the history
Discovered in #2619 that the propagator options might be misleading
  • Loading branch information
andiwand authored Nov 4, 2023
1 parent eb12644 commit c578923
Show file tree
Hide file tree
Showing 17 changed files with 61 additions and 75 deletions.
6 changes: 3 additions & 3 deletions Core/include/Acts/Navigation/DetectorNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,8 @@ class DetectorNavigator {
bool boundaryCheck = c.boundaryCheck;
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, surface, c.objectIntersection.index(),
state.options.direction, boundaryCheck, state.options.targetTolerance,
logger());
state.options.direction, boundaryCheck,
state.options.surfaceTolerance, logger());
if (surfaceStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE(volInfo(state)
<< posInfo(state, stepper)
Expand Down Expand Up @@ -299,7 +299,7 @@ class DetectorNavigator {
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, *nextSurface,
nState.surfaceCandidate->objectIntersection.index(),
state.options.direction, boundaryCheck, state.options.targetTolerance,
state.options.direction, boundaryCheck, state.options.surfaceTolerance,
logger());

// Check if we are at a surface
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Propagator/AtlasStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ class AtlasStepper {
2. * h *
(std::abs((A1 + A6) - (A3 + A4)) + std::abs((B1 + B6) - (B3 + B4)) +
std::abs((C1 + C6) - (C3 + C4)));
if (std::abs(EST) > std::abs(state.options.tolerance)) {
if (std::abs(EST) > std::abs(state.options.stepTolerance)) {
h = h * .5;
// neutralize the sign of h again
state.stepping.stepSize.setAccuracy(h * state.options.direction);
Expand Down
18 changes: 6 additions & 12 deletions Core/include/Acts/Propagator/DenseEnvironmentExtension.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,25 +56,19 @@ struct DenseStepperPropagatorOptions
extended_aborter_list_t aborters) const {
DenseStepperPropagatorOptions<action_list_t, extended_aborter_list_t>
eoptions(this->geoContext, this->magFieldContext);

// Copy the options over
eoptions.direction = this->direction;
eoptions.maxSteps = this->maxSteps;
eoptions.maxStepSize = this->maxStepSize;
eoptions.targetTolerance = this->targetTolerance;
eoptions.pathLimit = this->pathLimit;
eoptions.loopProtection = this->loopProtection;
eoptions.loopFraction = this->loopFraction;

// Stepper options
eoptions.tolerance = this->tolerance;
eoptions.stepSizeCutOff = this->stepSizeCutOff;
eoptions.setPlainOptions(*this);

// Action / abort list
eoptions.actionList = this->actionList;
eoptions.actionList = std::move(this->actionList);
eoptions.abortList = std::move(aborters);

// Copy dense environment specific parameters
eoptions.meanEnergyLoss = meanEnergyLoss;
eoptions.includeGgradient = includeGgradient;
eoptions.momentumCutOff = momentumCutOff;

// And return the options
return eoptions;
}
Expand Down
8 changes: 4 additions & 4 deletions Core/include/Acts/Propagator/DirectNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,11 @@ class DirectNavigator {
state.options.direction * stepper.direction(state.stepping),
false, std::numeric_limits<double>::max(),
stepper.overstepLimit(state.stepping),
state.options.targetTolerance)
state.options.surfaceTolerance)
.index();
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, surface, index, state.options.direction, false,
state.options.targetTolerance, *m_logger);
state.options.surfaceTolerance, *m_logger);
if (surfaceStatus == Intersection3D::Status::unreachable) {
ACTS_VERBOSE(
"Surface not reachable anymore, switching to next one in "
Expand Down Expand Up @@ -315,11 +315,11 @@ class DirectNavigator {
state.options.direction * stepper.direction(state.stepping),
false, std::numeric_limits<double>::max(),
stepper.overstepLimit(state.stepping),
state.options.targetTolerance)
state.options.surfaceTolerance)
.index();
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, surface, index, state.options.direction, false,
state.options.targetTolerance, *m_logger);
state.options.surfaceTolerance, *m_logger);
if (surfaceStatus == Intersection3D::Status::onSurface) {
// Set the current surface
state.navigation.currentSurface = *state.navigation.navSurfaceIter;
Expand Down
6 changes: 3 additions & 3 deletions Core/include/Acts/Propagator/EigenStepper.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ Acts::Result<double> Acts::EigenStepper<E, A>::step(
std::abs(sd.kQoP[0] - sd.kQoP[1] - sd.kQoP[2] + sd.kQoP[3]));
error_estimate = std::max(error_estimate, 1e-20);

return success(error_estimate <= state.options.tolerance);
return success(error_estimate <= state.options.stepTolerance);
};

const double initialH =
Expand All @@ -204,7 +204,7 @@ Acts::Result<double> Acts::EigenStepper<E, A>::step(

const double stepSizeScaling =
std::min(std::max(0.25f, std::sqrt(std::sqrt(static_cast<float>(
state.options.tolerance /
state.options.stepTolerance /
std::abs(2. * error_estimate))))),
4.0f);
h *= stepSizeScaling;
Expand Down Expand Up @@ -257,7 +257,7 @@ Acts::Result<double> Acts::EigenStepper<E, A>::step(
const double stepSizeScaling = std::min(
std::max(0.25f,
std::sqrt(std::sqrt(static_cast<float>(
state.options.tolerance / std::abs(error_estimate))))),
state.options.stepTolerance / std::abs(error_estimate))))),
4.0f);
const double nextAccuracy = std::abs(h * stepSizeScaling);
const double previousAccuracy = std::abs(state.stepping.stepSize.accuracy());
Expand Down
16 changes: 8 additions & 8 deletions Core/include/Acts/Propagator/Navigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ class Navigator {
// it the current one to pass it to the other actors
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, *surface, intersection.index(), state.options.direction,
true, state.options.targetTolerance, logger());
true, state.options.surfaceTolerance, logger());
if (surfaceStatus == Intersection3D::Status::onSurface) {
ACTS_VERBOSE(volInfo(state)
<< "Status Surface successfully hit, storing it.");
Expand Down Expand Up @@ -693,8 +693,8 @@ class Navigator {
}
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, *surface, intersection.index(),
state.options.direction, boundaryCheck, state.options.targetTolerance,
logger());
state.options.direction, boundaryCheck,
state.options.surfaceTolerance, logger());
if (surfaceStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE(volInfo(state)
<< "Surface reachable, step size updated to "
Expand Down Expand Up @@ -864,7 +864,7 @@ class Navigator {
// Try to step towards it
auto layerStatus = stepper.updateSurfaceStatus(
state.stepping, *layerSurface, intersection.index(),
state.options.direction, true, state.options.targetTolerance,
state.options.direction, true, state.options.surfaceTolerance,
logger());
if (layerStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE(volInfo(state) << "Layer reachable, step size updated to "
Expand Down Expand Up @@ -1009,7 +1009,7 @@ class Navigator {
// Step towards the boundary surfrace
auto boundaryStatus = stepper.updateSurfaceStatus(
state.stepping, *boundarySurface, intersection.index(),
state.options.direction, true, state.options.targetTolerance,
state.options.direction, true, state.options.surfaceTolerance,
logger());
if (boundaryStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE(volInfo(state)
Expand Down Expand Up @@ -1093,7 +1093,7 @@ class Navigator {
->intersect(
state.geoContext, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
false, state.options.targetTolerance)
false, state.options.surfaceTolerance)
.closest();
if (targetIntersection) {
ACTS_VERBOSE(volInfo(state)
Expand Down Expand Up @@ -1165,7 +1165,7 @@ class Navigator {
stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
// No overstepping on start layer, otherwise ask the stepper
navOpts.overstepLimit = (cLayer != nullptr)
? state.options.targetTolerance
? state.options.surfaceTolerance
: stepper.overstepLimit(state.stepping);

// get the surfaces
Expand Down Expand Up @@ -1316,7 +1316,7 @@ class Navigator {
// TODO we do not know the intersection index - passing 0
auto targetStatus = stepper.updateSurfaceStatus(
state.stepping, *state.navigation.targetSurface, 0,
state.options.direction, true, state.options.targetTolerance,
state.options.direction, true, state.options.surfaceTolerance,
logger());
// the only advance could have been to the target
if (targetStatus == Intersection3D::Status::onSurface) {
Expand Down
43 changes: 19 additions & 24 deletions Core/include/Acts/Propagator/Propagator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,28 +64,29 @@ struct PropagatorPlainOptions {
/// Maximum number of steps for one propagate call
unsigned int maxSteps = 1000;

/// Maximum number of Runge-Kutta steps for the stepper step call
unsigned int maxRungeKuttaStepTrials = 10000;

/// Absolute maximum step size
double maxStepSize = std::numeric_limits<double>::max();

/// Absolute maximum path length
double pathLimit = std::numeric_limits<double>::max();

/// Required tolerance to reach target (surface, pathlength)
double targetTolerance = s_onSurfaceTolerance;
/// Required tolerance to reach surface
double surfaceTolerance = s_onSurfaceTolerance;

/// Loop protection step, it adapts the pathLimit
bool loopProtection = true;
double loopFraction = 0.5; ///< Allowed loop fraction, 1 is a full loop

// Configurations for Stepper

/// Tolerance for the error of the integration
double tolerance = 1e-4;
double stepTolerance = 1e-4;

/// Cut-off value for the step size
double stepSizeCutOff = 0.;

/// Absolute maximum step size
double maxStepSize = std::numeric_limits<double>::max();

/// Maximum number of Runge-Kutta steps for the stepper step call
unsigned int maxRungeKuttaStepTrials = 10000;
};

/// @brief Options for propagate() call
Expand Down Expand Up @@ -124,22 +125,14 @@ struct PropagatorOptions : public PropagatorPlainOptions {
extended_aborter_list_t aborters) const {
PropagatorOptions<action_list_t, extended_aborter_list_t> eoptions(
geoContext, magFieldContext);

// Copy the options over
eoptions.direction = direction;
eoptions.maxSteps = maxSteps;
eoptions.maxRungeKuttaStepTrials = maxRungeKuttaStepTrials;
eoptions.maxStepSize = maxStepSize;
eoptions.targetTolerance = targetTolerance;
eoptions.pathLimit = direction * std::abs(pathLimit);
eoptions.loopProtection = loopProtection;
eoptions.loopFraction = loopFraction;
eoptions.setPlainOptions(*this);

// Stepper options
eoptions.tolerance = tolerance;
eoptions.stepSizeCutOff = stepSizeCutOff;
// Action / abort list
eoptions.actionList = std::move(actionList);
eoptions.abortList = std::move(aborters);

// And return the options
return eoptions;
}
Expand All @@ -151,14 +144,16 @@ struct PropagatorOptions : public PropagatorPlainOptions {
// Copy the options over
direction = pOptions.direction;
maxSteps = pOptions.maxSteps;
maxRungeKuttaStepTrials = pOptions.maxRungeKuttaStepTrials;
maxStepSize = pOptions.maxStepSize;
targetTolerance = pOptions.targetTolerance;
surfaceTolerance = pOptions.surfaceTolerance;
pathLimit = direction * std::abs(pOptions.pathLimit);
loopProtection = pOptions.loopProtection;
loopFraction = pOptions.loopFraction;
tolerance = pOptions.tolerance;

// Stepper options
stepTolerance = pOptions.stepTolerance;
stepSizeCutOff = pOptions.stepSizeCutOff;
maxStepSize = pOptions.maxStepSize;
maxRungeKuttaStepTrials = pOptions.maxRungeKuttaStepTrials;
}

/// List of actions
Expand Down
4 changes: 2 additions & 2 deletions Core/include/Acts/Propagator/StandardAborters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct PathLimitReached {
// Check if the maximum allowed step size has to be updated
double distance =
std::abs(internalLimit) - std::abs(state.stepping.pathAccumulated);
double tolerance = state.options.targetTolerance;
double tolerance = state.options.surfaceTolerance;
bool limitReached = (std::abs(distance) < std::abs(tolerance));
if (limitReached) {
ACTS_VERBOSE("Target: x | "
Expand Down Expand Up @@ -145,7 +145,7 @@ struct SurfaceReached {
// TODO the following code is mostly duplicated in updateSingleSurfaceStatus

// Calculate the distance to the surface
const double tolerance = state.options.targetTolerance;
const double tolerance = state.options.surfaceTolerance;

const auto sIntersection = targetSurface.intersect(
state.geoContext, stepper.position(state.stepping),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,7 @@ class CombinatorialKalmanFilter {
->intersect(
state.geoContext, freeVector.segment<3>(eFreePos0),
state.options.direction * freeVector.segment<3>(eFreeDir0),
true, state.options.targetTolerance)
true, state.options.surfaceTolerance)
.closest();
};

Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/TrackFitting/KalmanFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ class KalmanFitter {
->intersect(
state.geoContext, freeVector.segment<3>(eFreePos0),
state.options.direction * freeVector.segment<3>(eFreeDir0),
true, state.options.targetTolerance)
true, state.options.surfaceTolerance)
.closest();
};

Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Acts::Result<Acts::LinearizedTrack> Acts::

// Length scale at which we consider to be sufficiently close to the Perigee
// surface to skip the propagation.
pOptions.targetTolerance = m_cfg.targetTolerance;
pOptions.surfaceTolerance = m_cfg.targetTolerance;

// Get intersection of the track with the Perigee if the particle would
// move on a straight line.
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Vertexing/NumericalTrackLinearizer.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Acts::NumericalTrackLinearizer<propagator_t, propagator_options_t>::

// Length scale at which we consider to be sufficiently close to the Perigee
// surface to skip the propagation.
pOptions.targetTolerance = m_cfg.targetTolerance;
pOptions.surfaceTolerance = m_cfg.targetTolerance;

// Get intersection of the track with the Perigee if the particle would
// move on a straight line.
Expand Down
8 changes: 4 additions & 4 deletions Tests/IntegrationTests/PropagationTests.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,8 @@ inline std::pair<Acts::CurvilinearTrackParameters, double> transportFreely(
options_t<Actions, Aborts> options(geoCtx, magCtx);
options.direction = Acts::Direction::fromScalar(pathLength);
options.pathLimit = pathLength;
options.targetTolerance = 1_nm;
options.tolerance = 1_nm;
options.surfaceTolerance = 1_nm;
options.stepTolerance = 1_nm;

auto result = propagator.propagate(initialParams, options);
BOOST_CHECK(result.ok());
Expand All @@ -276,8 +276,8 @@ inline std::pair<Acts::BoundTrackParameters, double> transportToSurface(
options_t<Actions, Aborts> options(geoCtx, magCtx);
options.direction = Acts::Direction::Forward;
options.pathLimit = pathLimit;
options.targetTolerance = 1_nm;
options.tolerance = 1_nm;
options.surfaceTolerance = 1_nm;
options.stepTolerance = 1_nm;

auto result = propagator.propagate(initialParams, targetSurface, options);
BOOST_CHECK(result.ok());
Expand Down
2 changes: 1 addition & 1 deletion Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ struct MockPropagatorState {
Stepper::State stepping;
/// Propagator options with only the relevant components.
struct {
double tolerance = 10_um;
double stepTolerance = 10_um;
Direction direction = Direction::Backward;
} options;
};
Expand Down
8 changes: 4 additions & 4 deletions Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ struct PropState {
stepper_state_t stepping;
/// Propagator options which only carry the relevant components
struct {
double tolerance = 1e-4;
double stepTolerance = 1e-4;
double stepSizeCutOff = 0.;
unsigned int maxRungeKuttaStepTrials = 10000;
Direction direction = Direction::Forward;
Expand Down Expand Up @@ -135,7 +135,7 @@ struct EndOfWorld {
bool operator()(propagator_state_t& state, const stepper_t& stepper,
const navigator_t& /*navigator*/,
const Logger& /*logger*/) const {
const double tolerance = state.options.targetTolerance;
const double tolerance = state.options.surfaceTolerance;
if (maxX - std::abs(stepper.position(state.stepping).x()) <= tolerance ||
std::abs(stepper.position(state.stepping).y()) >= 0.5_m ||
std::abs(stepper.position(state.stepping).z()) >= 0.5_m) {
Expand Down Expand Up @@ -517,7 +517,7 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) {
CHECK_CLOSE_COVARIANCE(esState.cov, Covariance(2. * cov), eps);

// Test a case where no step size adjustment is required
ps.options.tolerance = 2. * 4.4258e+09;
ps.options.stepTolerance = 2. * 4.4258e+09;
double h0 = esState.stepSize.value();
es.step(ps, mockNavigator);
CHECK_CLOSE_ABS(h0, esState.stepSize.value(), eps);
Expand All @@ -529,7 +529,7 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) {
stepSize);
PropState nps(navDir, copyState(*nBfield, nesState));
// Test that we can reach the minimum step size
nps.options.tolerance = 1e-21;
nps.options.stepTolerance = 1e-21;
nps.options.stepSizeCutOff = 1e20;
auto res = nes.step(nps, mockNavigator);
BOOST_CHECK(!res.ok());
Expand Down
2 changes: 1 addition & 1 deletion Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const auto defaultNullBField = std::make_shared<NullBField>();
const auto particleHypothesis = ParticleHypothesis::pion();

struct Options {
double tolerance = 1e-4;
double stepTolerance = 1e-4;
double stepSizeCutOff = 0.0;
std::size_t maxRungeKuttaStepTrials = 10;
Direction direction = defaultNDir;
Expand Down
Loading

0 comments on commit c578923

Please sign in to comment.