Skip to content

Commit

Permalink
refactor: Refactor SurfaceReached aborter (#2603)
Browse files Browse the repository at this point in the history
With these changes I try to decouple the navigator and surface reached aborter further. I don't believe that the navigator has to care about the target surface other than optimization reasons while the surface reached aborter should only care about the surface it is supposed to care about without looking at or touching the navigation state.

blocked by
- #2621
  • Loading branch information
andiwand authored Nov 20, 2023
1 parent 27fc8ec commit eae867a
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 180 deletions.
Binary file modified CI/physmon/reference/performance_gsf.root
Binary file not shown.
9 changes: 5 additions & 4 deletions Core/include/Acts/Propagator/MaterialInteractor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "Acts/Geometry/TrackingVolume.hpp"
#include "Acts/Material/MaterialInteraction.hpp"
#include "Acts/Material/MaterialSlab.hpp"
#include "Acts/Propagator/Propagator.hpp"
#include "Acts/Propagator/detail/PointwiseMaterialInteraction.hpp"
#include "Acts/Propagator/detail/VolumeMaterialInteraction.hpp"
#include "Acts/Surfaces/Surface.hpp"
Expand Down Expand Up @@ -55,17 +56,17 @@ struct MaterialInteractor {
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;
}

// In case of Volume material update the result of the previous step
if (recordInteractions && !result.materialInteractions.empty() &&
!result.materialInteractions.back().volume.empty() &&
result.materialInteractions.back().updatedVolumeStep == false) {
updateResult(state, stepper, result);
}

// If we are on target, everything should have been done
if (navigator.targetReached(state.navigation)) {
return;
}
// Do nothing if nothing is what is requested.
if (!(multipleScattering || energyLoss || recordInteractions)) {
return;
Expand Down
88 changes: 39 additions & 49 deletions Core/include/Acts/Propagator/MultiStepperAborters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@

namespace Acts {

/// This
struct MultiStepperSurfaceReached {
MultiStepperSurfaceReached() = default;

struct MultiStepperSurfaceReached : public SurfaceReached {
/// If this is set, we are also happy if the mean of the components is on the
/// surface. How the averaging is performed depends on the stepper
/// implementation
Expand All @@ -27,6 +24,9 @@ struct MultiStepperSurfaceReached {
/// false
double averageOnSurfaceTolerance = 0.2;

MultiStepperSurfaceReached() = default;
MultiStepperSurfaceReached(double oLimit) : SurfaceReached(oLimit) {}

/// boolean operator for abort condition without using the result
///
/// @tparam propagator_state_t Type of the propagator state
Expand All @@ -41,72 +41,62 @@ struct MultiStepperSurfaceReached {
typename navigator_t>
bool operator()(propagator_state_t& state, const stepper_t& stepper,
const navigator_t& navigator, const Logger& logger) const {
return (*this)(state, stepper, navigator,
*navigator.targetSurface(state.navigation), logger);
}

/// boolean operator for abort condition without using the result
///
/// @tparam propagator_state_t Type of the propagator state
/// @tparam stepper_t Type of the stepper
/// @tparam navigator_t Type of the navigator
///
/// @param [in,out] state The propagation state object
/// @param [in] stepper Stepper used for the propagation
/// @param [in] navigator Navigator used for the propagation
/// @param [in] targetSurface The target surface
/// @param logger a logger instance
template <typename propagator_state_t, typename stepper_t,
typename navigator_t>
bool operator()(propagator_state_t& state, const stepper_t& stepper,
const navigator_t& navigator, const Surface& targetSurface,
const Logger& logger) const {
bool reached = true;
const auto oldCurrentSurface = navigator.currentSurface(state.navigation);

for (auto cmp : stepper.componentIterable(state.stepping)) {
auto singleState = cmp.singleState(state);
const auto& singleStepper = cmp.singleStepper(stepper);

if (!SurfaceReached{}(singleState, singleStepper, navigator,
targetSurface, logger)) {
reached = false;
} else {
cmp.status() = Acts::Intersection3D::Status::onSurface;
}
if (surface == nullptr) {
ACTS_VERBOSE(
"MultiStepperSurfaceReached aborter | "
"No target surface set.");
return false;
}

// However, if mean of all is on surface, we are happy as well
if (averageOnSurface) {
const auto sIntersection =
targetSurface
.intersect(
surface
->intersect(
state.geoContext, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
BoundaryCheck(true), averageOnSurfaceTolerance)
BoundaryCheck(boundaryCheck), averageOnSurfaceTolerance)
.closest();

if (sIntersection.status() == Intersection3D::Status::onSurface) {
ACTS_VERBOSE("Reached target in average mode");
navigator.currentSurface(state.navigation, &targetSurface);
navigator.targetReached(state.navigation, true);

ACTS_VERBOSE(
"MultiStepperSurfaceReached aborter | "
"Reached target in average mode");
for (auto cmp : stepper.componentIterable(state.stepping)) {
cmp.status() = Intersection3D::Status::onSurface;
}

return true;
}

ACTS_VERBOSE(
"MultiStepperSurfaceReached aborter | "
"Target intersection not found. Maybe next time?");
}

// These values are changed by the single component aborters but must be
// reset if not all components are on the target
if (!reached) {
navigator.currentSurface(state.navigation, oldCurrentSurface);
navigator.targetReached(state.navigation, false);
bool reached = true;

for (auto cmp : stepper.componentIterable(state.stepping)) {
// note that this is not copying anything heavy
auto singleState = cmp.singleState(state);
const auto& singleStepper = cmp.singleStepper(stepper);

if (!SurfaceReached::operator()(singleState, singleStepper, navigator,
logger)) {
reached = false;
} else {
cmp.status() = Acts::Intersection3D::Status::onSurface;
}
}

if (reached) {
ACTS_VERBOSE(
"MultiStepperSurfaceReached aborter | "
"Reached target in single component mode");
}

return reached;
}
};

} // namespace Acts
1 change: 1 addition & 0 deletions Core/include/Acts/Propagator/Propagator.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ auto Acts::Propagator<S, N>::propagate(

// Type of provided options
target_aborter_t targetAborter;
targetAborter.surface = &target;
path_aborter_t pathAborter;
pathAborter.internalLimit = options.pathLimit;
auto abortList = options.abortList.append(targetAborter, pathAborter);
Expand Down
108 changes: 27 additions & 81 deletions Core/include/Acts/Propagator/StandardAborters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,6 @@

namespace Acts {

/// @brief TargetOptions struct for geometry interface
struct TargetOptions {
/// Navigation direction
Direction navDir = Direction::Forward;

/// Target Boundary check directive - always false here
BoundaryCheck boundaryCheck = BoundaryCheck(false);

/// Object to check against - always nullptr here
const Surface* startObject = nullptr;

/// The path limit
double pathLimit = std::numeric_limits<double>::max();

/// create target options
TargetOptions(Direction ndir) : navDir(ndir) {}
};

/// This is the condition that the pathLimit has been reached
struct PathLimitReached {
/// Boolean switch for Loop protection
Expand Down Expand Up @@ -70,15 +52,15 @@ struct PathLimitReached {
double tolerance = state.options.surfaceTolerance;
bool limitReached = (std::abs(distance) < std::abs(tolerance));
if (limitReached) {
ACTS_VERBOSE("Target: x | "
ACTS_VERBOSE("PathLimit aborter | "
<< "Path limit reached at distance " << distance);
// reaching the target means navigation break
navigator.targetReached(state.navigation, true);
return true;
}
stepper.setStepSize(state.stepping, distance, ConstrainedStep::aborter,
false);
ACTS_VERBOSE("Target: 0 | "
ACTS_VERBOSE("PathLimit aborter | "
<< "Target stepSize (path limit) updated to "
<< stepper.outputStepSize(state.stepping));
return false;
Expand All @@ -88,6 +70,8 @@ struct PathLimitReached {
/// This is the condition that the Surface has been reached
/// it then triggers an propagation abort of the propagation
struct SurfaceReached {
const Surface* surface = nullptr;
BoundaryCheck boundaryCheck = BoundaryCheck(true);
std::optional<double> overstepLimit;

SurfaceReached() = default;
Expand All @@ -107,92 +91,54 @@ struct SurfaceReached {
typename navigator_t>
bool operator()(propagator_state_t& state, const stepper_t& stepper,
const navigator_t& navigator, const Logger& logger) const {
return (*this)(state, stepper, navigator,
*navigator.targetSurface(state.navigation), logger);
}

/// boolean operator for abort condition without using the result
///
/// @tparam propagator_state_t Type of the propagator state
/// @tparam stepper_t Type of the stepper
/// @tparam navigator_t Type of the navigator
///
/// @param [in,out] state The propagation state object
/// @param [in] stepper Stepper used for the propagation
/// @param [in] navigator Navigator used for the propagation
/// @param [in] targetSurface The target surface
/// @param logger a logger instance
template <typename propagator_state_t, typename stepper_t,
typename navigator_t>
bool operator()(propagator_state_t& state, const stepper_t& stepper,
const navigator_t& navigator, const Surface& targetSurface,
const Logger& logger) const {
if (navigator.targetReached(state.navigation)) {
return true;
if (surface == nullptr) {
ACTS_VERBOSE("SurfaceReached aborter | Target surface not set.");
return false;
}

// Check if the cache filled the currentSurface - or if we are on the
// surface
if ((navigator.currentSurface(state.navigation) &&
navigator.currentSurface(state.navigation) == &targetSurface)) {
ACTS_VERBOSE("Target: x | "
<< "Target surface reached.");
// reaching the target calls a navigation break
navigator.targetReached(state.navigation, true);
if (navigator.currentSurface(state.navigation) == surface) {
ACTS_VERBOSE("SurfaceReached aborter | Target surface reached.");
return true;
}

// TODO the following code is mostly duplicated in updateSingleSurfaceStatus

// Calculate the distance to the surface
const double pLimit =
state.stepping.stepSize.value(ConstrainedStep::aborter);
const double oLimit =
overstepLimit.value_or(stepper.overstepLimit(state.stepping));
const double tolerance = state.options.surfaceTolerance;

const auto sIntersection = targetSurface.intersect(
const auto sIntersection = surface->intersect(
state.geoContext, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
BoundaryCheck(true), tolerance);
boundaryCheck, tolerance);
const auto closest = sIntersection.closest();

// Return true if you fall below tolerance
if (closest.status() == Intersection3D::Status::onSurface) {
const double distance = closest.pathLength();
ACTS_VERBOSE("Target: x | "
<< "Target surface reached at distance (tolerance) "
<< distance << " (" << tolerance << ")");

// assigning the currentSurface
navigator.currentSurface(state.navigation, &targetSurface);
ACTS_VERBOSE("Target: x | "
<< "Current surface set to target surface "
<< navigator.currentSurface(state.navigation)->geometryId());

// reaching the target calls a navigation break
navigator.targetReached(state.navigation, true);

ACTS_VERBOSE(
"SurfaceReached aborter | "
"Target surface reached at distance (tolerance) "
<< distance << " (" << tolerance << ")");
return true;
}

const double pLimit =
state.stepping.stepSize.value(ConstrainedStep::aborter);
// not using the stepper overstep limit here because it does not always work
// for perigee surfaces
const double oLimit =
overstepLimit.value_or(stepper.overstepLimit(state.stepping));

for (const auto& intersection : sIntersection.split()) {
if (intersection &&
detail::checkIntersection(intersection.intersection(), pLimit, oLimit,
tolerance, logger)) {
stepper.setStepSize(state.stepping, intersection.pathLength(),
ConstrainedStep::aborter, false);
break;
ACTS_VERBOSE(
"SurfaceReached aborter | "
"Target stepSize (surface) updated to "
<< stepper.outputStepSize(state.stepping));
return false;
}
}

ACTS_VERBOSE("Target: 0 | "
<< "Target stepSize (surface) updated to "
<< stepper.outputStepSize(state.stepping));

ACTS_VERBOSE(
"SurfaceReached aborter | "
"Target intersection not found. Maybe next time?");
return false;
}
};
Expand Down
Loading

0 comments on commit eae867a

Please sign in to comment.