Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Release step constraint after stepper in propagation #2596

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified CI/physmon/reference/performance_gsf.root
Binary file not shown.
12 changes: 6 additions & 6 deletions Core/include/Acts/Propagator/AtlasStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,15 +422,14 @@ class AtlasStepper {
detail::updateSingleStepSize<AtlasStepper>(state, oIntersection, release);
}

/// Set Step size - explicitly with a double
/// Update step size - explicitly with a double
///
/// @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,
bool release = true) const {
void updateStepSize(State& state, double stepSize,
ConstrainedStep::Type stype, bool release = true) const {
state.previousStepSize = state.stepSize.value();
state.stepSize.update(stepSize, stype, release);
}
Expand All @@ -446,8 +445,9 @@ class AtlasStepper {
/// Release the Step size
///
/// @param [in,out] state The stepping state (thread-local cache)
void releaseStepSize(State& state) const {
state.stepSize.release(ConstrainedStep::actor);
/// @param [in] stype The step size type to be released
void releaseStepSize(State& state, ConstrainedStep::Type stype) const {
state.stepSize.release(stype);
}

/// Output the Step Size - single component
Expand Down
1 change: 0 additions & 1 deletion Core/include/Acts/Propagator/DirectNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,6 @@ class DirectNavigator {
state.navigation.navSurfaces.end()) {
ACTS_VERBOSE("Next surface candidate is "
<< (*state.navigation.navSurfaceIter)->geometryId());
stepper.releaseStepSize(state.stepping);
}
} else if (surfaceStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE("Next surface reachable at distance "
Expand Down
12 changes: 6 additions & 6 deletions Core/include/Acts/Propagator/EigenStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,14 @@ class EigenStepper {
detail::updateSingleStepSize<EigenStepper>(state, oIntersection, release);
}

/// Set Step size - explicitly with a double
/// Update step size - explicitly with a double
///
/// @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,
bool release = true) const {
void updateStepSize(State& state, double stepSize,
ConstrainedStep::Type stype, bool release = true) const {
state.previousStepSize = state.stepSize.value();
state.stepSize.update(stepSize, stype, release);
}
Expand All @@ -296,8 +295,9 @@ class EigenStepper {
/// Release the Step size
///
/// @param state [in,out] The stepping state (thread-local cache)
void releaseStepSize(State& state) const {
state.stepSize.release(ConstrainedStep::actor);
/// @param [in] stype The step size type to be released
void releaseStepSize(State& state, ConstrainedStep::Type stype) const {
state.stepSize.release(stype);
}

/// Output the Step Size - single component
Expand Down
14 changes: 7 additions & 7 deletions Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -675,17 +675,16 @@ class MultiEigenStepperLoop
}
}

/// Set Step size - explicitly with a double
/// Update step size - explicitly with a double
///
/// @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,
bool release = true) const {
void updateStepSize(State& state, double stepSize,
ConstrainedStep::Type stype, bool release = true) const {
for (auto& component : state.components) {
SingleStepper::setStepSize(component.state, stepSize, stype, release);
SingleStepper::updateStepSize(component.state, stepSize, stype, release);
}
}

Expand All @@ -708,9 +707,10 @@ class MultiEigenStepperLoop
/// Release the step-size for all components
///
/// @param state [in,out] The stepping state (thread-local cache)
void releaseStepSize(State& state) const {
/// @param [in] stype The step size type to be released
void releaseStepSize(State& state, ConstrainedStep::Type stype) const {
for (auto& component : state.components) {
SingleStepper::releaseStepSize(component.state);
SingleStepper::releaseStepSize(component.state, stype);
}
}

Expand Down
12 changes: 5 additions & 7 deletions Core/include/Acts/Propagator/Navigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ class Navigator {
<< "No further navigation action, proceed to target.");
// Set navigation break and release the navigation step size
state.navigation.navigationBreak = true;
stepper.releaseStepSize(state.stepping);
stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
}

// Navigator target always resets the current surface
Expand Down Expand Up @@ -536,7 +536,6 @@ class Navigator {
<< "No more volume to progress to, stopping navigation.");
// Navigation break & release navigation stepping
state.navigation.navigationBreak = true;
stepper.releaseStepSize(state.stepping);
return;
} else {
ACTS_VERBOSE(volInfo(state) << "Volume updated.");
Expand All @@ -562,7 +561,6 @@ class Navigator {
}
// Set navigation break and release the navigation step size
state.navigation.navigationBreak = true;
stepper.releaseStepSize(state.stepping);
} else {
ACTS_VERBOSE(volInfo(state)
<< "Status could not be determined - good luck.");
Expand Down Expand Up @@ -649,7 +647,7 @@ class Navigator {
<< "Start is target layer, nothing left to do.");
// set the navigation break
state.navigation.navigationBreak = true;
stepper.releaseStepSize(state.stepping);
stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
}
return startResolved;
}
Expand Down Expand Up @@ -934,15 +932,15 @@ class Navigator {
ACTS_VERBOSE(volInfo(state)
<< "No sufficient information to resolve boundary, "
"stopping navigation.");
stepper.releaseStepSize(state.stepping);
stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
return false;
} else if (state.navigation.currentVolume ==
state.navigation.targetVolume) {
ACTS_VERBOSE(volInfo(state)
<< "In target volume: no need to resolve boundary, "
"stopping navigation.");
state.navigation.navigationBreak = true;
stepper.releaseStepSize(state.stepping);
stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
return true;
}

Expand Down Expand Up @@ -1275,7 +1273,7 @@ class Navigator {
// Screen output - no layer candidates found
ACTS_VERBOSE(volInfo(state) << "No compatible layer candidates found.");
// Release the step size
stepper.releaseStepSize(state.stepping);
stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
return false;
}

Expand Down
4 changes: 4 additions & 0 deletions Core/include/Acts/Propagator/Propagator.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

#include "Acts/EventData/TrackParametersConcept.hpp"
#include "Acts/Propagator/ConstrainedStep.hpp"
#include "Acts/Propagator/PropagatorError.hpp"
#include "Acts/Propagator/detail/LoopProtection.hpp"

Expand Down Expand Up @@ -58,6 +59,9 @@ auto Acts::Propagator<S, N>::propagate_impl(propagator_state_t& state,
// pass error to caller
return res.error();
}
// release actor and aborter constrains after step was performed
m_stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
m_stepper.releaseStepSize(state.stepping, ConstrainedStep::aborter);
// Post-stepping:
// navigator post step call - action list - aborter list
state.stage = PropagatorStage::postStep;
Expand Down
8 changes: 4 additions & 4 deletions Core/include/Acts/Propagator/StandardAborters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ struct PathLimitReached {
navigator.targetReached(state.navigation, true);
return true;
}
stepper.setStepSize(state.stepping, distance, ConstrainedStep::aborter,
false);
stepper.updateStepSize(state.stepping, distance, ConstrainedStep::aborter,
false);
ACTS_VERBOSE("PathLimit aborter | "
<< "Target stepSize (path limit) updated to "
<< stepper.outputStepSize(state.stepping));
Expand Down Expand Up @@ -126,8 +126,8 @@ struct SurfaceReached {
if (intersection &&
detail::checkIntersection(intersection.intersection(), pLimit, oLimit,
tolerance, logger)) {
stepper.setStepSize(state.stepping, intersection.pathLength(),
ConstrainedStep::aborter, false);
stepper.updateStepSize(state.stepping, intersection.pathLength(),
ConstrainedStep::aborter, false);
ACTS_VERBOSE(
"SurfaceReached aborter | "
"Target stepSize (surface) updated to "
Expand Down
10 changes: 5 additions & 5 deletions Core/include/Acts/Propagator/StepperConcept.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ METHOD_TRAIT(covariance_transport_curvilinear_t,
transportCovarianceToCurvilinear);
METHOD_TRAIT(step_t, step);
METHOD_TRAIT(update_surface_status_t, updateSurfaceStatus);
METHOD_TRAIT(set_step_size_t, setStepSize);
METHOD_TRAIT(update_step_size_t, updateStepSize);
METHOD_TRAIT(get_step_size_t, getStepSize);
METHOD_TRAIT(release_step_size_t, releaseStepSize);
METHOD_TRAIT(output_step_size_t, outputStepSize);
Expand Down Expand Up @@ -126,11 +126,11 @@ constexpr bool MultiStepperStateConcept= require<
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&, std::uint8_t, Direction, const BoundaryCheck&, ActsScalar, const Logger&>;
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, bool>;
static_assert(set_step_size_exists, "setStepSize method not found");
constexpr static bool update_step_size_exists = has_method<const S, void, update_step_size_t, state&, double, ConstrainedStep::Type, bool>;
static_assert(update_step_size_exists, "updateStepSize 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&>;
constexpr static bool release_step_size_exists = has_method<const S, void, release_step_size_t, state&, ConstrainedStep::Type>;
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&>;
static_assert(output_step_size_exists, "outputStepSize method not found");
Expand All @@ -151,7 +151,7 @@ constexpr bool MultiStepperStateConcept= require<
curvilinear_state_method_exists,
covariance_transport_exists,
update_surface_exists,
set_step_size_exists,
update_step_size_exists,
release_step_size_exists,
output_step_size_exists>;

Expand Down
15 changes: 8 additions & 7 deletions Core/include/Acts/Propagator/StraightLineStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,15 +263,15 @@ class StraightLineStepper {
release);
}

/// Set Step size - explicitly with a double
/// Update step size - explicitly with a double
///
/// @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,
bool release = true) const {
void updateStepSize(State& state, double stepSize,
ConstrainedStep::Type stype = ConstrainedStep::actor,
bool release = true) const {
state.previousStepSize = state.stepSize.value();
state.stepSize.update(stepSize, stype, release);
}
Expand All @@ -286,9 +286,10 @@ class StraightLineStepper {

/// Release the Step size
///
/// @param state [in,out] The stepping state (thread-local cache)
void releaseStepSize(State& state) const {
state.stepSize.release(ConstrainedStep::actor);
/// @param [in,out] state The stepping state (thread-local cache)
/// @param [in] stype The step size type to be released
void releaseStepSize(State& state, ConstrainedStep::Type stype) const {
state.stepSize.release(stype);
}

/// Output the Step Size - single component
Expand Down
3 changes: 2 additions & 1 deletion Core/include/Acts/Propagator/detail/SteppingHelper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ Acts::Intersection3D::Status updateSingleSurfaceStatus(
detail::checkIntersection(sIntersection.intersection(), pLimit, oLimit,
surfaceTolerance, logger)) {
ACTS_VERBOSE("Surface is reachable");
stepper.setStepSize(state, sIntersection.pathLength());
stepper.updateStepSize(state, sIntersection.pathLength(),
ConstrainedStep::actor);
return Intersection3D::Status::reachable;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ class CombinatorialKalmanFilter {
if (!result.filtered &&
filterTargetReached(state, stepper, navigator, logger())) {
navigator.navigationBreak(state.navigation, true);
stepper.releaseStepSize(state.stepping);
stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
}

// Update:
Expand Down
8 changes: 4 additions & 4 deletions Examples/Python/tests/root_file_hashes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,21 @@ test_digitization_example_input[smeared]__measurements.root: 2e4fd9d3e6244e53486
test_digitization_example_input[geometric]__particles.root: 8549ba6e20338004ab8ba299fc65e1ee5071985b46df8f77f887cb6fef56a8ec
test_digitization_example_input[geometric]__measurements.root: 20128357c9238926d628d18b3b91a0a73947b2b5682762bf5d8fd94342faf976
test_ckf_tracks_example[generic-full_seeding]__trackstates_ckf.root: 5ddcc228073553ba14205897451951ea475541643bb677e7718bd3369b484bcd
test_ckf_tracks_example[generic-full_seeding]__tracksummary_ckf.root: 6ccdd72caf4d645debea57fc823d5ceedb74c70b9832b3a906ae4a06ac8fe796
test_ckf_tracks_example[generic-full_seeding]__tracksummary_ckf.root: afa6eaffe366533d343c78685015a10f71f8f49b318431878a1bb49098bcbfc4
test_ckf_tracks_example[generic-full_seeding]__performance_seeding_trees.root: 0e0676ffafdb27112fbda50d1cf627859fa745760f98073261dcf6db3f2f991e
test_ckf_tracks_example[generic-truth_estimated]__trackstates_ckf.root: 10ecbc093239670b0a116fad5a25caf941f2a401a03892a6853652d20ee98a67
test_ckf_tracks_example[generic-truth_estimated]__tracksummary_ckf.root: 068c236fd9f52801eb39e72498c7ec37857523a617d77b5987db4bfb736c6f88
test_ckf_tracks_example[generic-truth_estimated]__performance_seeding.root: 1facb05c066221f6361b61f015cdf0918e94d9f3fce2269ec7b6a4dffeb2bc7e
test_ckf_tracks_example[generic-truth_smeared]__trackstates_ckf.root: 4c081f8d984b7a9eb680ddc870d03ada2f3bbbb6fb0e41877720e449b82b87d8
test_ckf_tracks_example[generic-truth_smeared]__tracksummary_ckf.root: 3ac06b38c59d673099dd4474320423932c84eea47b226fad887ed1c2c83d18c1
test_ckf_tracks_example[odd-full_seeding]__trackstates_ckf.root: f8511a029465842e39894bbb6054bc330747e0948e686787aa9f90fea2e59102
test_ckf_tracks_example[odd-full_seeding]__tracksummary_ckf.root: 24eea0bc0c0c2a790fbf8038e0bd84c62325bccc9c9bc580f66c27e7a35654c3
test_ckf_tracks_example[odd-full_seeding]__tracksummary_ckf.root: fd2e99f7fef639aeb55991a55d31310f3a2e173c04d81010e4f60aba850be68d
test_ckf_tracks_example[odd-full_seeding]__performance_seeding_trees.root: 43c58577aafe07645e5660c4f43904efadf91d8cda45c5c04c248bbe0f59814f
test_ckf_tracks_example[odd-truth_estimated]__trackstates_ckf.root: f244ea1201fedaec0619d595945d33d976fc1e09e8d218e16ea0737f35debe0f
test_ckf_tracks_example[odd-truth_estimated]__tracksummary_ckf.root: f7b683c6c0c8a57dd61bd3ba2965bf5f689a6eb9c2e8c8e8e13a7907ea7411a3
test_ckf_tracks_example[odd-truth_estimated]__tracksummary_ckf.root: 91ce6097ba491dd83529e60a08fb97459f2e221af553545c600db209358d299a
test_ckf_tracks_example[odd-truth_estimated]__performance_seeding.root: 1a36b7017e59f1c08602ef3c2cb0483c51df248f112e3780c66594110719c575
test_ckf_tracks_example[odd-truth_smeared]__trackstates_ckf.root: cee2ef40dbc341861a9abac8e4d2503240c5522d9fef6edf30fa163129f17cef
test_ckf_tracks_example[odd-truth_smeared]__tracksummary_ckf.root: 6a8e6ef7c5a2b17b58d3c551e951a4cfb894c795e5c91df8b4499b5688020ee1
test_ckf_tracks_example[odd-truth_smeared]__tracksummary_ckf.root: 1ec3257d7eccedd73bde17f3554e2bce1f4402a169b6003b3908d25833decd40
test_vertex_fitting_reading[Truth-False-100]__performance_vertexing.root: 76ef6084d758dfdfc0151ddec2170e12d73394424e3dac4ffe46f0f339ec8293
test_vertex_fitting_reading[Iterative-False-100]__performance_vertexing.root: 60372210c830a04f95ceb78c6c68a9b0de217746ff59e8e73053750c837b57eb
test_vertex_fitting_reading[Iterative-True-100]__performance_vertexing.root: e34f217d524a5051dbb04a811d3407df3ebe2cc4bb7f54f6bda0847dbd7b52c3
Expand Down
4 changes: 2 additions & 2 deletions Fatras/include/ActsFatras/Kernel/detail/SimulationActor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ struct SimulationActor {
const auto stepSize = properTimeDiff *
result.particle.absoluteMomentum() /
result.particle.mass();
stepper.setStepSize(state.stepping, stepSize,
Acts::ConstrainedStep::user);
stepper.updateStepSize(state.stepping, stepSize,
Acts::ConstrainedStep::user);
}

// arm the point-like interaction limits in the first step
Expand Down
4 changes: 2 additions & 2 deletions Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -561,11 +561,11 @@ BOOST_AUTO_TEST_CASE(StepSize) {
// TODO figure out why this fails and what it should be
// BOOST_CHECK_EQUAL(stepper.overstepLimit(state), tolerance);

stepper.setStepSize(state, -5_cm);
stepper.updateStepSize(state, -5_cm, ConstrainedStep::actor);
BOOST_CHECK_EQUAL(state.previousStepSize, stepSize);
BOOST_CHECK_EQUAL(state.stepSize.value(), -5_cm);

stepper.releaseStepSize(state);
stepper.releaseStepSize(state, ConstrainedStep::actor);
BOOST_CHECK_EQUAL(state.stepSize.value(), stepSize);
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,11 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) {
// Step size modifies
const std::string originalStepSize = esState.stepSize.toString();

es.setStepSize(esState, -1337.);
es.updateStepSize(esState, -1337., ConstrainedStep::actor);
BOOST_CHECK_EQUAL(esState.previousStepSize, stepSize);
BOOST_CHECK_EQUAL(esState.stepSize.value(), -1337.);

es.releaseStepSize(esState);
es.releaseStepSize(esState, ConstrainedStep::actor);
BOOST_CHECK_EQUAL(esState.stepSize.value(), stepSize);
BOOST_CHECK_EQUAL(es.outputStepSize(esState), originalStepSize);

Expand Down
10 changes: 5 additions & 5 deletions Tests/UnitTests/Core/Propagator/NavigatorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ struct PropagatorState {
detail::updateSingleStepSize<Stepper>(state, oIntersection, release);
}

void setStepSize(State& state, double stepSize,
ConstrainedStep::Type stype = ConstrainedStep::actor,
bool release = true) const {
void updateStepSize(State& state, double stepSize,
ConstrainedStep::Type stype,
bool release = true) const {
state.previousStepSize = state.stepSize.value();
state.stepSize.update(stepSize, stype, release);
}
Expand All @@ -170,8 +170,8 @@ struct PropagatorState {
return state.stepSize.value(stype);
}

void releaseStepSize(State& state) const {
state.stepSize.release(ConstrainedStep::actor);
void releaseStepSize(State& state, ConstrainedStep::Type stype) const {
state.stepSize.release(stype);
}

std::string outputStepSize(const State& state) const {
Expand Down
Loading
Loading