Skip to content

Commit

Permalink
refactor!: Make Navigator use a config struct (#826)
Browse files Browse the repository at this point in the history
Navigator currently has a number of members that act as configuration properties. In order to comply with our general philosophy to use nested `Config` structs for these kinds of properties, this PR adds `Navigator::Config`.

BREAKING CHANGE: The constructor and public members of `Acts::Navigator` changes. When before it could be created like
```cpp
Acts::Navigator nav{tGeometry};
nav.resolveSensitive = true;
nav.resolveMaterial = true;
nav.resolvePassive = true;
```
it must now be created like 
```cpp
Acts::Navigator::Config cfg;
cfg.trackingGeometry = tGeometry;
nav.resolveSensitive = true;
nav.resolveMaterial = true;
nav.resolvePassive = true;
Acts::Navigator nav{cfg};
```
Since `trackingGeometry` is the first member of `Acts::Navigator::Config`, if you don't want to change the `resolve*` values, you can also write
```cpp
Acts::Navigator nav{{tGeometry}};
```
  • Loading branch information
paulgessinger committed Jun 9, 2021
1 parent 03dd438 commit 641e00c
Show file tree
Hide file tree
Showing 23 changed files with 236 additions and 206 deletions.
73 changes: 41 additions & 32 deletions Core/include/Acts/Propagator/Navigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,25 +136,21 @@ class Navigator {
boundaryTarget = 3
};

/// Constructor with shared tracking geometry
///
/// @param tGeometry The tracking geometry for the navigator
Navigator(std::shared_ptr<const TrackingGeometry> tGeometry = nullptr)
: trackingGeometry(std::move(tGeometry)) {}

/// Tracking Geometry for this Navigator
std::shared_ptr<const TrackingGeometry> trackingGeometry;

/// The tolerance used to defined "reached"
double tolerance = s_onSurfaceTolerance;

/// Configuration for this Navigator
/// stop at every sensitive surface (whether it has material or not)
bool resolveSensitive = true;
/// stop at every material surface (whether it is passive or not)
bool resolveMaterial = true;
/// stop at every surface regardless what it is
bool resolvePassive = false;
struct Config {
/// Tracking Geometry for this Navigator
std::shared_ptr<const TrackingGeometry> trackingGeometry{nullptr};

/// Configuration for this Navigator
/// stop at every sensitive surface (whether it has material or not)
bool resolveSensitive = true;
/// stop at every material surface (whether it is passive or not)
bool resolveMaterial = true;
/// stop at every surface regardless what it is
bool resolvePassive = false;

/// The tolerance used to defined "reached"
double tolerance = s_onSurfaceTolerance;
};

/// Nested State struct
///
Expand Down Expand Up @@ -216,6 +212,11 @@ class Navigator {
Stage navigationStage = Stage::undefined;
};

/// Constructor with configuration object
///
/// @param cfg The navigator configuration
explicit Navigator(Config cfg) : m_cfg{std::move(cfg)} {}

/// @brief Navigator status call, will be called in two modes
///
/// (a) It initializes the Navigation stream if start volume is
Expand Down Expand Up @@ -428,7 +429,8 @@ class Navigator {
ACTS_VERBOSE(volInfo(state) << "Initialization.");
// Set the world volume if it is not set
if (not state.navigation.worldVolume) {
state.navigation.worldVolume = trackingGeometry->highestTrackingVolume();
state.navigation.worldVolume =
m_cfg.trackingGeometry->highestTrackingVolume();
}

// We set the current surface to the start surface
Expand Down Expand Up @@ -475,8 +477,9 @@ class Navigator {
<< toString(stepper.position(state.stepping))
<< " and direction "
<< toString(stepper.direction(state.stepping)));
state.navigation.startVolume = trackingGeometry->lowestTrackingVolume(
state.geoContext, stepper.position(state.stepping));
state.navigation.startVolume =
m_cfg.trackingGeometry->lowestTrackingVolume(
state.geoContext, stepper.position(state.stepping));
state.navigation.startLayer =
state.navigation.startVolume
? state.navigation.startVolume->associatedLayer(
Expand Down Expand Up @@ -691,8 +694,9 @@ class Navigator {
if (state.navigation.currentVolume->hasBoundingVolumeHierarchy()) {
// has hierarchy, use that, skip layer resolution
NavigationOptions<Surface> navOpts(
state.stepping.navDir, true, resolveSensitive, resolveMaterial,
resolvePassive, nullptr, state.navigation.targetSurface);
state.stepping.navDir, true, m_cfg.resolveSensitive,
m_cfg.resolveMaterial, m_cfg.resolvePassive, nullptr,
state.navigation.targetSurface);
navOpts.overstepLimit = stepper.overstepLimit(state.stepping);
double opening_angle = 0;

Expand Down Expand Up @@ -1016,7 +1020,8 @@ class Navigator {
/// get the target volume from the intersection
auto tPosition = targetIntersection.intersection.position;
state.navigation.targetVolume =
trackingGeometry->lowestTrackingVolume(state.geoContext, tPosition);
m_cfg.trackingGeometry->lowestTrackingVolume(state.geoContext,
tPosition);
state.navigation.targetLayer =
state.navigation.targetVolume
? state.navigation.targetVolume->associatedLayer(
Expand Down Expand Up @@ -1054,8 +1059,9 @@ class Navigator {
auto startSurface = onStart ? state.navigation.startSurface : layerSurface;
// Use navigation parameters and NavigationOptions
NavigationOptions<Surface> navOpts(
state.stepping.navDir, true, resolveSensitive, resolveMaterial,
resolvePassive, startSurface, state.navigation.targetSurface);
state.stepping.navDir, true, m_cfg.resolveSensitive,
m_cfg.resolveMaterial, m_cfg.resolvePassive, startSurface,
state.navigation.targetSurface);

std::vector<GeometryIdentifier> externalSurfaces;
if (!state.navigation.externalSurfaces.empty()) {
Expand Down Expand Up @@ -1134,9 +1140,9 @@ class Navigator {
: nullptr;
// Create the navigation options
// - and get the compatible layers, start layer will be excluded
NavigationOptions<Layer> navOpts(state.stepping.navDir, true,
resolveSensitive, resolveMaterial,
resolvePassive, startLayer, nullptr);
NavigationOptions<Layer> navOpts(
state.stepping.navDir, true, m_cfg.resolveSensitive,
m_cfg.resolveMaterial, m_cfg.resolvePassive, startLayer, nullptr);
// Set also the target surface
navOpts.targetSurface = state.navigation.targetSurface;
navOpts.pathLimit = state.stepping.stepSize.value(ConstrainedStep::aborter);
Expand Down Expand Up @@ -1203,11 +1209,12 @@ class Navigator {
const auto& logger = state.options.logger;

// Void behavior in case no tracking geometry is present
if (!trackingGeometry) {
if (!m_cfg.trackingGeometry) {
return true;
}
// turn the navigator into void when you are intructed to do nothing
if (!resolveSensitive && !resolveMaterial && !resolvePassive) {
if (!m_cfg.resolveSensitive && !m_cfg.resolveMaterial &&
!m_cfg.resolvePassive) {
return true;
}

Expand Down Expand Up @@ -1246,6 +1253,8 @@ class Navigator {
: "No Volume") +
" | ";
}

Config m_cfg;
};

} // namespace Acts
5 changes: 3 additions & 2 deletions Examples/Algorithms/Fatras/src/FatrasAlgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,11 @@ struct FatrasAlgorithmSimulationT final
: simulation(
ChargedSimulation(
ChargedPropagator(ChargedStepper(cfg.magneticField),
cfg.trackingGeometry),
Acts::Navigator{{cfg.trackingGeometry}}),
Acts::getDefaultLogger("Simulation", lvl)),
NeutralSimulation(
NeutralPropagator(NeutralStepper(), cfg.trackingGeometry),
NeutralPropagator(NeutralStepper(),
Acts::Navigator{{cfg.trackingGeometry}}),
Acts::getDefaultLogger("Simulation", lvl))) {
using namespace ActsFatras;
using namespace ActsFatras::detail;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ ActsExamples::TrackFindingAlgorithm::makeTrackFinderFunction(
std::shared_ptr<const Acts::TrackingGeometry> trackingGeometry,
std::shared_ptr<const Acts::MagneticFieldProvider> magneticField) {
Stepper stepper(std::move(magneticField));
Navigator navigator(trackingGeometry);
navigator.resolvePassive = false;
navigator.resolveMaterial = true;
navigator.resolveSensitive = true;
Navigator::Config cfg{trackingGeometry};
cfg.resolvePassive = false;
cfg.resolveMaterial = true;
cfg.resolveSensitive = true;
Navigator navigator(cfg);
Propagator propagator(std::move(stepper), std::move(navigator));
CKF trackFinder(std::move(propagator));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,11 @@ ActsExamples::TrackFittingAlgorithm::makeTrackFitterFunction(
std::shared_ptr<const Acts::TrackingGeometry> trackingGeometry,
std::shared_ptr<const Acts::MagneticFieldProvider> magneticField) {
Stepper stepper(std::move(magneticField));
Acts::Navigator navigator(trackingGeometry);
navigator.resolvePassive = false;
navigator.resolveMaterial = true;
navigator.resolveSensitive = true;
Acts::Navigator::Config cfg{trackingGeometry};
cfg.resolvePassive = false;
cfg.resolveMaterial = true;
cfg.resolveSensitive = true;
Acts::Navigator navigator(cfg);
Propagator propagator(std::move(stepper), std::move(navigator));
Fitter trackFitter(std::move(propagator));

Expand Down
7 changes: 2 additions & 5 deletions Examples/Run/Common/src/MaterialMappingBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,7 @@ int materialMappingExample(int argc, char* argv[],
ActsExamples::MaterialMapping::Config mmAlgConfig(geoContext, mfContext);
if (mapSurface) {
// Get a Navigator
Acts::Navigator navigator(tGeometry);
navigator.resolveSensitive = true;
navigator.resolveMaterial = true;
navigator.resolvePassive = true;
Acts::Navigator navigator({tGeometry, true, true, true});
// Make stepper and propagator
SlStepper stepper;
Propagator propagator(std::move(stepper), std::move(navigator));
Expand All @@ -134,7 +131,7 @@ int materialMappingExample(int argc, char* argv[],
}
if (mapVolume) {
// Get a Navigator
Acts::Navigator navigator(tGeometry);
Acts::Navigator navigator({tGeometry});
// Make stepper and propagator
SlStepper stepper;
Propagator propagator(std::move(stepper), std::move(navigator));
Expand Down
12 changes: 7 additions & 5 deletions Examples/Run/Common/src/MaterialValidationBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ ActsExamples::ProcessCode setupPropagation(
auto logLevel = ActsExamples::Options::readLogLevel(vm);

// Get a Navigator
Acts::Navigator navigator(tGeometry);
navigator.resolvePassive = true;
navigator.resolveMaterial = true;
navigator.resolveSensitive = true;
Acts::Navigator::Config cfg;
cfg.trackingGeometry = tGeometry;
cfg.resolvePassive = true;
cfg.resolveMaterial = true;
cfg.resolveSensitive = true;
Acts::Navigator navigator(cfg);

// Resolve the bfield map template and create the propgator
using Stepper = Acts::EigenStepper<
Expand Down Expand Up @@ -98,7 +100,7 @@ ActsExamples::ProcessCode setupStraightLinePropagation(
auto logLevel = ActsExamples::Options::readLogLevel(vm);

// Get a Navigator
Acts::Navigator navigator(tGeometry);
Acts::Navigator navigator({tGeometry});

// Straight line stepper
using SlStepper = Acts::StraightLineStepper;
Expand Down
10 changes: 5 additions & 5 deletions Examples/Run/Common/src/PropagationExampleBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ int propagationExample(int argc, char* argv[],
auto setupPropagator = [&](auto&& stepper) {
using Stepper = std::decay_t<decltype(stepper)>;
using Propagator = Acts::Propagator<Stepper, Acts::Navigator>;
Acts::Navigator navigator(tGeometry);
navigator.resolveMaterial = vm["prop-resolve-material"].template as<bool>();
navigator.resolvePassive = vm["prop-resolve-passive"].template as<bool>();
navigator.resolveSensitive =
vm["prop-resolve-sensitive"].template as<bool>();
Acts::Navigator::Config navCfg{tGeometry};
navCfg.resolveMaterial = vm["prop-resolve-material"].template as<bool>();
navCfg.resolvePassive = vm["prop-resolve-passive"].template as<bool>();
navCfg.resolveSensitive = vm["prop-resolve-sensitive"].template as<bool>();
Acts::Navigator navigator(navCfg);

Propagator propagator(std::move(stepper), std::move(navigator));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,16 @@ inline Propagator makePropagator(double bz) {

auto magField = std::make_shared<MagneticField>(Acts::Vector3(0.0, 0.0, bz));
Stepper stepper(std::move(magField));
return Propagator(std::move(stepper), Acts::Navigator(makeDetector()));
return Propagator(std::move(stepper), Acts::Navigator({makeDetector()}));
}

inline RiddersPropagator makeRiddersPropagator(double bz) {
using namespace Acts;

auto magField = std::make_shared<MagneticField>(Acts::Vector3(0.0, 0.0, bz));
Stepper stepper(std::move(magField));
return RiddersPropagator(std::move(stepper), Acts::Navigator(makeDetector()));
return RiddersPropagator(std::move(stepper),
Acts::Navigator({makeDetector()}));
}

} // namespace
Expand Down
2 changes: 1 addition & 1 deletion Tests/IntegrationTests/Fatras/FatrasSimulationTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ BOOST_DATA_TEST_CASE(FatrasSimulation, dataset, pdg, phi, eta, p,
auto trackingGeometry = geoBuilder();

// construct the propagators
Navigator navigator(trackingGeometry);
Navigator navigator({trackingGeometry});
ChargedStepper chargedStepper(
std::make_shared<Acts::ConstantBField>(Acts::Vector3{0, 0, 1_T}));
ChargedPropagator chargedPropagator(std::move(chargedStepper), navigator);
Expand Down
5 changes: 3 additions & 2 deletions Tests/IntegrationTests/PropagationDenseConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,16 @@ inline Propagator makePropagator(double bz) {

auto magField = std::make_shared<MagneticField>(Acts::Vector3(0.0, 0.0, bz));
Stepper stepper(std::move(magField));
return Propagator(std::move(stepper), Acts::Navigator(makeDetector()));
return Propagator(std::move(stepper), Acts::Navigator{{makeDetector()}});
}

inline RiddersPropagator makeRiddersPropagator(double bz) {
using namespace Acts;

auto magField = std::make_shared<MagneticField>(Acts::Vector3(0.0, 0.0, bz));
Stepper stepper(std::move(magField));
return RiddersPropagator(std::move(stepper), Acts::Navigator(makeDetector()));
return RiddersPropagator(std::move(stepper),
Acts::Navigator{{makeDetector()}});
}

} // namespace
Expand Down
2 changes: 1 addition & 1 deletion Tests/UnitTests/Core/Geometry/BVHDataTestCase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ BOOST_DATA_TEST_CASE(
using PropagatorType = Propagator<Stepper, Navigator>;

Stepper stepper{};
Navigator navigator(tg);
Navigator navigator({tg});
PropagatorType propagator(std::move(stepper), navigator);

using ActionList = Acts::ActionList<SteppingLogger>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ namespace Test {
/// Test the filling and conversion
BOOST_AUTO_TEST_CASE(SurfaceMaterialMapper_tests) {
/// We need a Navigator, Stepper to build a Propagator
Navigator navigator(tGeometry);
Navigator navigator({tGeometry});
StraightLineStepper stepper;
SurfaceMaterialMapper::StraightLinePropagator propagator(
std::move(stepper), std::move(navigator));
Expand Down
6 changes: 4 additions & 2 deletions Tests/UnitTests/Core/Material/VolumeMaterialMapperTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ BOOST_AUTO_TEST_CASE(SurfaceMaterialMapper_tests) {
std::shared_ptr<const TrackingGeometry> tGeometry = tgb.trackingGeometry(gc);

/// We need a Navigator, Stepper to build a Propagator
Navigator navigator(tGeometry);
Navigator navigator({tGeometry});
StraightLineStepper stepper;
VolumeMaterialMapper::StraightLinePropagator propagator(std::move(stepper),
std::move(navigator));
Expand Down Expand Up @@ -231,7 +231,9 @@ BOOST_AUTO_TEST_CASE(VolumeMaterialMapper_comparison_tests) {

// Construct a simple propagation through the detector
StraightLineStepper sls;
Navigator nav(std::move(detector));
Navigator::Config navCfg;
navCfg.trackingGeometry = std::move(detector);
Navigator nav(navCfg);
Propagator<StraightLineStepper, Navigator> prop(sls, nav);

// Set some start parameters
Expand Down
2 changes: 1 addition & 1 deletion Tests/UnitTests/Core/Propagator/DirectNavigatorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ CylindricalTrackingGeometry cGeometry(tgContext);
auto tGeometry = cGeometry();

// Create a navigator for this tracking geometry
Navigator navigator(tGeometry);
Navigator navigator({tGeometry});
DirectNavigator dnavigator;

using BField = ConstantBField;
Expand Down
2 changes: 1 addition & 1 deletion Tests/UnitTests/Core/Propagator/ExtrapolatorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ CylindricalTrackingGeometry cGeometry(tgContext);
auto tGeometry = cGeometry();

// Get the navigator and provide the TrackingGeometry
Navigator navigator(tGeometry);
Navigator navigator({tGeometry});

using BFieldType = ConstantBField;
using EigenStepperType = EigenStepper<>;
Expand Down
9 changes: 5 additions & 4 deletions Tests/UnitTests/Core/Propagator/KalmanExtrapolatorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,11 @@ BOOST_AUTO_TEST_CASE(kalman_extrapolator) {
auto detector = cGeometry();

// The Navigator through the detector geometry
Navigator navigator(detector);
navigator.resolvePassive = false;
navigator.resolveMaterial = true;
navigator.resolveSensitive = true;
Navigator::Config cfg{detector};
cfg.resolvePassive = false;
cfg.resolveMaterial = true;
cfg.resolveSensitive = true;
Navigator navigator(cfg);

// Configure propagation with deactivated B-field
auto bField = std::make_shared<ConstantBField>(Vector3(0., 0., 0.));
Expand Down
4 changes: 2 additions & 2 deletions Tests/UnitTests/Core/Propagator/MaterialCollectionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ CylindricalTrackingGeometry cGeometry(tgContext);
auto tGeometry = cGeometry();

// create a navigator for this tracking geometry
Navigator navigatorES(tGeometry);
Navigator navigatorSL(tGeometry);
Navigator navigatorES({tGeometry});
Navigator navigatorSL({tGeometry});

using BField = ConstantBField;
using EigenStepper = Acts::EigenStepper<>;
Expand Down

0 comments on commit 641e00c

Please sign in to comment.