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 direction from NavigationOptions and drop constructor #2345

Merged
merged 10 commits into from
Aug 14, 2023
2 changes: 1 addition & 1 deletion Core/include/Acts/Navigation/DetectorNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class DetectorNavigator {

void resetState(State& state, const GeometryContext& /*geoContext*/,
const Vector3& /*pos*/, const Vector3& /*dir*/,
Direction /*navDir*/, const Surface* /*ssurface*/,
const Surface* /*ssurface*/,
const Surface* /*tsurface*/) const {
// Reset everything first
state = State();
Expand Down
3 changes: 1 addition & 2 deletions Core/include/Acts/Propagator/DirectNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,7 @@ class DirectNavigator {
/// @param tsurface is the target surface
void resetState(State& state, const GeometryContext& /*geoContext*/,
const Vector3& /*pos*/, const Vector3& /*dir*/,
Direction /*navDir*/, const Surface* ssurface,
const Surface* tsurface) const {
const Surface* ssurface, const Surface* tsurface) const {
// Reset everything except the navSurfaces
auto navSurfaces = state.navSurfaces;
state = State();
Expand Down
87 changes: 34 additions & 53 deletions Core/include/Acts/Propagator/Navigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ namespace Acts {
/// @tparam object_t Type of the object for navigation to check against
template <typename object_t>
struct NavigationOptions {
/// The navigation direction
Direction navDir = Direction::Forward;

/// The boundary check directive
BoundaryCheck boundaryCheck = true;

Expand All @@ -63,35 +60,10 @@ struct NavigationOptions {
double pathLimit = std::numeric_limits<double>::max();

/// The overstep tolerance for this navigation step
/// @note must be negative as it describes overstepping
/// @todo could be dynamic in the future (pT dependent)
double overstepLimit = -1 * UnitConstants::um;
double overstepLimit = 0;

/// Force intersection with boundaries
bool forceIntersectBoundaries = false;

/// Constructor
///
/// @param ndir Navigation direction prescription
/// @param bcheck Boundary check for the navigation action
/// @param resolves Boolean whether to resolve sensitives
/// @param resolvem Boolean whether to resolve material
/// @param resolvep Boolean whether to resolve passives
/// @param sobject Start object to check against
/// @param eobject End object to check against
NavigationOptions(Direction ndir, BoundaryCheck bcheck, bool resolves = true,
bool resolvem = true, bool resolvep = false,
const object_t* sobject = nullptr,
const object_t* eobject = nullptr)
: navDir(ndir),
boundaryCheck(std::move(bcheck)),
resolveSensitive(resolves),
resolveMaterial(resolvem),
resolvePassive(resolvep),
startObject(sobject),
endObject(eobject),
pathLimit(ndir * std::numeric_limits<double>::max()),
overstepLimit(-1 * UnitConstants::um) {}
};

/// Navigator class
Expand Down Expand Up @@ -244,12 +216,11 @@ class Navigator {
/// @param state is the state
/// @param geoContext is the geometry context
/// @param pos is the global position
/// @param dir is the momentum direction
/// @param navDir is the navigation direction
/// @param dir is the direction of navigation
/// @param ssurface is the new starting surface
/// @param tsurface is the target surface
void resetState(State& state, const GeometryContext& geoContext,
const Vector3& pos, const Vector3& dir, Direction navDir,
const Vector3& pos, const Vector3& dir,
const Surface* ssurface, const Surface* tsurface) const {
// Reset everything first
state = State();
Expand All @@ -267,8 +238,10 @@ class Navigator {
state.targetSurface = tsurface;

// Get the compatible layers (including the current layer)
NavigationOptions<Layer> navOpts(navDir, true, true, true, true, nullptr,
nullptr);
NavigationOptions<Layer> navOpts;
navOpts.resolveSensitive = true;
navOpts.resolveMaterial = true;
navOpts.resolvePassive = true;
state.navLayers =
state.currentVolume->compatibleLayers(geoContext, pos, dir, navOpts);

Expand Down Expand Up @@ -786,10 +759,11 @@ class Navigator {
// check if current volume has BVH, or layers
if (state.navigation.currentVolume->hasBoundingVolumeHierarchy()) {
// has hierarchy, use that, skip layer resolution
NavigationOptions<Surface> navOpts(
state.options.direction, true, m_cfg.resolveSensitive,
m_cfg.resolveMaterial, m_cfg.resolvePassive, nullptr,
state.navigation.targetSurface);
NavigationOptions<Surface> navOpts;
navOpts.resolveSensitive = m_cfg.resolveSensitive;
navOpts.resolveMaterial = m_cfg.resolveMaterial;
navOpts.resolvePassive = m_cfg.resolvePassive;
navOpts.endObject = state.navigation.targetSurface;
navOpts.overstepLimit = stepper.overstepLimit(state.stepping);
double opening_angle = 0;

Expand Down Expand Up @@ -826,7 +800,8 @@ class Navigator {
auto protoNavSurfaces =
state.navigation.currentVolume->compatibleSurfacesFromHierarchy(
state.geoContext, stepper.position(state.stepping),
stepper.direction(state.stepping), opening_angle, navOpts);
state.options.direction * stepper.direction(state.stepping),
opening_angle, navOpts);
if (!protoNavSurfaces.empty()) {
// did we find any surfaces?

Expand Down Expand Up @@ -967,15 +942,15 @@ class Navigator {
// Helper function to find boundaries
auto findBoundaries = [&]() -> bool {
// The navigation options
NavigationOptions<Surface> navOpts(state.options.direction, true);
NavigationOptions<Surface> navOpts;
// Exclude the current surface in case it's a boundary
navOpts.startObject = state.navigation.currentSurface;
navOpts.pathLimit =
stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
navOpts.overstepLimit = stepper.overstepLimit(state.stepping);
navOpts.forceIntersectBoundaries =
state.navigation.forceIntersectBoundaries;

// Exclude the current surface in case it's a boundary
navOpts.startObject = state.navigation.currentSurface;
ACTS_VERBOSE(volInfo(state)
<< "Try to find boundaries, we are at: "
<< stepper.position(state.stepping).transpose() << ", dir: "
Expand All @@ -985,7 +960,8 @@ class Navigator {
state.navigation.navBoundaries =
state.navigation.currentVolume->compatibleBoundaries(
state.geoContext, stepper.position(state.stepping),
stepper.direction(state.stepping), navOpts, logger());
state.options.direction * stepper.direction(state.stepping),
navOpts, logger());
// The number of boundary candidates
if (logger().doPrint(Logging::VERBOSE)) {
std::ostringstream os;
Expand Down Expand Up @@ -1151,10 +1127,12 @@ class Navigator {
bool onStart = (navLayer == state.navigation.startLayer);
auto startSurface = onStart ? state.navigation.startSurface : layerSurface;
// Use navigation parameters and NavigationOptions
NavigationOptions<Surface> navOpts(
state.options.direction, true, m_cfg.resolveSensitive,
m_cfg.resolveMaterial, m_cfg.resolvePassive, startSurface,
state.navigation.targetSurface);
NavigationOptions<Surface> navOpts;
navOpts.resolveSensitive = m_cfg.resolveSensitive;
navOpts.resolveMaterial = m_cfg.resolveMaterial;
navOpts.resolvePassive = m_cfg.resolvePassive;
navOpts.startObject = startSurface;
navOpts.endObject = state.navigation.targetSurface;

std::vector<GeometryIdentifier> externalSurfaces;
if (!state.navigation.externalSurfaces.empty()) {
Expand All @@ -1179,7 +1157,7 @@ class Navigator {
// get the surfaces
state.navigation.navSurfaces = navLayer->compatibleSurfaces(
state.geoContext, stepper.position(state.stepping),
stepper.direction(state.stepping), navOpts);
state.options.direction * stepper.direction(state.stepping), navOpts);
// the number of layer candidates
if (!state.navigation.navSurfaces.empty()) {
if (logger().doPrint(Logging::VERBOSE)) {
Expand Down Expand Up @@ -1232,10 +1210,12 @@ class Navigator {
: nullptr;
// Create the navigation options
// - and get the compatible layers, start layer will be excluded
NavigationOptions<Layer> navOpts(
state.options.direction, m_cfg.boundaryCheckLayerResolving,
m_cfg.resolveSensitive, m_cfg.resolveMaterial, m_cfg.resolvePassive,
startLayer, nullptr);
NavigationOptions<Layer> navOpts;
navOpts.boundaryCheck = m_cfg.boundaryCheckLayerResolving;
navOpts.resolveSensitive = m_cfg.resolveSensitive;
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 =
Expand All @@ -1245,7 +1225,8 @@ class Navigator {
state.navigation.navLayers =
state.navigation.currentVolume->compatibleLayers(
state.geoContext, stepper.position(state.stepping),
stepper.direction(state.stepping), navOpts);
state.options.direction * stepper.direction(state.stepping),
navOpts);

// Layer candidates have been found
if (!state.navigation.navLayers.empty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ class CombinatorialKalmanFilter {
// after smoothing
navigator.resetState(
state.navigation, state.geoContext, stepper.position(state.stepping),
stepper.direction(state.stepping), state.options.direction,
state.options.direction * stepper.direction(state.stepping),
&currentState.referenceSurface(), nullptr);

// No Kalman filtering for the starting surface, but still need
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/TrackFitting/KalmanFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ class KalmanFitter {
// Reset navigation state
navigator.resetState(
state.navigation, state.geoContext, stepper.position(state.stepping),
stepper.direction(state.stepping), state.options.direction,
state.options.direction * stepper.direction(state.stepping),
&st.referenceSurface(), targetSurface);

// Update material effects for last measurement state in reversed
Expand Down
20 changes: 6 additions & 14 deletions Core/src/Geometry/Layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ Acts::Layer::compatibleSurfaces(
// intersect the end surface
// - it is the final one don't use the boundary check at all
SurfaceIntersection endInter = options.endObject->intersect(
gctx, position, options.navDir * direction, BoundaryCheck(true));
gctx, position, direction, BoundaryCheck(true));
// non-valid intersection with the end surface provided at this layer
// indicates wrong direction or faulty setup
// -> do not return compatible surfaces since they may lead you on a wrong
Expand All @@ -148,7 +148,7 @@ Acts::Layer::compatibleSurfaces(
// -> this avoids punch through for cylinders
double pCorrection =
surfaceRepresentation().pathCorrection(gctx, position, direction);
pathLimit = 1.5 * thickness() * pCorrection * options.navDir;
pathLimit = 1.5 * thickness() * pCorrection;
}

// lemma 0 : accept the surface
Expand Down Expand Up @@ -185,12 +185,11 @@ Acts::Layer::compatibleSurfaces(
}
// the surface intersection
SurfaceIntersection sfi =
sf.intersect(gctx, position, options.navDir * direction, boundaryCheck);
sf.intersect(gctx, position, direction, boundaryCheck);
// check if intersection is valid and pathLimit has not been exceeded
if (sfi && detail::checkIntersection(sfi.intersection, pathLimit,
overstepLimit, s_onSurfaceTolerance)) {
// Now put the right sign on it
sfi.intersection.pathLength *= options.navDir;
sIntersections.push_back(sfi);
}
};
Expand Down Expand Up @@ -250,11 +249,7 @@ Acts::Layer::compatibleSurfaces(
sIntersections.resize(std::distance(sIntersections.begin(), it));

// sort according to the path length
if (options.navDir == Direction::Forward) {
std::sort(sIntersections.begin(), sIntersections.end());
} else {
std::sort(sIntersections.begin(), sIntersections.end(), std::greater<>());
}
std::sort(sIntersections.begin(), sIntersections.end());

return sIntersections;
}
Expand All @@ -272,9 +267,6 @@ Acts::SurfaceIntersection Acts::Layer::surfaceOnApproach(
(m_ssSensitiveSurfaces > 1 || m_ssApproachSurfaces > 1 ||
(surfaceRepresentation().surfaceMaterial() != nullptr));

// The signed direction: solution (except overstepping) is positive
auto sDirection = options.navDir * direction;

// The Limits: current path & overstepping
double pLimit = options.pathLimit;
double oLimit = options.overstepLimit;
Expand Down Expand Up @@ -305,13 +297,13 @@ Acts::SurfaceIntersection Acts::Layer::surfaceOnApproach(
// Approach descriptor present and resolving is necessary
if (m_approachDescriptor && (resolvePS || resolveMS)) {
SurfaceIntersection aSurface = m_approachDescriptor->approachSurface(
gctx, position, sDirection, options.boundaryCheck);
gctx, position, direction, options.boundaryCheck);
return checkIntersection(aSurface);
}

// Intersect and check the representing surface
const Surface& rSurface = surfaceRepresentation();
auto sIntersection =
rSurface.intersect(gctx, position, sDirection, options.boundaryCheck);
rSurface.intersect(gctx, position, direction, options.boundaryCheck);
return checkIntersection(sIntersection);
}
27 changes: 8 additions & 19 deletions Core/src/Geometry/TrackingVolume.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,6 @@ Acts::TrackingVolume::compatibleBoundaries(
auto excludeObject = options.startObject;
boost::container::small_vector<Acts::BoundaryIntersection, 4> bIntersections;

// The signed direction: solution (except overstepping) is positive
auto sDirection = options.navDir * direction;

// The Limits: current, path & overstepping
double pLimit = options.pathLimit;
double oLimit = options.overstepLimit;
Expand Down Expand Up @@ -550,7 +547,7 @@ Acts::TrackingVolume::compatibleBoundaries(

// Exclude the boundary where you are on
if (excludeObject != &bSurfaceRep) {
auto bCandidate = bSurfaceRep.intersect(gctx, position, sDirection,
auto bCandidate = bSurfaceRep.intersect(gctx, position, direction,
options.boundaryCheck);
// Intersect and continue
auto bIntersection = checkIntersection(bCandidate, bsIter.get());
Expand Down Expand Up @@ -635,10 +632,9 @@ Acts::TrackingVolume::compatibleLayers(
}
}
// move to next one or break because you reached the end layer
tLayer =
(tLayer == options.endObject)
? nullptr
: tLayer->nextLayer(gctx, position, options.navDir * direction);
tLayer = (tLayer == options.endObject)
? nullptr
: tLayer->nextLayer(gctx, position, direction);
}
std::sort(lIntersections.begin(), lIntersections.end());
}
Expand Down Expand Up @@ -693,16 +689,13 @@ Acts::TrackingVolume::compatibleSurfacesFromHierarchy(
return sIntersections;
}

// The signed direction
Vector3 sdir = options.navDir * direction;

std::vector<const Volume*> hits;
if (angle == 0) {
// use ray
Ray3D obj(position, sdir);
Ray3D obj(position, direction);
hits = intersectSearchHierarchy(std::move(obj), m_bvhTop);
} else {
Acts::Frustum<ActsScalar, 3, 4> obj(position, sdir, angle);
Acts::Frustum<ActsScalar, 3, 4> obj(position, direction, angle);
hits = intersectSearchHierarchy(std::move(obj), m_bvhTop);
}

Expand All @@ -713,7 +706,7 @@ Acts::TrackingVolume::compatibleSurfacesFromHierarchy(
boundarySurfaces = avol->boundarySurfaces();
for (const auto& bs : boundarySurfaces) {
const Surface& srf = bs->surfaceRepresentation();
auto sfi = srf.intersect(gctx, position, sdir, false);
auto sfi = srf.intersect(gctx, position, direction, false);
if (sfi and sfi.intersection.pathLength > oLimit and
sfi.intersection.pathLength <= pLimit) {
sIntersections.push_back(std::move(sfi));
Expand All @@ -722,11 +715,7 @@ Acts::TrackingVolume::compatibleSurfacesFromHierarchy(
}

// Sort according to the path length
if (options.navDir == Direction::Forward) {
std::sort(sIntersections.begin(), sIntersections.end());
} else {
std::sort(sIntersections.begin(), sIntersections.end(), std::greater<>());
}
std::sort(sIntersections.begin(), sIntersections.end());

return sIntersections;
}
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 @@ -25,19 +25,19 @@ test_material_recording__geant4_material_tracks.root: e411152d370775463c22b19a35
test_truth_tracking_kalman[generic-0.0]__trackstates_fitter.root: 7e933e0219728ad8c139c2c8d0a7b09d133108a2c3053b0e1972189e34534592
test_truth_tracking_kalman[generic-0.0]__tracksummary_fitter.root: e41639378960b2ecbcc31ad432e29eb56b6d900e642fdc53879d0708f8ab9155
test_truth_tracking_kalman[generic-0.0]__performance_track_finder.root: 7fc6f717723c9eddcbf44820b384b373cee6f04b72f79902f938f35e3ff9b470
test_truth_tracking_kalman[generic-1000.0]__trackstates_fitter.root: 6c0313a50148aa71e20d817642cd2dffc85e467915298f00d75bef33ad83cf3c
test_truth_tracking_kalman[generic-1000.0]__trackstates_fitter.root: 2298c2ae839e8208bc640e7208d2dc406f3316042c50f23ff0a65e4652a0bc2c
test_truth_tracking_kalman[generic-1000.0]__tracksummary_fitter.root: fd522a5636614a4b987802bbb8c9ff9b5eb0d9b0e876a3066e66354eaa297398
test_truth_tracking_kalman[generic-1000.0]__performance_track_finder.root: 7fc6f717723c9eddcbf44820b384b373cee6f04b72f79902f938f35e3ff9b470
test_truth_tracking_kalman[odd-0.0]__trackstates_fitter.root: 4093d18a84cd01ddec712adadcf8f271e7dd1fa473a734f44b80963477fc4f54
test_truth_tracking_kalman[odd-0.0]__tracksummary_fitter.root: d33319894494feb8c0d6f0f90a1d1e59caf9de32eda480ab73d0e40dfcdd1322
test_truth_tracking_kalman[odd-0.0]__performance_track_finder.root: 39aec6316cceb90e314e16b02947faa691c18f57c3a851a25e547a8fc05a4593
test_truth_tracking_kalman[odd-1000.0]__trackstates_fitter.root: 0fcf2020a10ddd7030cd1b58ef7affd60121b83eabb92e9547818e93bab9c777
test_truth_tracking_kalman[odd-1000.0]__trackstates_fitter.root: 11f863960ba24d32832a620e3a4447fd8efceda09b620bdaef35a5fcc54a5676
test_truth_tracking_kalman[odd-1000.0]__tracksummary_fitter.root: 99fe9c3be034549816c73f7f3d2190df7efb0adf6aed16cceb86aa4a9ac271f0
test_truth_tracking_kalman[odd-1000.0]__performance_track_finder.root: 39aec6316cceb90e314e16b02947faa691c18f57c3a851a25e547a8fc05a4593
test_truth_tracking_gsf[generic]__trackstates_gsf.root: 787294f42fadbd14827ae47133ba90d657999fa815df3fa01e3ddc3c0709d880
test_truth_tracking_gsf[generic]__tracksummary_gsf.root: abfa600668eda81fa0542df2d9ced6d550d744f1f8f3f4c789d137f7748363e2
test_truth_tracking_gsf[odd]__trackstates_gsf.root: 0c6b46ee55e992c0846abea69d69205cbec358aa48330396ce83955bb2153177
test_truth_tracking_gsf[odd]__tracksummary_gsf.root: 2b6f6b95af4bd36d0a1084a6fb79e0d09dac622f81c8fd1a0c5550c4cc458b66
test_truth_tracking_gsf[odd]__trackstates_gsf.root: 8ea8bdeda5ba2d4fbedd5b25b1d9f01a47980f9ae5398ad157b40ec5fa070cda
test_truth_tracking_gsf[odd]__tracksummary_gsf.root: 690b72c9ed7a6779d6233b8b68d05e9b860165efd258ad80581485a4e01d9996
test_particle_gun__particles.root: 8549ba6e20338004ab8ba299fc65e1ee5071985b46df8f77f887cb6fef56a8ec
test_material_mapping__material-map_tracks.root: 4e1c866038f0c06b099aa74fd01c3d875f07b89f54898f90debd9b558d8e4025
test_material_mapping__propagation-material.root: 646b8e2bbacec40d0bc4132236f9ab3f03b088e656e6e9b80c47ae03eaf6eab5
Expand Down