Skip to content

Commit

Permalink
refactor: Intersection index for stepper surface update (#2599)
Browse files Browse the repository at this point in the history
After refactoring the surface intersection #2336 I missed to update this function of the stepper

discovered in #2595 and #2596
  • Loading branch information
andiwand authored Nov 2, 2023
1 parent c3aa32f commit 3fd6166
Show file tree
Hide file tree
Showing 14 changed files with 123 additions and 68 deletions.
11 changes: 7 additions & 4 deletions Core/include/Acts/Navigation/DetectorNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,9 @@ class DetectorNavigator {
// Estimate the surface status
bool boundaryCheck = c.boundaryCheck;
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, surface, state.options.direction, boundaryCheck,
state.options.targetTolerance, logger());
state.stepping, surface, c.objectIntersection.index(),
state.options.direction, boundaryCheck, state.options.targetTolerance,
logger());
if (surfaceStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE(volInfo(state)
<< posInfo(state, stepper)
Expand Down Expand Up @@ -296,8 +297,10 @@ class DetectorNavigator {

// TODO not sure about the boundary check
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, *nextSurface, state.options.direction, boundaryCheck,
state.options.targetTolerance, logger());
state.stepping, *nextSurface,
nState.surfaceCandidate->objectIntersection.index(),
state.options.direction, boundaryCheck, state.options.targetTolerance,
logger());

// Check if we are at a surface
if (surfaceStatus == Intersection3D::Status::onSurface) {
Expand Down
7 changes: 4 additions & 3 deletions Core/include/Acts/Propagator/AtlasStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,17 +394,18 @@ class AtlasStepper {
///
/// @param [in,out] state The stepping state (thread-local cache)
/// @param [in] surface The surface provided
/// @param [in] index The surface intersection index
/// @param [in] navDir The navigation direction
/// @param [in] bcheck The boundary check for this status update
/// @param [in] surfaceTolerance Surface tolerance used for intersection
/// @param [in] logger Logger instance to use
Intersection3D::Status updateSurfaceStatus(
State& state, const Surface& surface, Direction navDir,
const BoundaryCheck& bcheck,
State& state, const Surface& surface, std::uint8_t index,
Direction navDir, const BoundaryCheck& bcheck,
ActsScalar surfaceTolerance = s_onSurfaceTolerance,
const Logger& logger = getDummyLogger()) const {
return detail::updateSingleSurfaceStatus<AtlasStepper>(
*this, state, surface, navDir, bcheck, surfaceTolerance, logger);
*this, state, surface, index, navDir, bcheck, surfaceTolerance, logger);
}

/// Update step size
Expand Down
52 changes: 46 additions & 6 deletions Core/include/Acts/Propagator/DirectNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
#include "Acts/Geometry/TrackingVolume.hpp"
#include "Acts/Propagator/Propagator.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Utilities/Intersection.hpp"

#include <algorithm>
#include <iterator>
#include <limits>
#include <memory>
#include <vector>

Expand Down Expand Up @@ -247,10 +249,19 @@ class DirectNavigator {

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 auto index =
chooseIntersection(
state.geoContext, surface, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
false, std::numeric_limits<double>::max(),
stepper.overstepLimit(state.stepping),
state.options.targetTolerance)
.index();
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, **state.navigation.navSurfaceIter,
state.options.direction, false, state.options.targetTolerance,
*m_logger);
state.stepping, surface, index, state.options.direction, false,
state.options.targetTolerance, *m_logger);
if (surfaceStatus == Intersection3D::Status::unreachable) {
ACTS_VERBOSE(
"Surface not reachable anymore, switching to next one in "
Expand Down Expand Up @@ -296,10 +307,19 @@ class DirectNavigator {
// 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 auto index =
chooseIntersection(
state.geoContext, surface, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
false, std::numeric_limits<double>::max(),
stepper.overstepLimit(state.stepping),
state.options.targetTolerance)
.index();
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, **state.navigation.navSurfaceIter,
state.options.direction, false, state.options.targetTolerance,
*m_logger);
state.stepping, surface, index, state.options.direction, false,
state.options.targetTolerance, *m_logger);
if (surfaceStatus == Intersection3D::Status::onSurface) {
// Set the current surface
state.navigation.currentSurface = *state.navigation.navSurfaceIter;
Expand Down Expand Up @@ -329,6 +349,26 @@ class DirectNavigator {
" | ";
}

ObjectIntersection<Surface> chooseIntersection(const GeometryContext& gctx,
const Surface& surface,
const Vector3& position,
const Vector3& direction,
const BoundaryCheck& bcheck,
double pLimit, double oLimit,
double tolerance) const {
auto intersections =
surface.intersect(gctx, position, direction, bcheck, tolerance);

for (auto& intersection : intersections.split()) {
if (detail::checkIntersection(intersection, pLimit, oLimit, tolerance,
logger())) {
return intersection;
}
}

return ObjectIntersection<Surface>::invalid();
}

const Logger& logger() const { return *m_logger; }

std::unique_ptr<const Logger> m_logger;
Expand Down
7 changes: 4 additions & 3 deletions Core/include/Acts/Propagator/EigenStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,17 +242,18 @@ class EigenStepper {
///
/// @param [in,out] state The stepping state (thread-local cache)
/// @param [in] surface The surface provided
/// @param [in] index The surface intersection index
/// @param [in] navDir The navigation direction
/// @param [in] bcheck The boundary check for this status update
/// @param [in] surfaceTolerance Surface tolerance used for intersection
/// @param [in] logger A @c Logger instance
Intersection3D::Status updateSurfaceStatus(
State& state, const Surface& surface, Direction navDir,
const BoundaryCheck& bcheck,
State& state, const Surface& surface, std::uint8_t index,
Direction navDir, const BoundaryCheck& bcheck,
ActsScalar surfaceTolerance = s_onSurfaceTolerance,
const Logger& logger = getDummyLogger()) const {
return detail::updateSingleSurfaceStatus<EigenStepper>(
*this, state, surface, navDir, bcheck, surfaceTolerance, logger);
*this, state, surface, index, navDir, bcheck, surfaceTolerance, logger);
}

/// Update step size
Expand Down
11 changes: 6 additions & 5 deletions Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -690,23 +690,24 @@ class MultiEigenStepperLoop
///
/// @param [in,out] state The stepping state (thread-local cache)
/// @param [in] surface The surface provided
/// @param [in] index The surface intersection index
/// @param [in] navDir The navigation direction
/// @param [in] bcheck The boundary check for this status update
/// @param [in] surfaceTolerance Surface tolerance used for intersection
/// @param [in] logger A @c Logger instance
Intersection3D::Status updateSurfaceStatus(
State& state, const Surface& surface, Direction navDir,
const BoundaryCheck& bcheck,
State& state, const Surface& surface, std::uint8_t index,
Direction navDir, const BoundaryCheck& bcheck,
ActsScalar surfaceTolerance = s_onSurfaceTolerance,
const Logger& logger = getDummyLogger()) const {
using Status = Intersection3D::Status;

std::array<int, 4> counts = {0, 0, 0, 0};
std::array<int, 3> counts = {0, 0, 0};

for (auto& component : state.components) {
component.status = detail::updateSingleSurfaceStatus<SingleStepper>(
*this, component.state, surface, navDir, bcheck, surfaceTolerance,
logger);
*this, component.state, surface, index, navDir, bcheck,
surfaceTolerance, logger);
++counts[static_cast<std::size_t>(component.status)];
}

Expand Down
34 changes: 21 additions & 13 deletions Core/include/Acts/Propagator/Navigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -594,14 +594,15 @@ class Navigator {
if (navSurfaces.empty() || navIndex == navSurfaces.size()) {
return false;
}
const auto& intersection = navSurfaces.at(navIndex);
// Take the current surface
auto surface = navSurfaces.at(navIndex).representation();
auto surface = intersection.representation();
// Check if we are at a surface
// If we are on the surface pointed at by the index, we can make
// it the current one to pass it to the other actors
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, *surface, state.options.direction, true,
state.options.targetTolerance, logger());
state.stepping, *surface, intersection.index(), state.options.direction,
true, state.options.targetTolerance, logger());
if (surfaceStatus == Intersection3D::Status::onSurface) {
ACTS_VERBOSE(volInfo(state)
<< "Status Surface successfully hit, storing it.");
Expand Down Expand Up @@ -675,8 +676,9 @@ class Navigator {
state.navigation.navSurfaceIndex)
<< " out of " << state.navigation.navSurfaces.size()
<< " surfaces remain to try.");
const auto& intersection = state.navigation.navSurface();
// Take the surface
auto surface = state.navigation.navSurface().object();
const auto* surface = intersection.object();
// Screen output which surface you are on
ACTS_VERBOSE(volInfo(state) << "Next surface candidate will be "
<< surface->geometryId());
Expand All @@ -690,8 +692,9 @@ class Navigator {
}
}
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, *surface, state.options.direction, boundaryCheck,
state.options.targetTolerance, logger());
state.stepping, *surface, intersection.index(),
state.options.direction, boundaryCheck, state.options.targetTolerance,
logger());
if (surfaceStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE(volInfo(state)
<< "Surface reachable, step size updated to "
Expand Down Expand Up @@ -843,8 +846,9 @@ class Navigator {
// loop over the available navigation layer candidates
while (state.navigation.navLayerIndex !=
state.navigation.navLayers.size()) {
const auto& intersection = state.navigation.navLayer();
// The layer surface
auto layerSurface = state.navigation.navLayer().representation();
const auto* layerSurface = intersection.representation();
// We are on the layer
if (state.navigation.currentSurface == layerSurface) {
ACTS_VERBOSE(volInfo(state) << "We are on a layer, resolve Surfaces.");
Expand All @@ -859,8 +863,9 @@ class Navigator {
}
// Try to step towards it
auto layerStatus = stepper.updateSurfaceStatus(
state.stepping, *layerSurface, state.options.direction, true,
state.options.targetTolerance, logger());
state.stepping, *layerSurface, intersection.index(),
state.options.direction, true, state.options.targetTolerance,
logger());
if (layerStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE(volInfo(state) << "Layer reachable, step size updated to "
<< stepper.outputStepSize(state.stepping));
Expand Down Expand Up @@ -998,12 +1003,14 @@ class Navigator {
// Loop over the boundary surface
while (state.navigation.navBoundaryIndex !=
state.navigation.navBoundaries.size()) {
const auto& intersection = state.navigation.navBoundary();
// That is the current boundary surface
auto boundarySurface = state.navigation.navBoundary().representation();
const auto* boundarySurface = intersection.representation();
// Step towards the boundary surfrace
auto boundaryStatus = stepper.updateSurfaceStatus(
state.stepping, *boundarySurface, state.options.direction, true,
state.options.targetTolerance, logger());
state.stepping, *boundarySurface, intersection.index(),
state.options.direction, true, state.options.targetTolerance,
logger());
if (boundaryStatus == Intersection3D::Status::reachable) {
ACTS_VERBOSE(volInfo(state)
<< "Boundary reachable, step size updated to "
Expand Down Expand Up @@ -1306,8 +1313,9 @@ class Navigator {
if (state.navigation.targetReached || !state.navigation.targetSurface) {
return true;
}
// TODO we do not know the intersection index - passing 0
auto targetStatus = stepper.updateSurfaceStatus(
state.stepping, *state.navigation.targetSurface,
state.stepping, *state.navigation.targetSurface, 0,
state.options.direction, true, state.options.targetTolerance,
logger());
// the only advance could have been to the target
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Propagator/StepperConcept.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ constexpr bool MultiStepperStateConcept= require<
constexpr static bool covariance_transport_exists = require<has_method<const S, void, covariance_transport_curvilinear_t, state&>,
has_method<const S, void, covariance_transport_bound_t, state&, const Surface&, const FreeToBoundCorrection&>>;
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&, Direction, const BoundaryCheck&, ActsScalar, const Logger&>;
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");
Expand Down
7 changes: 4 additions & 3 deletions Core/include/Acts/Propagator/StraightLineStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,17 +234,18 @@ class StraightLineStepper {
///
/// @param [in,out] state The stepping state (thread-local cache)
/// @param [in] surface The surface provided
/// @param [in] index The surface intersection index
/// @param [in] navDir The navigation direction
/// @param [in] bcheck The boundary check for this status update
/// @param [in] surfaceTolerance Surface tolerance used for intersection
/// @param [in] logger A logger instance
Intersection3D::Status updateSurfaceStatus(
State& state, const Surface& surface, Direction navDir,
const BoundaryCheck& bcheck,
State& state, const Surface& surface, std::uint8_t index,
Direction navDir, const BoundaryCheck& bcheck,
ActsScalar surfaceTolerance = s_onSurfaceTolerance,
const Logger& logger = getDummyLogger()) const {
return detail::updateSingleSurfaceStatus<StraightLineStepper>(
*this, state, surface, navDir, bcheck, surfaceTolerance, logger);
*this, state, surface, index, navDir, bcheck, surfaceTolerance, logger);
}

/// Update step size
Expand Down
23 changes: 11 additions & 12 deletions Core/include/Acts/Propagator/detail/SteppingHelper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,18 @@ namespace detail {
template <typename stepper_t>
Acts::Intersection3D::Status updateSingleSurfaceStatus(
const stepper_t& stepper, typename stepper_t::State& state,
const Surface& surface, Direction navDir, const BoundaryCheck& bcheck,
ActsScalar surfaceTolerance, const Logger& logger) {
const Surface& surface, std::uint8_t index, Direction navDir,
const BoundaryCheck& bcheck, ActsScalar surfaceTolerance,
const Logger& logger) {
ACTS_VERBOSE(
"Update single surface status for surface: " << surface.geometryId());

auto sIntersection = surface.intersect(
state.geoContext, stepper.position(state),
navDir * stepper.direction(state), bcheck, surfaceTolerance);
navDir * stepper.direction(state), bcheck, surfaceTolerance)[index];

// The intersection is on surface already
if (sIntersection.closest().status() == Intersection3D::Status::onSurface) {
if (sIntersection.status() == Intersection3D::Status::onSurface) {
// Release navigation step size
state.stepSize.release(ConstrainedStep::actor);
ACTS_VERBOSE("Intersection: state is ON SURFACE");
Expand All @@ -54,14 +55,12 @@ Acts::Intersection3D::Status updateSingleSurfaceStatus(
const double pLimit = state.stepSize.value(ConstrainedStep::aborter);
const double oLimit = stepper.overstepLimit(state);

for (const auto& intersection : sIntersection.split()) {
if (intersection &&
detail::checkIntersection(intersection.intersection(), pLimit, oLimit,
surfaceTolerance, logger)) {
ACTS_VERBOSE("Surface is reachable");
stepper.setStepSize(state, intersection.pathLength());
return Intersection3D::Status::reachable;
}
if (sIntersection &&
detail::checkIntersection(sIntersection.intersection(), pLimit, oLimit,
surfaceTolerance, logger)) {
ACTS_VERBOSE("Surface is reachable");
stepper.setStepSize(state, sIntersection.pathLength());
return Intersection3D::Status::reachable;
}

ACTS_VERBOSE("Surface is NOT reachable");
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 @@ -582,7 +582,7 @@ BOOST_AUTO_TEST_CASE(StepSizeSurface) {
auto target = Surface::makeShared<PlaneSurface>(
pos + navDir * distance * unitDir, unitDir);

stepper.updateSurfaceStatus(state, *target, navDir, BoundaryCheck(false));
stepper.updateSurfaceStatus(state, *target, 0, navDir, BoundaryCheck(false));
BOOST_CHECK_EQUAL(state.stepSize.value(ConstrainedStep::actor), distance);

// test the step size modification in the context of a surface
Expand Down
3 changes: 2 additions & 1 deletion Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,8 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) {
// Test the intersection in the context of a surface
auto targetSurface =
Surface::makeShared<PlaneSurface>(pos + navDir * 2. * dir, dir);
es.updateSurfaceStatus(esState, *targetSurface, navDir, BoundaryCheck(false));
es.updateSurfaceStatus(esState, *targetSurface, 0, navDir,
BoundaryCheck(false));
CHECK_CLOSE_ABS(esState.stepSize.value(ConstrainedStep::actor), navDir * 2.,
eps);

Expand Down
Loading

0 comments on commit 3fd6166

Please sign in to comment.