Skip to content

feat(material): Support stable keys for surface material assignments - #6109

Draft
paulgessinger wants to merge 2 commits into
acts-project:mainfrom
paulgessinger:codex/stable-material-keys
Draft

paulgessinger wants to merge 2 commits into
acts-project:mainfrom
paulgessinger:codex/stable-material-keys

Conversation

@paulgessinger

@paulgessinger paulgessinger commented Sep 17, 2026

Copy link
Copy Markdown
Member

Depends on #6123, which extracts the breaking TrackingGeometryMaterial pair-to-struct migration into a standalone API-only PR. This branch contains that base commit followed by the stable-key implementation; merge #6123 first. All callers use named map members, so the stable-key extension is additive relative to that API base.

Material maps currently bind assignments to geometry IDs, so rebuilding an equivalent geometry with different IDs can attach material to the wrong surfaces. Add optional stable string keys to the proto-material configureFace overloads and carry those identities through mapping, serialization, and loading. Existing calls without keys keep ID-based assignment.

  • Discover keys by downcasting proto materials; leave Surface and ISurfaceMaterial unchanged.
  • Validate duplicate keys and geometry IDs before accumulation, retaining the registry in the binned accumulator state with its implementation type in detail.
  • Apply assignments through format-independent TrackingGeometryMaterial::apply in Core. Resolve surface assignments before updating them. Reject keyed maps on Gen1 geometry, including the JSON decorator construction path.
  • Serialize keyed assignments in a plain KeyedSurfaces array without a nested version header or duplicated ID-map entries. Each entry contains its key, diagnostic geometry ID, and material payload.
  • Require a matching key for keyed targets, without numeric-ID fallback. Allow unused keys for combined maps and subset geometries.
  • Preserve original IDs and keys in merge-marker provenance. Retain the existing public marker API.
  • Expose the maps and geometry application in Python; document configuration with compiled snippets.

Loading replaces the placeholder and consumes its key. Rebuild the designators before applying another keyed map; retain TrackingGeometryMaterial to re-export keyed assignments. ROOT writers reject keyed assignments; JSON/CBOR preserve them.

Validation: the API-only base passed MaterialMapper, MaterialMapJsonConverter, MaterialJsonDocumentation, and RootMaterialMapIo independently. The restacked implementation passed TrackingGeometryMaterial, MaterialMapper, MaterialMapJsonConverter, and RootMaterialMapIo. Earlier feature validation also passed Portal, SurfaceMaterialJsonConverter, and the optional-key Python integration test; the history split changes only named-member access at existing call sites.

@github-actions github-actions Bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins Component - Documentation Affects the documentation labels Sep 17, 2026
@github-actions github-actions Bot added this to the next milestone Sep 17, 2026
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Public API surface diff

+27 added, 1 breaking.

⚠️ Breaking API changes (source-level)

Removed types / aliases / enums / variables (1)
  • aliases Acts::TrackingGeometryMaterial

➕ Added public API

New types / aliases / enums / variables / concepts (4)
  • aliases Acts::KeyedSurfaceMaterialMaps
  • type Acts::KeyedSurfaceMaterial
  • type Acts::MergedMaterialMarker::Origin
  • type Acts::TrackingGeometryMaterial
New call signatures (incl. defaulted-arg overloads) (15)
  • Acts::BinnedSurfaceMaterialAccumulator::finalizeMaps(ISurfaceMaterialAccumulator::State &, const GeometryContext &) const
  • Acts::ISurfaceMaterialAccumulator::finalizeMaps(State &, const GeometryContext &) const
  • Acts::JsonMaterialDecorator::materialMaps() const
  • Acts::MaterialDesignatorBlueprintNode::configureFace(CuboidVolumeBounds::Face, const AxisSpec &, const AxisSpec &, std::optional< std::string >)
  • Acts::MaterialDesignatorBlueprintNode::configureFace(CuboidVolumeBounds::Face, const DirectedProtoAxis &, const DirectedProtoAxis &, std::optional< std::string >)
  • Acts::MaterialDesignatorBlueprintNode::configureFace(CylinderVolumeBounds::Face, const AxisSpec &, const AxisSpec &, std::optional< std::string >)
  • Acts::MaterialDesignatorBlueprintNode::configureFace(CylinderVolumeBounds::Face, const DirectedProtoAxis &, const DirectedProtoAxis &, std::optional< std::string >)
  • Acts::MergedMaterialMarker::MergedMaterialMarker(std::vector< Origin >)
  • Acts::MergedMaterialMarker::origins() const
  • Acts::ProtoSurfaceMaterialT::ProtoSurfaceMaterialT(const BinningType &, MappingType, std::optional< std::string >)
  • Acts::ProtoSurfaceMaterialT::materialKey() const
  • Acts::TrackingGeometryMaterial::apply(Surface &) const
  • Acts::TrackingGeometryMaterial::apply(TrackingGeometry &) const
  • Acts::TrackingGeometryMaterial::apply(TrackingVolume &) const
  • Acts::TrackingGeometryMaterial::apply(std::span< Surface *const >) const
New public data members (8)
  • Acts::BinnedSurfaceMaterialAccumulator::State::materialSurfaceRegistry
  • Acts::KeyedSurfaceMaterial::geometryId
  • Acts::KeyedSurfaceMaterial::material
  • Acts::MergedMaterialMarker::Origin::geometryId
  • Acts::MergedMaterialMarker::Origin::materialKey
  • Acts::TrackingGeometryMaterial::keyedSurfaces
  • Acts::TrackingGeometryMaterial::surfaceMaterials
  • Acts::TrackingGeometryMaterial::volumeMaterials

@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

📊: Physics performance monitoring for 9ec1a69

Full contents

physmon summary

❗️: Downstream build failure

  • Key4hep (cc @acts-project/key4hep-contacts)

@paulgessinger
paulgessinger force-pushed the codex/stable-material-keys branch from 33ad18b to 109ed99 Compare September 18, 2026 08:20
@noemina
noemina self-requested a review September 18, 2026 14:31
@sonarqubecloud

Copy link
Copy Markdown

andiwand pushed a commit to andiwand/acts that referenced this pull request Sep 19, 2026
… fields (acts-project#6123)

`TrackingGeometryMaterial` is currently a `std::pair`, so callers depend
on positional members and tuple decomposition. Replace it with a
dedicated struct containing `surfaceMaterials` and `volumeMaterials`,
and migrate the existing mapper, JSON/ROOT I/O, and tests to named
member access.

This is the standalone API prerequisite for acts-project#6109. It contains no stable
keys, application methods, validation changes, or material-map format
changes. The two existing maps and their behavior are unchanged.

Breaking change and migration:
- Replace `.first` with `.surfaceMaterials` and `.second` with
`.volumeMaterials`.
- Replace pair-specific operations (`std::get`, pair conversions, and
tuple-style access) with named member access. Avoid structured bindings
so later fields can be added without changing callers.
- Existing two-map brace initialization remains valid.

Validation: built and passed MaterialMapper, MaterialMapJsonConverter,
MaterialJsonDocumentation, and RootMaterialMapIo against this commit
independently of acts-project#6109. JSON examples and ROOT round trips remain
unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API breaking Component - Core Affects the Core module Component - Documentation Affects the documentation Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins Public API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant