feat: Gen3 DD4hep construction by paulgessinger · Pull Request #5193 · acts-project/acts

paulgessinger · 2026-03-03T17:21:04Z

No description provided.

tmadlener · 2026-03-06T19:03:43Z

From my point of view, I am pretty happy with the state of this. I have exercised a few of the geometries we have in k4geo in key4hep/k4ActsTracking#36 and at least for me the still existing amount of boilerplate is acceptable (and probably also necessary) given the layouts of the tracking devices we have.

Core/include/Acts/Geometry/BlueprintBuilder.hpp

github-actions · 2026-03-27T18:47:41Z

📊: Physics performance monitoring for `70a3c8c`

Full contents

physmon summary

paulgessinger · 2026-03-31T12:28:26Z

It's finally green now @asalzburger @andiwand @tmadlener

tmadlener: Lifted from earlier work done on the old API design proposal

Do not prescribe CylinderContainers

Some helpers and at least some of the necessary disambiguation

…TGeo

…ackends

…ounts

Move ODD TGeo constants/layer binning logic into the DD4hep builder path and remove the generic TGeoBackend constant-provider plumbing.

sonarqubecloud · 2026-04-01T12:02:07Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
6.4% Duplication on New Code

See analysis details on SonarQube Cloud

andiwand

couple of minor comments and one thought on the general implementation

but overall OK from my side - approving

andiwand · 2026-04-07T08:00:06Z

Core/include/Acts/Geometry/detail/BlueprintBuilder_impl.hpp

+template <typename F, typename G, typename... Rest>
+auto compose(const F& f, const G& g, Rest... rest) {
+  auto fg = [=]<typename T>(T&& x) -> decltype(auto) {
+    return std::invoke(f, g(std::forward<T>(x)));
+  };
+  if constexpr (sizeof...(rest) == 0) {
+    return fg;
+  } else {
+    return compose(fg, rest...);
+  }
+}


nitpick: could be worth upstreaming into a utility

andiwand · 2026-04-07T08:02:16Z

Core/include/Acts/Geometry/detail/BlueprintBuilder_impl.hpp

+// ElementLayerAssembler
+template <detail::BlueprintBackend BackendT>
+ElementLayerAssembler<BackendT>::ElementLayerAssembler(const Builder& builder)
+    : m_builder{&builder} {}
+
+template <detail::BlueprintBackend BackendT>
+ElementLayerAssembler<BackendT>&& ElementLayerAssembler<BackendT>::setLayerType(
+    LayerType layerType) && {
+  m_layerType = layerType;
+  return std::move(*this);
+}


nitpick: maybe of these functions are one-liners - might be more convenient to put them in the "main" header file

andiwand · 2026-04-07T08:03:24Z

Core/include/Acts/Geometry/detail/BlueprintBuilder_impl.hpp

+    it->sensors.push_back(sensor);
+  }
+
+  for (auto& group : groups) {


Suggested change

for (auto& group : groups) {

for (const auto& group : groups) {

does this need to be mutable?

andiwand · 2026-04-07T08:08:32Z

blueprint_builder_api.md

should this go into doc somewhere or was this accidentally committed?

andiwand · 2026-04-07T08:11:01Z

Plugins/Root/include/ActsPlugins/Root/TGeoVolumeConverter.hpp

+/// Helper struct for converting supported ROOT `TGeoShape` instances into ACTS
+/// volumes.
+struct TGeoVolumeConverter {
+  /// Utility type; not instantiable.
+  TGeoVolumeConverter() = delete;


nitpick: struct vs namespace - do you think we will need some config or state for this in the future?

andiwand · 2026-04-07T08:44:23Z

Core/include/Acts/Geometry/BlueprintBuilder.hpp

+  /// @param builder The owning @ref BlueprintBuilder; must outlive this object.
+  explicit ElementLayerAssembler(const Builder& builder);
+
+  const Builder* m_builder;


Suggested change

const Builder* m_builder;

const Builder* m_builder = nullptr;

nitpick: since other variables are defaulted

andiwand · 2026-04-07T08:46:41Z

Core/include/Acts/Geometry/BlueprintBuilder.hpp

+      layerSpec.layerAxes = std::move(layerAxes);
+    };
+
+using LayerNodePtr = std::shared_ptr<Acts::Experimental::LayerBlueprintNode>;


Suggested change

using LayerNodePtr = std::shared_ptr<Acts::Experimental::LayerBlueprintNode>;

using LayerNodePtr = std::shared_ptr<LayerBlueprintNode>;

nitpick: I think the namespace can be collapsed. there are a couple more cases in this file

andiwand · 2026-04-07T08:46:58Z

Core/include/Acts/Geometry/BlueprintBuilder.hpp

+  /// @param builder The owning @ref BlueprintBuilder; must outlive this object.
+  explicit SensorLayerAssembler(const Builder& builder);
+
+  const Builder* m_builder;


Suggested change

const Builder* m_builder;

const Builder* m_builder{};

andiwand · 2026-04-07T08:49:34Z

Core/include/Acts/Geometry/BlueprintBuilder.hpp

+  /// @param builder The owning @ref BlueprintBuilder; must outlive this object.
+  explicit SensorLayer(const Builder& builder);
+
+  const Builder* m_builder;


Suggested change

const Builder* m_builder;

const Builder* m_builder{};

andiwand · 2026-04-07T09:00:18Z

Core/include/Acts/Geometry/BlueprintBuilder.hpp

+      { BackendT::kIdentifier } -> std::convertible_to<std::string_view>;
+      { backend.world() } -> std::same_as<typename BackendT::Element>;
+      { backend.nameOf(element) } -> std::same_as<std::string>;
+      {
+        backend.children(element)
+      } -> std::same_as<std::vector<typename BackendT::Element>>;
+      { backend.parent(element) } -> std::same_as<typename BackendT::Element>;
+      { backend.isSensitive(element) } -> std::same_as<bool>;
+      requires requires(const typename BackendT::Element& a,
+                        const typename BackendT::Element& b) {
+        { a == b } -> std::convertible_to<bool>;
+      };


Some thoughts after going through the code:

I wonder if alternatively the Element can be type-erased via std::any or even a simple element index. This would allow to convert the backend to a virtual interface or maybe remove the backend and let the user derive from BlueprintBuilder.

Another option would be to have a virtual interface for Element and talk to it during construction. But this would require multiple dynamic allocations to traverse the tree. But for construction not critical I guess.

I don't think there is a huge advantage in either direction, just some thoughts on relaxing the templating. At the end the deciding factor should be the interface, flexibility and maintainability. Here I cannot really say anything as I haven't used the interface. Maybe @noemina @tmadlener @asalzburger can say more

My 2 cents (from a user perspective for DD4hep). I am fairly happy with the current (templated) implementation. In the end what shows up in user code is ActsPlugins::DD4hep::BlueprintBuilder (and friends) and very few (if any) dedicated templates from the backend (see key4hep/k4ActsTracking#36). I think it would be almost transparent for us if this was switched to a pure virtual BlueprintBuilder, as long as the ActsPlugins::DD4hep::BlueprintBuilder survives with the same interface. The one thing where I think having dedicated element types is nice is for writing small utility functionality (e.g. for visitors) is rather straight forward at the moment because I can directly use the dd4hep types instead of, e.g. having to do an any_cast first.

github-actions bot added this to the next milestone Mar 3, 2026

github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins labels Mar 3, 2026

paulgessinger force-pushed the feat/dd4hep-gen3 branch from f4d3d88 to 8538d86 Compare March 5, 2026 16:45

tmadlener mentioned this pull request Mar 6, 2026

[WIP] Acts Geo Gen3 DD4hep conversion key4hep/k4ActsTracking#36

Open

2 tasks

tmadlener reviewed Mar 9, 2026

View reviewed changes

Core/include/Acts/Geometry/BlueprintBuilder.hpp Outdated Show resolved Hide resolved

paulgessinger force-pushed the feat/dd4hep-gen3 branch from b17a972 to 9acc9b5 Compare March 16, 2026 12:39

paulgessinger mentioned this pull request Mar 17, 2026

feat: dlopen-based geometry *module* support #5253

Merged

paulgessinger force-pushed the feat/dd4hep-gen3 branch 2 times, most recently from 65e8d40 to 3d94567 Compare March 27, 2026 14:37

paulgessinger marked this pull request as ready for review March 27, 2026 14:39

paulgessinger force-pushed the feat/dd4hep-gen3 branch from 95c300f to d98d492 Compare March 30, 2026 17:04

paulgessinger and others added 14 commits April 1, 2026 12:30

feat: First draft for partial DD4hep Gen3 support

0266680

feat: Test Gen3 DD4hep code on ODD

bcbaeee

fix: Add missing symbol

dd88203

fix: dd4hep -> DD4hep on disk

6435905

layer helper API and use it

0d6a86b

(sort of) auto detect layer transform if axes are specified

7bc1569

feat: Beampipe maker for blueprint builder

10532e0

rename addLayer -> makeLayer

8a154b6

add flag to LayerHelper whether empty is ok or not

66b4463

remove artifact

3d94b18

fix beampipe builder and make it build to r=0

9039e6d

fix: Check if cylinder conversion failed

d3f1438

Make the layer names use the full path name

377abc7

Use boost join

fef2118

Yufeng Wang and others added 27 commits April 1, 2026 12:47

layerType check

6e49a9e

tmadlener: Lifted from earlier work done on the old API design proposal

Make customizers more generic

b8fe33d

Do not prescribe CylinderContainers

Add first facilities for handling Plane Layers

9d7a1bf

Some helpers and at least some of the necessary disambiguation

Remove old code from original API design

9b3db5c

Remove some unnecessary / leftover changes

e0efd5b

blueprintbuilder impl tuning

5af76dd

move odd construction to free funcs in core

e3f155c

docs update

5e45a81

lint

8b9b9dd

feat(DD4hep/Root): add initial TGeo blueprint backend and ODD harness

30e5201

refactor(Root): make the TGeo blueprint backend lazy and path-derived

5732012

refactor(DD4hep/TGeo): move beampipe handling to DD4hep and simplify …

b8cdeb7

…TGeo

refactor(Blueprint): make barrel-endcap classification optional for b…

5c66f7c

…ackends

fix(DD4hep): use stable grouped layer names and align Gen3 geometry c…

03031a2

…ounts

fix(TGeo ODD): expose volume metadata in Python and match Gen3 counts

e8e53c1

dd4hep: make TGeo layer customization self-contained

52414be

Move ODD TGeo constants/layer binning logic into the DD4hep builder path and remove the generic TGeoBackend constant-provider plumbing.

prepare for tgeo backend

55b52c3

include tuning, namespaces

cf19998

drop tgeo related constants

20d245b

format

210338e

add doc comments

3c44a91

fix compiler warnings

d775c1d

another clang-tidy fix

4cd8386

fix test expectations

afc3d03

lint

b3006b8

clang-format is drunk

827f910

even more drunk?

70a3c8c

paulgessinger force-pushed the feat/dd4hep-gen3 branch from 46ce132 to 70a3c8c Compare April 1, 2026 10:47

andiwand approved these changes Apr 7, 2026

View reviewed changes

	for (auto& group : groups) {
	for (const auto& group : groups) {

	const Builder* m_builder;
	const Builder* m_builder = nullptr;

	using LayerNodePtr = std::shared_ptr<Acts::Experimental::LayerBlueprintNode>;
	using LayerNodePtr = std::shared_ptr<LayerBlueprintNode>;

Conversation

paulgessinger commented Mar 3, 2026

Uh oh!

tmadlener commented Mar 6, 2026

Uh oh!

Uh oh!

github-actions bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊: Physics performance monitoring for 70a3c8c

physmon summary

Uh oh!

paulgessinger commented Mar 31, 2026

Uh oh!

sonarqubecloud bot commented Apr 1, 2026

Quality Gate passed

Uh oh!

andiwand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Mar 27, 2026 •

edited

Loading

📊: Physics performance monitoring for `70a3c8c`