Skip to content

Commit

Permalink
refactor!: stepper-state must not contain a stepSize object anymore (#…
Browse files Browse the repository at this point in the history 
…1113)

This PR is in preparation for the MultiStepper PR #1110

Since a single stepSize isn't useful for a multi-component state, this removes the requirement to have such an object and add/modifies the accessors in the steppers. In particular it does:

Modify the stepper.setStepSize(...) function to allow it to replace calls to stepSize.update(...)`
This is done by adding an optional bool parameter to control wether or not release the stepSize. This defaults to true, so the default behaviour is unchanged, so I think this is not breaking.
This does also modify the previousStepSize in the stepper-states. But I think this is not problematic, since this variable seems to be used just in unit tests, which still run through as far as I can tell.
    state.stepping.stepSize.update(size, ConstrainedStep::aborter);
    // ->
    stepper.setStepSize(state.stepping, size, ConstrainedStep::aborter, false);
Add the stepper-method stepper.getStepSize(...) to the steppers. This does change the StepperConcept and thus is a breaking change I think, since custom steppers might not work anymore.
    state.stepping.stepSize.value(ConstrainedStep::aborter);
    // ->
    stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
Use stepper.outputStepSize(...) instead of stepSize.toString() when necessary.
BREAKING CHANGE: The StepperConcept does now require a getStepSize(...) method.
  • Loading branch information
@benjaminhuth
benjaminhuth committed Jan 5, 2022
1 parent 6414195 commit c62039a
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 24 deletions.
14 changes: 12 additions & 2 deletions Core/include/Acts/Propagator/AtlasStepper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,10 +543,20 @@ class AtlasStepper {
/// @param [in,out] state The stepping state (thread-local cache)
/// @param [in] stepSize The step size value
/// @param [in] stype The step size type to be set
/// @param release [in] Do we release the step size?
void setStepSize(State& state, double stepSize,
ConstrainedStep::Type stype = ConstrainedStep::actor) const {
ConstrainedStep::Type stype = ConstrainedStep::actor,
bool release = true) const {
state.previousStepSize = state.stepSize;
state.stepSize.update(stepSize, stype, true);
state.stepSize.update(stepSize, stype, release);
}

/// Get the step size
///
/// @param state [in] The stepping state (thread-local cache)
/// @param stype [in] The step size type to be returned
double getStepSize(const State& state, ConstrainedStep::Type stype) const {
return state.stepSize.value(stype);
}

/// Release the Step size
Expand Down
14 changes: 12 additions & 2 deletions Core/include/Acts/Propagator/EigenStepper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,20 @@ class EigenStepper {
/// @param state [in,out] The stepping state (thread-local cache)
/// @param stepSize [in] The step size value
/// @param stype [in] The step size type to be set
/// @param release [in] Do we release the step size?
void setStepSize(State& state, double stepSize,
ConstrainedStep::Type stype = ConstrainedStep::actor) const {
ConstrainedStep::Type stype = ConstrainedStep::actor,
bool release = true) const {
state.previousStepSize = state.stepSize;
state.stepSize.update(stepSize, stype, true);
state.stepSize.update(stepSize, stype, release);
}

/// Get the step size
///
/// @param state [in] The stepping state (thread-local cache)
/// @param stype [in] The step size type to be returned
double getStepSize(const State& state, ConstrainedStep::Type stype) const {
return state.stepSize.value(stype);
}

/// Release the Step size
Expand Down
8 changes: 5 additions & 3 deletions Core/include/Acts/Propagator/Navigator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@ class Navigator {
// The navigation options
NavigationOptions<Surface> navOpts(state.stepping.navDir, true);
navOpts.pathLimit =
state.stepping.stepSize.value(ConstrainedStep::aborter);
stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
navOpts.overstepLimit = stepper.overstepLimit(state.stepping);

// Exclude the current surface in case it's a boundary
Expand Down Expand Up @@ -1128,7 +1128,8 @@ class Navigator {
}
}
// Check the limit
navOpts.pathLimit = state.stepping.stepSize.value(ConstrainedStep::aborter);
navOpts.pathLimit =
stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
// No overstepping on start layer, otherwise ask the stepper
navOpts.overstepLimit = (cLayer != nullptr)
? s_onSurfaceTolerance
Expand Down Expand Up @@ -1197,7 +1198,8 @@ class Navigator {
m_cfg.resolveMaterial, m_cfg.resolvePassive, startLayer, nullptr);
// Set also the target surface
navOpts.targetSurface = state.navigation.targetSurface;
navOpts.pathLimit = state.stepping.stepSize.value(ConstrainedStep::aborter);
navOpts.pathLimit =
stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
navOpts.overstepLimit = stepper.overstepLimit(state.stepping);
// Request the compatible layers
state.navigation.navLayers =
Expand Down
16 changes: 9 additions & 7 deletions Core/include/Acts/Propagator/StandardAborters.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ struct PathLimitReached {
/// @tparam stepper_t Type of the stepper
///
/// @param [in,out] state The propagation state object
/// @param [in] stepper Stepper used for propagation
template <typename propagator_state_t, typename stepper_t>
bool operator()(propagator_state_t& state,
const stepper_t& /*unused*/) const {
bool operator()(propagator_state_t& state, const stepper_t& stepper) const {
const auto& logger = state.options.logger;
if (state.navigation.targetReached) {
return true;
Expand All @@ -61,7 +61,8 @@ struct PathLimitReached {
double distance = state.stepping.navDir * std::abs(internalLimit) -
state.stepping.pathAccumulated;
double tolerance = state.options.targetTolerance;
state.stepping.stepSize.update(distance, ConstrainedStep::aborter);
stepper.setStepSize(state.stepping, distance, ConstrainedStep::aborter,
false);
bool limitReached = (distance * distance < tolerance * tolerance);
if (limitReached) {
ACTS_VERBOSE("Target: x | "
Expand All @@ -71,7 +72,7 @@ struct PathLimitReached {
} else {
ACTS_VERBOSE("Target: 0 | "
<< "Target stepSize (path limit) updated to "
<< state.stepping.stepSize.toString());
<< stepper.outputStepSize(state.stepping));
}
// path limit check
return limitReached;
Expand Down Expand Up @@ -152,11 +153,12 @@ struct SurfaceReached {
// Update the distance to the alternative solution
distance = sIntersection.alternative.pathLength;
}
state.stepping.stepSize.update(state.stepping.navDir * distance,
ConstrainedStep::aborter);
stepper.setStepSize(state.stepping, state.stepping.navDir * distance,
ConstrainedStep::aborter, false);

ACTS_VERBOSE("Target: 0 | "
<< "Target stepSize (surface) updated to "
<< state.stepping.stepSize.toString());
<< stepper.outputStepSize(state.stepping));
}
// path limit check
return targetReached;
Expand Down
9 changes: 6 additions & 3 deletions Core/include/Acts/Propagator/StepperConcept.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ METHOD_TRAIT(covariance_transport_curvilinear_t,
METHOD_TRAIT(step_t, step);
METHOD_TRAIT(update_surface_status_t, updateSurfaceStatus);
METHOD_TRAIT(set_step_size_t, setStepSize);
METHOD_TRAIT(get_step_size_t, getStepSize);
METHOD_TRAIT(release_step_size_t, releaseStepSize);
METHOD_TRAIT(output_step_size_t, outputStepSize);

Expand All @@ -72,8 +73,8 @@ using step_size_t = decltype(std::declval<T>().stepSize);
= require<has_member<S, cov_transport_t, bool>,
has_member<S, cov_t, BoundSymMatrix>,
has_member<S, nav_dir_t, NavigationDirection>,
has_member<S, path_accumulated_t, double>,
has_member<S, step_size_t, ConstrainedStep>
has_member<S, path_accumulated_t, double>//,
// has_member<S, step_size_t, ConstrainedStep>
>;
// clang-format on

Expand Down Expand Up @@ -118,8 +119,10 @@ using step_size_t = decltype(std::declval<T>().stepSize);
static_assert(covariance_transport_exists, "covarianceTransport method not found");
constexpr static bool update_surface_exists = has_method<const S, Intersection3D::Status, update_surface_status_t, state&, const Surface&, const BoundaryCheck&, LoggerWrapper>;
static_assert(update_surface_exists, "updateSurfaceStatus method not found");
constexpr static bool set_step_size_exists = has_method<const S, void, set_step_size_t, state&, double, ConstrainedStep::Type>;
constexpr static bool set_step_size_exists = has_method<const S, void, set_step_size_t, state&, double, ConstrainedStep::Type, bool>;
static_assert(set_step_size_exists, "setStepSize method not found");
constexpr static bool get_step_size_exists = has_method<const S, double, get_step_size_t, const state &, ConstrainedStep::Type>;
static_assert(get_step_size_exists, "getStepSize method not found");
constexpr static bool release_step_size_exists = has_method<const S, void, release_step_size_t, state&>;
static_assert(release_step_size_exists, "releaseStepSize method not found");
constexpr static bool output_step_size_exists = has_method<const S, std::string, output_step_size_t, const state&>;
Expand Down
14 changes: 12 additions & 2 deletions Core/include/Acts/Propagator/StraightLineStepper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,20 @@ class StraightLineStepper {
/// @param state [in,out] The stepping state (thread-local cache)
/// @param stepSize [in] The step size value
/// @param stype [in] The step size type to be set
/// @param release [in] Do we release the step size?
void setStepSize(State& state, double stepSize,
ConstrainedStep::Type stype = ConstrainedStep::actor) const {
ConstrainedStep::Type stype = ConstrainedStep::actor,
bool release = true) const {
state.previousStepSize = state.stepSize;
state.stepSize.update(stepSize, stype, true);
state.stepSize.update(stepSize, stype, release);
}

/// Get the step size
///
/// @param state [in] The stepping state (thread-local cache)
/// @param stype [in] The step size type to be returned
double getStepSize(const State& state, ConstrainedStep::Type stype) const {
return state.stepSize.value(stype);
}

/// Release the Step size
Expand Down
17 changes: 16 additions & 1 deletion Tests/UnitTests/Core/Propagator/AbortListTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,22 @@ struct PropagatorState {
};

/// This is a struct to mimic the stepper
struct Stepper {};
struct Stepper {
auto outputStepSize(const PropagatorState::StepperState&) const {
return std::string{};
}

double getStepSize(const PropagatorState::StepperState& state,
ConstrainedStep::Type stype) const {
return state.stepSize.value(stype);
}

void setStepSize(PropagatorState::StepperState& state, double stepSize,
ConstrainedStep::Type stype = ConstrainedStep::actor,
bool release = true) const {
state.stepSize.update(stepSize, stype, release);
}
};

/// This is a simple result struct to mimic the
/// propagator result
Expand Down
12 changes: 8 additions & 4 deletions Tests/UnitTests/Core/Propagator/NavigatorTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,15 @@ struct PropagatorState {
detail::updateSingleStepSize<Stepper>(state, oIntersection, release);
}

void setStepSize(
State& state, double stepSize,
ConstrainedStep::Type stype = ConstrainedStep::actor) const {
void setStepSize(State& state, double stepSize,
ConstrainedStep::Type stype = ConstrainedStep::actor,
bool release = true) const {
state.previousStepSize = state.stepSize;
state.stepSize.update(stepSize, stype, true);
state.stepSize.update(stepSize, stype, release);
}

double getStepSize(const State& state, ConstrainedStep::Type stype) const {
return state.stepSize.value(stype);
}

void releaseStepSize(State& state) const {
Expand Down

0 comments on commit c62039a

Please sign in to comment.