Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@andiwand
andiwand committed Dec 13, 2024
1 parent 00ffc14 commit 48d5905
Show file tree
Hide file tree
Showing 38 changed files with 628 additions and 663 deletions.
8 changes: 4 additions & 4 deletions Core/include/Acts/Navigation/DetectorNavigator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,15 @@ class DetectorNavigator {
updateCandidateSurfaces(state, position);
}

NavigationTarget estimateNextTarget(State& state, const Vector3& position,
const Vector3& direction) const {
NavigationTarget nextTarget(State& state, const Vector3& position,
const Vector3& direction) const {
ACTS_VERBOSE(volInfo(state)
<< posInfo(state, position) << "Entering navigator::preStep.");

if (inactive()) {
ACTS_VERBOSE(volInfo(state)
<< posInfo(state, position) << "navigator inactive");
return NavigationTarget::invalid();
return NavigationTarget::None();
}

fillNavigationState(position, direction, state);
Expand All @@ -163,7 +163,7 @@ class DetectorNavigator {
if (state.surfaceCandidateIndex == state.surfaceCandidates.size()) {
ACTS_VERBOSE(volInfo(state)
<< posInfo(state, position) << "no surface candidates");
return NavigationTarget::invalid();
return NavigationTarget::None();
}

// Screen output how much is left to try
Expand Down
90 changes: 43 additions & 47 deletions Core/include/Acts/Propagator/AtlasStepper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
#include "Acts/Utilities/Result.hpp"

#include <cmath>
#include <functional>

// This is based original stepper code from the ATLAS RungeKuttaPropagator
namespace Acts {

/// @brief the AtlasStepper implementation for the
///
/// This is based original stepper code from the ATLAS RungeKuttaPropagator
class AtlasStepper {
public:
using Jacobian = BoundMatrix;
Expand All @@ -47,41 +47,36 @@ class AtlasStepper {
};

struct Options : public StepperPlainOptions {
explicit Options(const GeometryContext& gctx,
const MagneticFieldContext& mctx)
: StepperPlainOptions(gctx, mctx) {}

void setPlainOptions(const StepperPlainOptions& options) {
static_cast<StepperPlainOptions&>(*this) = options;
}
};

/// @brief Nested State struct for the local caching
struct State {
/// Default constructor - deleted
State() = delete;

/// Constructor
///
/// @tparam Type of TrackParameters
///
/// @param [in] gctx The geometry context tof this call
/// @param [in] optionsIn The stepper options
/// @param [in] fieldCacheIn The magnetic field cache for this call
/// @param [in] pars Input parameters
/// @param [in] ssize the steps size limitation
/// @param [in] stolerance is the stepping tolerance
template <typename Parameters>
State(const GeometryContext& gctx,
MagneticFieldProvider::Cache fieldCacheIn, const Parameters& pars,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance)
: particleHypothesis(pars.particleHypothesis()),
State(const Options& optionsIn, MagneticFieldProvider::Cache fieldCacheIn,
const Parameters& pars)
: options(optionsIn),
particleHypothesis(pars.particleHypothesis()),
field(0., 0., 0.),
stepSize(ssize),
tolerance(stolerance),
fieldCache(std::move(fieldCacheIn)),
geoContext(gctx) {
fieldCache(std::move(fieldCacheIn)) {
// The rest of this constructor is copy&paste of AtlasStepper::update() -
// this is a nasty but working solution for the stepper state without
// functions

const auto pos = pars.position(gctx);
const auto pos = pars.position(options.geoContext);
const auto Vp = pars.parameters();

double Sf = std::sin(Vp[eBoundPhi]);
Expand Down Expand Up @@ -111,7 +106,7 @@ class AtlasStepper {
covTransport = true;
useJacobian = true;
const auto transform = pars.referenceSurface().referenceFrame(
geoContext, pos, pars.direction());
options.geoContext, pos, pars.direction());

pVector[8] = transform(0, eBoundLoc0);
pVector[16] = transform(0, eBoundLoc1);
Expand Down Expand Up @@ -240,6 +235,8 @@ class AtlasStepper {
state_ready = true;
}

Options options;

ParticleHypothesis particleHypothesis;

// optimisation that init is not called twice
Expand Down Expand Up @@ -289,16 +286,10 @@ class AtlasStepper {
// Previous step size for overstep estimation
double previousStepSize = 0.;

/// The tolerance for the stepping
double tolerance = s_onSurfaceTolerance;

/// It caches the current magnetic field cell and stays (and interpolates)
/// within as long as this is valid. See step() code for details.
MagneticFieldProvider::Cache fieldCache;

/// Cache the geometry context
std::reference_wrapper<const GeometryContext> geoContext;

/// Debug output
/// the string where debug messages are stored (optionally)
bool debug = false;
Expand All @@ -316,12 +307,11 @@ class AtlasStepper {

explicit AtlasStepper(const Config& config) : m_bField(config.bField) {}

State makeState(std::reference_wrapper<const GeometryContext> gctx,
std::reference_wrapper<const MagneticFieldContext> mctx,
const BoundTrackParameters& par,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance) const {
return State{gctx, m_bField->makeCache(mctx), par, ssize, stolerance};
State makeState(const Options& options,
const BoundTrackParameters& par) const {
State state{options, m_bField->makeCache(options.magFieldContext), par};
state.stepSize = ConstrainedStep(options.maxStepSize);
return state;
}

/// @brief Resets the state
Expand All @@ -336,10 +326,10 @@ class AtlasStepper {
const BoundSquareMatrix& cov, const Surface& surface,
const double stepSize = std::numeric_limits<double>::max()) const {
// Update the stepping state
update(
state,
transformBoundToFreeParameters(surface, state.geoContext, boundParams),
boundParams, cov, surface);
update(state,
transformBoundToFreeParameters(surface, state.options.geoContext,
boundParams),
boundParams, cov, surface);
state.stepSize = ConstrainedStep(stepSize);
state.pathAccumulated = 0.;

Expand Down Expand Up @@ -418,15 +408,17 @@ class AtlasStepper {
/// @param [in] navDir The navigation direction
/// @param [in] boundaryTolerance The boundary check for this status update
/// @param [in] surfaceTolerance Surface tolerance used for intersection
/// @param [in] stype The step size type to be set
/// @param [in] release Do we release the step size?
/// @param [in] logger Logger instance to use
IntersectionStatus updateSurfaceStatus(
State& state, const Surface& surface, std::uint8_t index,
Direction navDir, const BoundaryTolerance& boundaryTolerance,
double surfaceTolerance = s_onSurfaceTolerance,
double surfaceTolerance, ConstrainedStep::Type stype, bool release,
const Logger& logger = getDummyLogger()) const {
return detail::updateSingleSurfaceStatus<AtlasStepper>(
*this, state, surface, index, navDir, boundaryTolerance,
surfaceTolerance, logger);
surfaceTolerance, stype, release, logger);
}

/// Update step size
Expand All @@ -439,8 +431,10 @@ class AtlasStepper {
/// @param release [in] boolean to trigger step size release
template <typename object_intersection_t>
void updateStepSize(State& state, const object_intersection_t& oIntersection,
Direction /*direction*/, bool release = true) const {
detail::updateSingleStepSize<AtlasStepper>(state, oIntersection, release);
Direction /*direction*/, ConstrainedStep::Type stype,
bool release) const {
double stepSize = oIntersection.pathLength();
updateStepSize(state, stepSize, stype, release);
}

/// Update step size - explicitly with a double
Expand All @@ -450,7 +444,7 @@ class AtlasStepper {
/// @param [in] stype The step size type to be set
/// @param release [in] Do we release the step size?
void updateStepSize(State& state, double stepSize,
ConstrainedStep::Type stype, bool release = true) const {
ConstrainedStep::Type stype, bool release) const {
state.previousStepSize = state.stepSize.value();
state.stepSize.update(stepSize, stype, release);
}
Expand Down Expand Up @@ -519,7 +513,7 @@ class AtlasStepper {

// Fill the end parameters
auto parameters = BoundTrackParameters::create(
surface.getSharedPtr(), state.geoContext, pos4, dir, qOverP,
surface.getSharedPtr(), state.options.geoContext, pos4, dir, qOverP,
std::move(covOpt), state.particleHypothesis);
if (!parameters.ok()) {
return parameters.error();
Expand Down Expand Up @@ -626,7 +620,8 @@ class AtlasStepper {
double Se = std::sin(boundParams[eBoundTheta]);
double Ce = std::cos(boundParams[eBoundTheta]);

const auto transform = surface.referenceFrame(state.geoContext, pos, mom);
const auto transform =
surface.referenceFrame(state.options.geoContext, pos, mom);

state.pVector[8] = transform(0, eBoundLoc0);
state.pVector[16] = transform(0, eBoundLoc1);
Expand Down Expand Up @@ -956,7 +951,8 @@ class AtlasStepper {
P[45] *= p;
P[46] *= p;

const auto fFrame = surface.referenceFrame(state.geoContext, gp, mom);
const auto fFrame =
surface.referenceFrame(state.options.geoContext, gp, mom);

double Ax[3] = {fFrame(0, 0), fFrame(1, 0), fFrame(2, 0)};
double Ay[3] = {fFrame(0, 1), fFrame(1, 1), fFrame(2, 1)};
Expand Down Expand Up @@ -985,9 +981,9 @@ class AtlasStepper {
if (surface.type() == Surface::Straw ||
surface.type() == Surface::Perigee) {
// vector from position to center
double x = P[0] - surface.center(state.geoContext).x();
double y = P[1] - surface.center(state.geoContext).y();
double z = P[2] - surface.center(state.geoContext).z();
double x = P[0] - surface.center(state.options.geoContext).x();
double y = P[1] - surface.center(state.options.geoContext).y();
double z = P[2] - surface.center(state.options.geoContext).z();

// this is the projection of the direction onto the local y axis
double d = P[4] * Ay[0] + P[5] * Ay[1] + P[6] * Ay[2];
Expand Down Expand Up @@ -1085,7 +1081,7 @@ class AtlasStepper {
// Jacobian production of transport and to_local
if (surface.type() == Surface::Disc) {
// the vector from the disc surface to the p
const auto& sfc = surface.center(state.geoContext);
const auto& sfc = surface.center(state.options.geoContext);
double d[3] = {P[0] - sfc(0), P[1] - sfc(1), P[2] - sfc(2)};
// this needs the transformation to polar coordinates
double RC = d[0] * Ax[0] + d[1] * Ax[1] + d[2] * Ax[2];
Expand Down
22 changes: 13 additions & 9 deletions Core/include/Acts/Propagator/DirectNavigator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ class DirectNavigator {
Direction direction = Direction::Forward;

/// Index of the next surface to try
/// @note -1 means before the first surface in the sequence and size()
/// means after the last surface in the sequence
int surfaceIndex = -1;

/// Navigation state - external interface: the current surface
Expand All @@ -105,8 +107,10 @@ class DirectNavigator {
}

bool endOfSurfaces() const {
return surfaceIndex < 0 ||
surfaceIndex >= static_cast<int>(options.surfaces.size());
if (direction == Direction::Forward) {
return surfaceIndex >= static_cast<int>(options.surfaces.size());
}
return surfaceIndex < 0;
}

int remainingSurfaces() const {
Expand Down Expand Up @@ -213,23 +217,23 @@ class DirectNavigator {
state.navigationBreak = false;
}

/// @brief Estimate the next target surface
/// @brief Get the next target surface
///
/// This function estimates the next target surface for the propagation. For
/// This function gets the next target surface for the propagation. For
/// the direct navigator this is always the next surface in the sequence.
///
/// @param state The navigation state
/// @param position The current position
/// @param direction The current direction
///
/// @return The next target surface
NavigationTarget estimateNextTarget(State& state, const Vector3& position,
const Vector3& direction) const {
NavigationTarget nextTarget(State& state, const Vector3& position,
const Vector3& direction) const {
if (state.navigationBreak) {
return NavigationTarget::invalid();
return NavigationTarget::None();
}

ACTS_VERBOSE("DirectNavigator::estimateNextTarget");
ACTS_VERBOSE("DirectNavigator::nextTarget");

// Navigator target always resets the current surface
state.currentSurface = nullptr;
Expand All @@ -246,7 +250,7 @@ class DirectNavigator {
} else {
ACTS_VERBOSE("End of surfaces reached, navigation break.");
state.navigationBreak = true;
return NavigationTarget::invalid();
return NavigationTarget::None();
}

// Establish & update the surface status
Expand Down
Loading

0 comments on commit 48d5905

Please sign in to comment.