Skip to content

Commit

Permalink
refactor: Python binding bits and pieces (#3717)
Browse files Browse the repository at this point in the history
This is a collection of python bindings bits that have accumulated.

Related #3502.

---

This pull request includes several updates to the `Acts` geometry and Python binding code. The most important changes involve the introduction of new enums and the addition of Python bindings for new geometry classes and methods.

### Geometry and Enum Updates:

* Added `Face` enum to `CylinderVolumeBounds` to describe possible faces of a cylinder volume. (`Core/include/Acts/Geometry/CylinderVolumeBounds.hpp`)
* Refactored `PortalShellBase` to use the `Face` enum from `CylinderVolumeBounds` instead of defining its own. (`Core/include/Acts/Geometry/PortalShell.hpp`)

### Python Bindings Enhancements:

* Added new binning values to the `addBinning` function in `Base.cpp`. (`Examples/Python/src/Base.cpp`)
* Extended `addGeometry` function to include new methods and enums for `CylinderVolumeBounds` and `ExtentEnvelope`. (`Examples/Python/src/Geometry.cpp`) [[1]](diffhunk://#diff-a103b5682fb7c6e7ea58777983e4381b62783b2facd3e367e03ff0a7aa49816dL181-R213) [[2]](diffhunk://#diff-a103b5682fb7c6e7ea58777983e4381b62783b2facd3e367e03ff0a7aa49816dL212-R303)
* Introduced Python bindings for `CylinderVolumeStack` and its enums `AttachmentStrategy` and `ResizeStrategy`. (`Examples/Python/src/Geometry.cpp`)

### Code Cleanup:

* Removed redundant includes and updated include paths to reflect new dependencies. (`Core/include/Acts/Geometry/PortalShell.hpp`, `Examples/Python/src/Geometry.cpp`) [[1]](diffhunk://#diff-80595cf723b4c4b0a2cf3de28ac0da38793f2955a2e0ce1dc4fd87381fac79aeL11-R11) [[2]](diffhunk://#diff-a103b5682fb7c6e7ea58777983e4381b62783b2facd3e367e03ff0a7aa49816dR40) [[3]](diffhunk://#diff-a103b5682fb7c6e7ea58777983e4381b62783b2facd3e367e03ff0a7aa49816dR51)

These changes improve the modularity and functionality of the geometry components and enhance the Python interface for better usability.
  • Loading branch information
paulgessinger authored Oct 13, 2024
1 parent 010e397 commit 6709342
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 53 deletions.
13 changes: 13 additions & 0 deletions Core/include/Acts/Geometry/CylinderVolumeBounds.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#pragma once

#include "Acts/Definitions/Algebra.hpp"
#include "Acts/Geometry/BoundarySurfaceFace.hpp"
#include "Acts/Geometry/Volume.hpp"
#include "Acts/Geometry/VolumeBounds.hpp"
#include "Acts/Utilities/BinningType.hpp"
Expand Down Expand Up @@ -80,6 +81,18 @@ class CylinderVolumeBounds : public VolumeBounds {
eSize
};

/// Enum describing the possible faces of a cylinder volume
/// @note These values are synchronized with the BoundarySurfaceFace enum.
/// Once Gen1 is removed, this can be changed.
enum class Face : unsigned int {
PositiveDisc = BoundarySurfaceFace::positiveFaceXY,
NegativeDisc = BoundarySurfaceFace::negativeFaceXY,
OuterCylinder = BoundarySurfaceFace::tubeOuterCover,
InnerCylinder = BoundarySurfaceFace::tubeInnerCover,
NegativePhiPlane = BoundarySurfaceFace::tubeSectorNegativePhi,
PositivePhiPlane = BoundarySurfaceFace::tubeSectorPositivePhi
};

CylinderVolumeBounds() = delete;

/// Constructor
Expand Down
18 changes: 4 additions & 14 deletions Core/include/Acts/Geometry/PortalShell.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

#pragma once

#include "Acts/Geometry/BoundarySurfaceFace.hpp"
#include "Acts/Geometry/CylinderVolumeBounds.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/Logger.hpp"

Expand Down Expand Up @@ -68,19 +68,9 @@ class PortalShellBase {
/// volumes
class CylinderPortalShell : public PortalShellBase {
public:
/// Enum describing the possible faces of a cylinder portal shell
/// @note These values are synchronized with the BoundarySurfaceFace enum.
/// Once Gen1 is removed, this can be changed.
enum class Face : unsigned int {
PositiveDisc = BoundarySurfaceFace::positiveFaceXY,
NegativeDisc = BoundarySurfaceFace::negativeFaceXY,
OuterCylinder = BoundarySurfaceFace::tubeOuterCover,
InnerCylinder = BoundarySurfaceFace::tubeInnerCover,
NegativePhiPlane = BoundarySurfaceFace::tubeSectorNegativePhi,
PositivePhiPlane = BoundarySurfaceFace::tubeSectorPositivePhi
};

using enum Face;
using Face = CylinderVolumeBounds::Face;

using enum CylinderVolumeBounds::Face;

/// Retrieve the portal associated to the given face. Can be nullptr if unset.
/// @param face The face to retrieve the portal for
Expand Down
2 changes: 1 addition & 1 deletion Core/src/Geometry/PortalShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ std::string CylinderStackPortalShell::label() const {

std::ostream& operator<<(std::ostream& os, CylinderPortalShell::Face face) {
switch (face) {
using enum CylinderPortalShell::Face;
using enum CylinderVolumeBounds::Face;
case PositiveDisc:
return os << "PositiveDisc";
case NegativeDisc:
Expand Down
12 changes: 8 additions & 4 deletions Examples/Python/src/Base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,12 +364,16 @@ void addBinning(Context& ctx) {
.value("binY", Acts::BinningValue::binY)
.value("binZ", Acts::BinningValue::binZ)
.value("binR", Acts::BinningValue::binR)
.value("binPhi", Acts::BinningValue::binPhi);
.value("binPhi", Acts::BinningValue::binPhi)
.value("binRPhi", Acts::BinningValue::binRPhi)
.value("binH", Acts::BinningValue::binH)
.value("binEta", Acts::BinningValue::binEta)
.value("binMag", Acts::BinningValue::binMag);

auto boundaryType = py::enum_<Acts::AxisBoundaryType>(m, "AxisBoundaryType")
.value("bound", Acts::AxisBoundaryType::Bound)
.value("closed", Acts::AxisBoundaryType::Closed)
.value("open", Acts::AxisBoundaryType::Open);
.value("Bound", Acts::AxisBoundaryType::Bound)
.value("Closed", Acts::AxisBoundaryType::Closed)
.value("Open", Acts::AxisBoundaryType::Open);

auto axisType = py::enum_<Acts::AxisType>(m, "AxisType")
.value("equidistant", Acts::AxisType::Equidistant)
Expand Down
133 changes: 106 additions & 27 deletions Examples/Python/src/Geometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "Acts/Detector/interface/IInternalStructureBuilder.hpp"
#include "Acts/Detector/interface/IRootVolumeFinderBuilder.hpp"
#include "Acts/Geometry/CylinderVolumeBounds.hpp"
#include "Acts/Geometry/CylinderVolumeStack.hpp"
#include "Acts/Geometry/Extent.hpp"
#include "Acts/Geometry/GeometryContext.hpp"
#include "Acts/Geometry/GeometryHierarchyMap.hpp"
Expand All @@ -36,6 +37,7 @@
#include "Acts/Plugins/Python/Utilities.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Surfaces/SurfaceArray.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/Helpers.hpp"
#include "Acts/Utilities/RangeXD.hpp"
#include "Acts/Visualization/ViewConfig.hpp"
Expand All @@ -46,6 +48,7 @@
#include <unordered_map>
#include <vector>

#include <boost/algorithm/string/join.hpp>
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

Expand Down Expand Up @@ -106,7 +109,12 @@ void addGeometry(Context& ctx) {
.def("approach", &Acts::GeometryIdentifier::approach)
.def("sensitive", &Acts::GeometryIdentifier::sensitive)
.def("extra", &Acts::GeometryIdentifier::extra)
.def("value", &Acts::GeometryIdentifier::value);
.def("value", &Acts::GeometryIdentifier::value)
.def("__str__", [](const Acts::GeometryIdentifier& self) {
std::stringstream ss;
ss << self;
return ss.str();
});
}

{
Expand Down Expand Up @@ -178,15 +186,31 @@ void addGeometry(Context& ctx) {

{
py::class_<Acts::VolumeBounds, std::shared_ptr<Acts::VolumeBounds>>(
m, "VolumeBounds");

py::class_<Acts::CylinderVolumeBounds,
std::shared_ptr<Acts::CylinderVolumeBounds>, Acts::VolumeBounds>(
m, "CylinderVolumeBounds")
.def(py::init<ActsScalar, ActsScalar, ActsScalar, ActsScalar,
ActsScalar, ActsScalar, ActsScalar>(),
"rmin"_a, "rmax"_a, "halfz"_a, "halfphi"_a = M_PI, "avgphi"_a = 0.,
"bevelMinZ"_a = 0., "bevelMaxZ"_a = 0.);
m, "VolumeBounds")
.def("type", &Acts::VolumeBounds::type)
.def("__str__", [](const Acts::VolumeBounds& self) {
std::stringstream ss;
ss << self;
return ss.str();
});

auto cvb =
py::class_<Acts::CylinderVolumeBounds,
std::shared_ptr<Acts::CylinderVolumeBounds>,
Acts::VolumeBounds>(m, "CylinderVolumeBounds")
.def(py::init<ActsScalar, ActsScalar, ActsScalar, ActsScalar,
ActsScalar, ActsScalar, ActsScalar>(),
"rmin"_a, "rmax"_a, "halfz"_a, "halfphi"_a = M_PI,
"avgphi"_a = 0., "bevelMinZ"_a = 0., "bevelMaxZ"_a = 0.);

py::enum_<CylinderVolumeBounds::Face>(cvb, "Face")
.value("PositiveDisc", CylinderVolumeBounds::Face::PositiveDisc)
.value("NegativeDisc", CylinderVolumeBounds::Face::NegativeDisc)
.value("OuterCylinder", CylinderVolumeBounds::Face::OuterCylinder)
.value("InnerCylinder", CylinderVolumeBounds::Face::InnerCylinder)
.value("NegativePhiPlane", CylinderVolumeBounds::Face::NegativePhiPlane)
.value("PositivePhiPlane",
CylinderVolumeBounds::Face::PositivePhiPlane);
}

{
Expand All @@ -209,22 +233,72 @@ void addGeometry(Context& ctx) {
}));
}

py::class_<ExtentEnvelope>(m, "ExtentEnvelope")
.def(py::init<>())
.def(py::init<const Envelope&>())
.def(py::init([](Envelope x, Envelope y, Envelope z, Envelope r,
Envelope phi, Envelope rPhi, Envelope h, Envelope eta,
Envelope mag) {
return ExtentEnvelope({.x = x,
.y = y,
.z = z,
.r = r,
.phi = phi,
.rPhi = rPhi,
.h = h,
.eta = eta,
.mag = mag});
}),
py::arg("x") = zeroEnvelope, py::arg("y") = zeroEnvelope,
py::arg("z") = zeroEnvelope, py::arg("r") = zeroEnvelope,
py::arg("phi") = zeroEnvelope, py::arg("rPhi") = zeroEnvelope,
py::arg("h") = zeroEnvelope, py::arg("eta") = zeroEnvelope,
py::arg("mag") = zeroEnvelope)
.def_static("Zero", &ExtentEnvelope::Zero)
.def("__getitem__", [](ExtentEnvelope& self,
BinningValue bValue) { return self[bValue]; })
.def("__setitem__", [](ExtentEnvelope& self, BinningValue bValue,
const Envelope& value) { self[bValue] = value; })
.def("__str__", [](const ExtentEnvelope& self) {
std::array<std::string, numBinningValues()> values;

std::stringstream ss;
for (BinningValue val : allBinningValues()) {
ss << val << "=(" << self[val][0] << ", " << self[val][1] << ")";
values.at(toUnderlying(val)) = ss.str();
ss.str("");
}

ss.str("");
ss << "ExtentEnvelope(";
ss << boost::algorithm::join(values, ", ");
ss << ")";
return ss.str();
});

py::class_<Extent>(m, "Extent")
.def(py::init<const ExtentEnvelope&>(),
py::arg("envelope") = ExtentEnvelope::Zero())
.def("range",
[](const Acts::Extent& self,
Acts::BinningValue bval) -> std::array<ActsScalar, 2> {
return {self.min(bval), self.max(bval)};
})
.def("__str__", &Extent::toString);

{
py::class_<Acts::Extent>(m, "Extent")
.def(py::init(
[](const std::vector<std::tuple<Acts::BinningValue,
std::array<Acts::ActsScalar, 2u>>>&
franges) {
Acts::Extent extent;
for (const auto& [bval, frange] : franges) {
extent.set(bval, frange[0], frange[1]);
}
return extent;
}))
.def("range", [](const Acts::Extent& self, Acts::BinningValue bval) {
return std::array<Acts::ActsScalar, 2u>{self.min(bval),
self.max(bval)};
});
auto cylStack = py::class_<CylinderVolumeStack>(m, "CylinderVolumeStack");

py::enum_<CylinderVolumeStack::AttachmentStrategy>(cylStack,
"AttachmentStrategy")
.value("Gap", CylinderVolumeStack::AttachmentStrategy::Gap)
.value("First", CylinderVolumeStack::AttachmentStrategy::First)
.value("Second", CylinderVolumeStack::AttachmentStrategy::Second)
.value("Midpoint", CylinderVolumeStack::AttachmentStrategy::Midpoint);

py::enum_<CylinderVolumeStack::ResizeStrategy>(cylStack, "ResizeStrategy")
.value("Gap", CylinderVolumeStack::ResizeStrategy::Gap)
.value("Expand", CylinderVolumeStack::ResizeStrategy::Expand);
}
}

Expand Down Expand Up @@ -307,10 +381,15 @@ void addExperimentalGeometry(Context& ctx) {
// Be able to construct a proto binning
py::class_<ProtoBinning>(m, "ProtoBinning")
.def(py::init<Acts::BinningValue, Acts::AxisBoundaryType,
const std::vector<Acts::ActsScalar>&, std::size_t>())
const std::vector<Acts::ActsScalar>&, std::size_t>(),
"bValue"_a, "bType"_a, "e"_a, "exp"_a = 0u)
.def(py::init<Acts::BinningValue, Acts::AxisBoundaryType,
Acts::ActsScalar, Acts::ActsScalar, std::size_t,
std::size_t>());
std::size_t>(),
"bValue"_a, "bType"_a, "minE"_a, "maxE"_a, "nbins"_a, "exp"_a = 0u)
.def(py::init<Acts::BinningValue, Acts::AxisBoundaryType, std::size_t,
std::size_t>(),
"bValue"_a, "bType"_a, "nbins"_a, "exp"_a = 0u);
}

{
Expand Down
14 changes: 7 additions & 7 deletions Tests/UnitTests/Core/Geometry/PortalShellTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(ConstructionFromVolume) {
SingleCylinderPortalShell shell1{cyl1};
BOOST_CHECK_EQUAL(shell1.size(), 4);

using enum CylinderPortalShell::Face;
using enum CylinderVolumeBounds::Face;

const auto* pDisc = shell1.portal(PositiveDisc);
BOOST_REQUIRE_NE(pDisc, nullptr);
Expand Down Expand Up @@ -299,7 +299,7 @@ BOOST_AUTO_TEST_CASE(ConstructionFromVolume) {
// inner cylinder

BOOST_AUTO_TEST_CASE(PortalAssignment) {
using enum CylinderPortalShell::Face;
using enum CylinderVolumeBounds::Face;
TrackingVolume vol(
Transform3::Identity(),
std::make_shared<CylinderVolumeBounds>(30_mm, 100_mm, 100_mm));
Expand Down Expand Up @@ -350,7 +350,7 @@ BOOST_AUTO_TEST_CASE(PortalAssignment) {

BOOST_AUTO_TEST_SUITE(CylinderStack)
BOOST_AUTO_TEST_CASE(ZDirection) {
using enum CylinderPortalShell::Face;
using enum CylinderVolumeBounds::Face;
BOOST_TEST_CONTEXT("rMin>0") {
TrackingVolume vol1(
Transform3{Translation3{Vector3::UnitZ() * -100_mm}},
Expand Down Expand Up @@ -447,7 +447,7 @@ BOOST_AUTO_TEST_CASE(ZDirection) {
}

BOOST_AUTO_TEST_CASE(RDirection) {
using enum CylinderPortalShell::Face;
using enum CylinderVolumeBounds::Face;
BOOST_TEST_CONTEXT("rMin>0") {
TrackingVolume vol1(
Transform3::Identity(),
Expand Down Expand Up @@ -610,7 +610,7 @@ BOOST_AUTO_TEST_CASE(NestedStacks) {
gctx, {&stack, &shell3}, BinningValue::binZ, *logger};
BOOST_CHECK(stack2.isValid());

using enum CylinderPortalShell::Face;
using enum CylinderVolumeBounds::Face;

auto lookup = [](auto& shell, CylinderPortalShell::Face face,
Vector3 position,
Expand Down Expand Up @@ -726,7 +726,7 @@ BOOST_AUTO_TEST_CASE(ConnectOuter) {

SingleCylinderPortalShell shell{cyl1};

using enum CylinderPortalShell::Face;
using enum CylinderVolumeBounds::Face;
BOOST_CHECK_EQUAL(
shell.portal(OuterCylinder)->getLink(Direction::AlongNormal), nullptr);
BOOST_CHECK_EQUAL(
Expand All @@ -749,7 +749,7 @@ BOOST_AUTO_TEST_CASE(ConnectOuter) {
}

BOOST_AUTO_TEST_CASE(RegisterInto) {
using enum CylinderPortalShell::Face;
using enum CylinderVolumeBounds::Face;
TrackingVolume vol1(
Transform3::Identity(),
std::make_shared<CylinderVolumeBounds>(0_mm, 100_mm, 100_mm));
Expand Down

0 comments on commit 6709342

Please sign in to comment.