Skip to content

Commit

Permalink
fix: TryAllNavigationPolicy config exposed to python (acts-project#3822)
Browse files Browse the repository at this point in the history
I previously neglected to include the overload with the configuration in the python bindings. This PR does that and also includes test coverage for this behavior.

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

- **New Features**
	- Introduced a new method to the navigation policy factory for adding `TryAllNavigationPolicy` with configuration parameters.
	- Added Python bindings for the `TryAllNavigationPolicy::Config` class.

- **Bug Fixes**
	- Adjusted constructor parameter order for `TryAllNavigationPolicy`, enhancing clarity and usability.

- **Tests**
	- Added a new test function to validate the addition of `TryAllNavigationPolicy` with specific configuration options.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
paulgessinger authored and Rosie-Hasan committed Nov 13, 2024
1 parent 5beb4d2 commit f335b09
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 8 deletions.
7 changes: 4 additions & 3 deletions Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ class TryAllNavigationPolicy final : public INavigationPolicy {
};

/// Constructor from a volume
/// @param config The configuration for the policy
/// @param gctx is the geometry context
/// @param volume is the volume to navigate
/// @param logger is the logger
TryAllNavigationPolicy(const Config& config, const GeometryContext& gctx,
const TrackingVolume& volume, const Logger& logger);
/// @param config The configuration for the policy
TryAllNavigationPolicy(const GeometryContext& gctx,
const TrackingVolume& volume, const Logger& logger,
const Config& config);

/// Constructor from a volume
/// @param gctx is the geometry context
Expand Down
8 changes: 4 additions & 4 deletions Core/src/Navigation/TryAllNavigationPolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@

namespace Acts {

TryAllNavigationPolicy::TryAllNavigationPolicy(const Config& config,
const GeometryContext& /*gctx*/,
TryAllNavigationPolicy::TryAllNavigationPolicy(const GeometryContext& /*gctx*/,
const TrackingVolume& volume,
const Logger& logger)
const Logger& logger,
const Config& config)
: m_cfg{config}, m_volume(&volume) {
assert(m_volume != nullptr);
ACTS_VERBOSE("TryAllNavigationPolicy created for volume "
Expand All @@ -26,7 +26,7 @@ TryAllNavigationPolicy::TryAllNavigationPolicy(const Config& config,
TryAllNavigationPolicy::TryAllNavigationPolicy(const GeometryContext& gctx,
const TrackingVolume& volume,
const Logger& logger)
: TryAllNavigationPolicy({}, gctx, volume, logger) {}
: TryAllNavigationPolicy(gctx, volume, logger, {}) {}

void TryAllNavigationPolicy::initializeCandidates(
const NavigationArguments& args, AppendOnlyNavigationStream& stream,
Expand Down
28 changes: 27 additions & 1 deletion Examples/Python/src/Navigation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ struct AnyNavigationPolicyFactory : public Acts::NavigationPolicyFactory {
virtual std::unique_ptr<AnyNavigationPolicyFactory> add(
TypeTag<SurfaceArrayNavigationPolicy> /*type*/,
SurfaceArrayNavigationPolicy::Config config) = 0;

virtual std::unique_ptr<AnyNavigationPolicyFactory> add(
TypeTag<TryAllNavigationPolicy> /*type*/,
TryAllNavigationPolicy::Config config) = 0;
};

template <typename Factory = detail::NavigationPolicyFactoryImpl<>,
Expand All @@ -61,6 +65,12 @@ struct NavigationPolicyFactoryT : public AnyNavigationPolicyFactory {
return add<SurfaceArrayNavigationPolicy>(std::move(config));
}

std::unique_ptr<AnyNavigationPolicyFactory> add(
TypeTag<TryAllNavigationPolicy> /*type*/,
TryAllNavigationPolicy::Config config) override {
return add<TryAllNavigationPolicy>(config);
}

std::unique_ptr<INavigationPolicy> build(
const GeometryContext& gctx, const TrackingVolume& volume,
const Logger& logger) const override {
Expand Down Expand Up @@ -108,6 +118,12 @@ class NavigationPolicyFactory : public Acts::NavigationPolicyFactory {
return *this;
}

NavigationPolicyFactory& addTryAll(
const py::object& /*cls*/, const TryAllNavigationPolicy::Config& config) {
m_impl = m_impl->add(Type<TryAllNavigationPolicy>, config);
return *this;
}

std::unique_ptr<INavigationPolicy> build(
const GeometryContext& gctx, const TrackingVolume& volume,
const Logger& logger) const override {
Expand Down Expand Up @@ -153,7 +169,16 @@ void addNavigation(Context& ctx) {
std::shared_ptr<Acts::NavigationPolicyFactory>>(
m, "_NavigationPolicyFactory");

py::class_<TryAllNavigationPolicy>(m, "TryAllNavigationPolicy");
{
auto tryAll =
py::class_<TryAllNavigationPolicy>(m, "TryAllNavigationPolicy");
using Config = TryAllNavigationPolicy::Config;
auto c = py::class_<Config>(tryAll, "Config").def(py::init<>());
ACTS_PYTHON_STRUCT_BEGIN(c, Config);
ACTS_PYTHON_MEMBER(portals);
ACTS_PYTHON_MEMBER(sensitives);
ACTS_PYTHON_STRUCT_END();
}

py::class_<NavigationPolicyFactory, Acts::NavigationPolicyFactory,
std::shared_ptr<NavigationPolicyFactory>>(
Expand All @@ -162,6 +187,7 @@ void addNavigation(Context& ctx) {
.def_static("make", []() { return NavigationPolicyFactory{}; })
.def("add", &NavigationPolicyFactory::addNoArguments)
.def("add", &NavigationPolicyFactory::addSurfaceArray)
.def("add", &NavigationPolicyFactory::addTryAll)
.def("_buildTest", [](NavigationPolicyFactory& self) {
auto vol1 = std::make_shared<TrackingVolume>(
Transform3::Identity(),
Expand Down
6 changes: 6 additions & 0 deletions Examples/Python/tests/test_navigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,9 @@ def test_navigation_policy_factory_add_multiple():
.add(acts.TryAllNavigationPolicy)
.add(acts.TryAllNavigationPolicy)
)


def test_try_all_arguments():
acts.NavigationPolicyFactory.make().add(
acts.TryAllNavigationPolicy, acts.TryAllNavigationPolicy.Config(sensitives=True)
)

0 comments on commit f335b09

Please sign in to comment.