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: Remove target estimation from navigator #2598

Closed
Closed
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
98 changes: 3 additions & 95 deletions Core/include/Acts/Propagator/Navigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ struct NavigationOptions {
/// object to check against: at end
const object_t* endObject = nullptr;

/// Target surface to exclude
const Surface* targetSurface = nullptr;
/// External surface identifier for which the boundary check is ignored
std::vector<GeometryIdentifier> externalSurfaces = {};

Expand Down Expand Up @@ -401,11 +399,6 @@ class Navigator {
// Call the navigation helper prior to actual navigation
ACTS_VERBOSE(volInfo(state) << "Entering navigator::preStep.");

// Initialize the target and target volume
if (state.navigation.targetSurface && !state.navigation.targetVolume) {
// Find out about the target as much as you can
initializeTarget(state, stepper);
}
// Try targeting the surfaces - then layers - then boundaries
if (state.navigation.navigationStage <= Stage::surfaceTarget &&
targetSurfaces(state, stepper)) {
Expand Down Expand Up @@ -564,6 +557,9 @@ class Navigator {
state.navigation.navigationBreak = true;
stepper.releaseStepSize(state.stepping);
} else {
// Set navigation break and release the navigation step size
state.navigation.navigationBreak = true;
stepper.releaseStepSize(state.stepping);
ACTS_VERBOSE(volInfo(state)
<< "Status could not be determined - good luck.");
}
Expand Down Expand Up @@ -876,11 +872,6 @@ class Navigator {
++state.navigation.navLayerIndex;
}

// Re-initialize target at last layer, only in case it is the target volume
// This avoids a wrong target volume estimation
if (state.navigation.currentVolume == state.navigation.targetVolume) {
initializeTarget(state, stepper);
}
// Screen output
if (logger().doPrint(Logging::VERBOSE)) {
std::ostringstream os;
Expand Down Expand Up @@ -946,9 +937,6 @@ class Navigator {
return true;
}

// Re-initialize target at volume boundary
initializeTarget(state, stepper);

// Helper function to find boundaries
auto findBoundaries = [&]() -> bool {
// The navigation options
Expand Down Expand Up @@ -1042,84 +1030,6 @@ class Navigator {
return false;
}

/// @brief Navigation (re-)initialisation for the target
///
/// @note This is only called a few times every propagation/extrapolation
///
/// As a straight line estimate can lead you to the wrong destination
/// Volume, this will be called at:
/// - initialization
/// - attempted volume switch
/// Target finding by association will not be done again
///
/// @tparam propagator_state_t The state type of the propagator
/// @tparam stepper_t The type of stepper used for the propagation
///
/// @param [in,out] state is the propagation state object
/// @param [in] stepper Stepper in use
///
/// boolean return triggers exit to stepper
template <typename propagator_state_t, typename stepper_t>
void initializeTarget(propagator_state_t& state,
const stepper_t& stepper) const {
if (state.navigation.targetVolume && state.stepping.pathAccumulated == 0.) {
ACTS_VERBOSE(volInfo(state)
<< "Re-initialzing cancelled as it is the first step.");
return;
}
// Fast Navigation initialization for target:
if (state.navigation.targetSurface &&
state.navigation.targetSurface->associatedLayer() &&
!state.navigation.targetVolume) {
ACTS_VERBOSE(volInfo(state)
<< "Fast target initialization through association.");
ACTS_VERBOSE(volInfo(state)
<< "Target surface set to "
<< state.navigation.targetSurface->geometryId());
state.navigation.targetLayer =
state.navigation.targetSurface->associatedLayer();
state.navigation.targetVolume =
state.navigation.targetLayer->trackingVolume();
} else if (state.navigation.targetSurface) {
// screen output that you do a re-initialization
if (state.navigation.targetVolume) {
ACTS_VERBOSE(volInfo(state)
<< "Re-initialization of target volume triggered.");
}
// Slow navigation initialization for target:
// target volume and layer search through global search
auto targetIntersection =
state.navigation.targetSurface
->intersect(
state.geoContext, stepper.position(state.stepping),
state.options.direction * stepper.direction(state.stepping),
false, state.options.targetTolerance)
.closest();
if (targetIntersection) {
ACTS_VERBOSE(volInfo(state)
<< "Target estimate position ("
<< targetIntersection.position().x() << ", "
<< targetIntersection.position().y() << ", "
<< targetIntersection.position().z() << ")");
/// get the target volume from the intersection
auto tPosition = targetIntersection.position();
state.navigation.targetVolume =
m_cfg.trackingGeometry->lowestTrackingVolume(state.geoContext,
tPosition);
state.navigation.targetLayer =
state.navigation.targetVolume
? state.navigation.targetVolume->associatedLayer(
state.geoContext, tPosition)
: nullptr;
if (state.navigation.targetVolume) {
ACTS_VERBOSE(volInfo(state)
<< "Target volume estimated : "
<< state.navigation.targetVolume->volumeName());
}
}
}
}

/// @brief Resolve the surfaces of this layer
///
/// @tparam propagator_state_t The state type of the propagator
Expand Down Expand Up @@ -1230,8 +1140,6 @@ class Navigator {
navOpts.resolveMaterial = m_cfg.resolveMaterial;
navOpts.resolvePassive = m_cfg.resolvePassive;
navOpts.startObject = startLayer;
// Set also the target surface
navOpts.targetSurface = state.navigation.targetSurface;
navOpts.pathLimit =
stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
navOpts.overstepLimit = stepper.overstepLimit(state.stepping);
Expand Down
3 changes: 1 addition & 2 deletions Core/src/Geometry/TrackingVolume.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -611,8 +611,7 @@ Acts::TrackingVolume::compatibleLayers(
auto path = atIntersection.pathLength();
bool withinLimit = std::abs(path) <= std::abs(options.pathLimit);
// Intersection is ok - take it (move to surface on approach)
if (atIntersection &&
(atIntersection.object() != options.targetSurface) && withinLimit) {
if (atIntersection && withinLimit) {
// create a layer intersection
lIntersections.push_back(LayerIntersection(
atIntersection.intersection(), tLayer, atIntersection.object(),
Expand Down
Loading