refactor(material)!: Replace TrackingGeometryMaterial pair with named fields - #6123
Merged
andiwand merged 1 commit intoSep 19, 2026
Conversation
paulgessinger
added
Component - Core
Contributor
Public API surface diff+3 added, 1 breaking.
|
|
Contributor
andiwand
approved these changes
Sep 19, 2026
andiwand
enabled auto-merge
September 19, 2026 07:18
madbaron
pushed a commit
to key4hep/k4ActsTracking
that referenced
this pull request
Sep 19, 2026
Handle the TrackingGeometryMaterial struct introduced by acts-project/acts#6123 while retaining support for released ACTS versions that use a pair.
goblirsc
pushed a commit
to goblirsc/acts
that referenced
this pull request
Sep 21, 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.
stephenswat
pushed a commit
to stephenswat/acts
that referenced
this pull request
Sep 23, 2026
…cts-project#6109) Builds on the merged API-only prerequisite acts-project#6123. This PR now contains only the stable-key implementation on current main; the `TrackingGeometryMaterial` pair-to-struct migration is no longer part of its diff. 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 after rebasing onto main (`47d16806`): rebuilt Core, JSON/ROOT, and Core/JSON Python bindings. TrackingGeometryMaterial, MaterialMapper, MaterialMapJsonConverter, RootMaterialMapIo, and Portal tests passed, along with the optional-key Python integration test. The rebase retains main’s removal of deprecated material mappers and the existing axis checks in `Surface::assignSurfaceMaterial`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TrackingGeometryMaterialis currently astd::pair, so callers depend on positional members and tuple decomposition. Replace it with a dedicated struct containingsurfaceMaterialsandvolumeMaterials, and migrate the existing mapper, JSON/ROOT I/O, and tests to named member access.This is the standalone API prerequisite for #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:
.firstwith.surfaceMaterialsand.secondwith.volumeMaterials.std::get, pair conversions, and tuple-style access) with named member access. Avoid structured bindings so later fields can be added without changing callers.Validation: built and passed MaterialMapper, MaterialMapJsonConverter, MaterialJsonDocumentation, and RootMaterialMapIo against this commit independently of #6109. JSON examples and ROOT round trips remain unchanged.