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: Refactor navigation #2768

Merged
merged 22 commits into from
Dec 19, 2023
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
9 changes: 4 additions & 5 deletions Core/include/Acts/Geometry/ApproachDescriptor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,16 @@ class ApproachDescriptor {
/// @param position is the position from start of the search
/// @param direction is the direction at the start of the search
/// @param bcheck is the boundary check directive
/// @param pLimit The path limit
/// @param oLimit The overstep limit
/// @param tolerance The surface tolerance
/// @param nearLimit The minimum distance for an intersection to be considered
/// @param farLimit The maximum distance for an intersection to be considered
///
/// @return is a surface intersection
virtual SurfaceIntersection approachSurface(const GeometryContext& gctx,
const Vector3& position,
const Vector3& direction,
const BoundaryCheck& bcheck,
double pLimit, double oLimit,
double tolerance) const = 0;
double nearLimit,
double farLimit) const = 0;

/// Get all the contained surfaces
/// @return all contained surfaces of this approach descriptor
Expand Down
12 changes: 5 additions & 7 deletions Core/include/Acts/Geometry/BoundarySurfaceT.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,12 @@ class BoundarySurfaceT {
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param pos The global position on surface
/// @param mom The direction on the surface
/// @param dir is an additional direction corrective
/// @param dir The direction on the surface
///
/// @return The attached volume at that position
virtual const volume_t* attachedVolume(const GeometryContext& gctx,
const Vector3& pos, const Vector3& mom,
Direction dir) const;
const Vector3& pos,
const Vector3& dir) const;

/// templated onBoundary method
///
Expand Down Expand Up @@ -181,11 +180,10 @@ void BoundarySurfaceT<volume_t>::attachVolumeArray(

template <class volume_t>
const volume_t* BoundarySurfaceT<volume_t>::attachedVolume(
const GeometryContext& gctx, const Vector3& pos, const Vector3& mom,
Direction dir) const {
const GeometryContext& gctx, const Vector3& pos, const Vector3& dir) const {
const volume_t* attVolume = nullptr;
// dot product with normal vector to distinguish inside/outside
if ((surfaceRepresentation().normal(gctx, pos)).dot(dir * mom) > 0.) {
if ((surfaceRepresentation().normal(gctx, pos)).dot(dir) > 0.) {
attVolume = m_alongVolumeArray ? m_alongVolumeArray->object(pos).get()
: m_alongVolume;
} else {
Expand Down
9 changes: 4 additions & 5 deletions Core/include/Acts/Geometry/GenericApproachDescriptor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,16 @@ class GenericApproachDescriptor : public ApproachDescriptor {
/// @param position The global position to start the approach from
/// @param direction The momentum vector
/// @param bcheck The boundary check prescription
/// @param pLimit The path limit
/// @param oLimit The overstep limit
/// @param tolerance The surface tolerance
/// @param nearLimit The minimum distance for an intersection to be considered
/// @param farLimit The maximum distance for an intersection to be considered
///
/// @return : a @c SurfaceIntersection
SurfaceIntersection approachSurface(const GeometryContext& gctx,
const Vector3& position,
const Vector3& direction,
const BoundaryCheck& bcheck,
double pLimit, double oLimit,
double tolerance) const override;
double nearLimit,
double farLimit) const override;

/// return all contained surfaces of this approach descriptor
const std::vector<const Surface*>& containedSurfaces() const override;
Expand Down
1 change: 0 additions & 1 deletion Core/include/Acts/Geometry/NavigationLayer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ namespace Acts {
/// Class to be used for gaps in Volumes as a navigational link.
/// Navigation Layers have a surface representation, but should usually never be
/// propagated to.

class NavigationLayer : public Layer {
public:
/// Factory Constructor - the surface representation is given by pointer
Expand Down
40 changes: 15 additions & 25 deletions Core/include/Acts/Navigation/DetectorNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,6 @@ class DetectorNavigator {
return result;
}

void resetState(State& state, const GeometryContext& /*geoContext*/,
const Vector3& /*pos*/, const Vector3& /*dir*/,
const Surface* /*ssurface*/,
const Surface* /*tsurface*/) const {
// Reset everything first
state = State();

// TODO fill state
}
andiwand marked this conversation as resolved.
Show resolved Hide resolved

const Surface* currentSurface(const State& state) const {
return state.currentSurface;
}
Expand Down Expand Up @@ -202,24 +192,24 @@ class DetectorNavigator {
<< posInfo(state, stepper) << "stepping through portal");

nState.surfaceCandidates.clear();
nState.surfaceCandidate = nState.surfaceCandidates.cend();
nState.surfaceCandidateIndex = 0;

nState.currentPortal->updateDetectorVolume(state.geoContext, nState);

initializeTarget(state, stepper);
}

for (; nState.surfaceCandidate != nState.surfaceCandidates.cend();
++nState.surfaceCandidate) {
for (; nState.surfaceCandidateIndex != nState.surfaceCandidates.size();
++nState.surfaceCandidateIndex) {
// Screen output how much is left to try
ACTS_VERBOSE(volInfo(state)
<< posInfo(state, stepper)
<< std::distance(nState.surfaceCandidate,
nState.surfaceCandidates.cend())
<< (nState.surfaceCandidates.size() -
nState.surfaceCandidateIndex)
<< " out of " << nState.surfaceCandidates.size()
<< " surfaces remain to try.");
// Take the surface
const auto& c = *(nState.surfaceCandidate);
const auto& c = nState.surfaceCandidate();
const auto& surface =
(c.surface != nullptr) ? (*c.surface) : (c.portal->surface());
// Screen output which surface you are on
Expand Down Expand Up @@ -271,7 +261,7 @@ class DetectorNavigator {
return;
}

if (nState.surfaceCandidate == nState.surfaceCandidates.cend()) {
if (nState.surfaceCandidateIndex == nState.surfaceCandidates.size()) {
ACTS_VERBOSE(volInfo(state)
<< posInfo(state, stepper)
<< "no surface candidates - waiting for target call");
Expand All @@ -281,12 +271,12 @@ class DetectorNavigator {
const Portal* nextPortal = nullptr;
const Surface* nextSurface = nullptr;
bool isPortal = false;
bool boundaryCheck = nState.surfaceCandidate->boundaryCheck;
bool boundaryCheck = nState.surfaceCandidate().boundaryCheck;

if (nState.surfaceCandidate->surface != nullptr) {
nextSurface = nState.surfaceCandidate->surface;
} else if (nState.surfaceCandidate->portal != nullptr) {
nextPortal = nState.surfaceCandidate->portal;
if (nState.surfaceCandidate().surface != nullptr) {
nextSurface = nState.surfaceCandidate().surface;
} else if (nState.surfaceCandidate().portal != nullptr) {
nextPortal = nState.surfaceCandidate().portal;
nextSurface = &nextPortal->surface();
isPortal = true;
} else {
Expand All @@ -299,7 +289,7 @@ class DetectorNavigator {
// TODO not sure about the boundary check
auto surfaceStatus = stepper.updateSurfaceStatus(
state.stepping, *nextSurface,
nState.surfaceCandidate->objectIntersection.index(),
nState.surfaceCandidate().objectIntersection.index(),
state.options.direction, BoundaryCheck(boundaryCheck),
state.options.surfaceTolerance, logger());

Expand Down Expand Up @@ -327,7 +317,7 @@ class DetectorNavigator {
ACTS_VERBOSE(volInfo(state)
<< posInfo(state, stepper) << "current surface set to "
<< nState.currentSurface->geometryId());
++nState.surfaceCandidate;
++nState.surfaceCandidateIndex;
}
}
}
Expand Down Expand Up @@ -426,7 +416,7 @@ class DetectorNavigator {
return pathToA < pathToB;
});
// Set the surface candidate
nState.surfaceCandidate = nCandidates.begin();
nState.surfaceCandidateIndex = 0;
}

template <typename propagator_state_t, typename stepper_t>
Expand Down
10 changes: 8 additions & 2 deletions Core/include/Acts/Navigation/NavigationState.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
#pragma once

#include "Acts/Definitions/Algebra.hpp"
#include "Acts/Definitions/Units.hpp"
#include "Acts/Geometry/GeometryContext.hpp"
#include "Acts/Surfaces/BoundaryCheck.hpp"
#include "Acts/Utilities/Delegate.hpp"
#include "Acts/Utilities/Intersection.hpp"

#include <any>
#include <cstddef>
#include <vector>

namespace Acts {
Expand Down Expand Up @@ -80,16 +82,20 @@ struct NavigationState {

/// That are the candidate surfaces to process
SurfaceCandidates surfaceCandidates = {};
SurfaceCandidates::const_iterator surfaceCandidate = surfaceCandidates.cend();
std::size_t surfaceCandidateIndex = 0;
paulgessinger marked this conversation as resolved.
Show resolved Hide resolved

/// Boundary directives for surfaces
BoundaryCheck surfaceBoundaryCheck = BoundaryCheck(true);

/// An overstep tolerance
ActsScalar overstepTolerance = -0.1;
ActsScalar overstepTolerance = -100 * UnitConstants::um;

/// Auxiliary attached information
std::any auxiliary;

const SurfaceCandidate& surfaceCandidate() const {
return surfaceCandidates.at(surfaceCandidateIndex);
}
};

} // namespace Experimental
Expand Down
13 changes: 11 additions & 2 deletions Core/include/Acts/Navigation/NavigationStateUpdaters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "Acts/Definitions/Common.hpp"
#include "Acts/Detector/Portal.hpp"
#include "Acts/Navigation/NavigationDelegates.hpp"
#include "Acts/Navigation/NavigationState.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/Enumerate.hpp"
Expand All @@ -38,9 +39,10 @@ inline void updateCandidates(const GeometryContext& gctx,
NavigationState& nState) {
const auto& position = nState.position;
const auto& direction = nState.direction;
auto& nCandidates = nState.surfaceCandidates;

for (auto& c : nCandidates) {
NavigationState::SurfaceCandidates nextSurfaceCandidates;

for (NavigationState::SurfaceCandidate c : nState.surfaceCandidates) {
// Get the surface representation: either native surfcae of portal
const Surface& sRep =
c.surface != nullptr ? *c.surface : c.portal->surface();
Expand All @@ -50,7 +52,14 @@ inline void updateCandidates(const GeometryContext& gctx,
auto sIntersection = sRep.intersect(gctx, position, direction,
c.boundaryCheck, s_onSurfaceTolerance);
c.objectIntersection = sIntersection[c.objectIntersection.index()];

if (c.objectIntersection &&
c.objectIntersection.pathLength() > nState.overstepTolerance) {
nextSurfaceCandidates.emplace_back(std::move(c));
}
}

nState.surfaceCandidates = std::move(nextSurfaceCandidates);
}

/// @brief This sets a single object, e.g. single surface or single volume
Expand Down
Loading
Loading