fix(detray): allow homogeneous material on a sparse set of surfaces by paulgessinger · Pull Request #5815 · acts-project/acts

paulgessinger · 2026-07-31T15:07:58Z

What

Lets the Detray conversion produce material on a sparse set of surfaces, and removes the workaround that was needed because it could not.

Why

homogeneous_material_factory::operator() indexed its own data by surface index - smallest surface index:

std::size_t sf_offset{*std::ranges::min_element(m_indices)};
for (auto [i, sf] : detray::views::pick(surfaces, m_indices)) {
  std::size_t sf_idx{i - sf_offset};
  const material<scalar_type> &mat = m_materials.at(sf_idx);

This silently assumes the surfaces carrying material form a contiguous block. Since max(sf_idx) = max(m_indices) - min(m_indices) >= n-1, with equality only when contiguous, any gap runs past the end of m_materials and throws out_of_range. So in practice only detectors where every surface carried material could be built.

The DetrayPayloadConverter worked around this by padding every volume with a dummy slab per surface (// @HACK: Detray does not like homoegeneous material only on SOME surfaces.). The filler could not be anything inert: material_slab::operator bool() rejects vacuum, zero thickness, thickness == max(), and zero density alike, and consistency_checker throws on any slab it rejects. So the padding used thickness = 42, mat = {42, ...} — roughly a radiation length of fictitious material on every surface that ACTS considered material-free, including surfaces whose real material is a grid.

Changes

homogeneous_material_factory: index the factory data by position in the sequence. The n-th entries of m_indices, m_materials, m_thickness and m_links all describe the same material instance, while m_indices holds the surface it belongs to. Surfaces carrying material no longer have to be contiguous.
DetrayPayloadConverter: drop the dummy-slab padding. Also stop pinning index_in_coll to the surface index — that field is the position in the detector's material collection, and forcing the two to match made insert_in_container size the collection to the largest surface index and default-fill the gaps with invalid material. Left unset, the collection packs in payload order.
DetrayGeometryConverter: only instantiate the homogeneous material reader when the target metadata supports slabs or rods (it hard-fails a static_assert otherwise), and check slab and rod support separately against what the payload actually contains. Only allocate the homogeneous payload when a volume contributes material, so a null payload means "none" rather than requiring callers to inspect contents.
itk_metadata: enable homogeneous material, which the generated ITk metadata otherwise lacks (it supported only grid-based maps, so DetrayGeometryConverter::convert() could not be instantiated for it).

Tests

New regression test homogeneous_material_on_sparse_surfaces: five surfaces, material on #1 and #4 — a gap, not starting at the first surface, not a trailing block.

Without the factory fix: vector::_M_range_check: __n (which is 3) >= this->size() (which is 2)
With it: passes.

DetrayPayloadConverterTests passes, including check_consistency. On the same geometry the slab count goes from 2517 (one per surface, i.e. all padding) to 2500 — 17 surfaces now correctly carry no homogeneous material, and detray accepts the sparse result.

Its assertions encoded the padding (volumes.size() == 6, "exactly one slab per surface") and were replaced with structural invariants: every registered volume exists and is non-empty, at most one entry per surface, each surface link in range and unique, index_in_coll unset.

🤖 Generated with Claude Code

https://claude.ai/code/session_015ZfKcVzPeWvRKHFGzVzXqc

DetrayGeometryConverter::convert() unconditionally instantiated the homogeneous material reader, which hard-fails a static_assert for metadata types that carry no material slabs or rods. Guard the reader instantiation with `if constexpr` on has_material_slabs/has_material_rods, and check slab and rod support separately against what the payload actually contains, so a geometry only converts material the target metadata can hold. Only allocate the homogeneous material payload when a volume actually contributes homogeneous material, so a null payload now means "this geometry has none" rather than requiring callers to inspect the contents. Also enable homogeneous material in the generated ITk metadata, which otherwise supports only grid-based material maps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZfKcVzPeWvRKHFGzVzXqc

The homogeneous material factory indexed its own data by (surface index - smallest surface index), which assumes the surfaces carrying material form a contiguous block. For material on a non-contiguous set of surfaces that index ran past the end of the factory's material vector and threw out_of_range, so only detectors where every surface carried material could be built. Index the factory data by position in the sequence instead: the n-th entries of m_indices, m_materials, m_thickness and m_links all describe the same material instance, while m_indices holds the surface it belongs to. Add a regression test covering material on a non-contiguous subset. With that, the Detray payload converter no longer has to pad every surface with a dummy material slab to keep the block contiguous. The padding gave every material-free surface a bogus slab (thickness 42, X0 42), which no valid filler could replace: material_slab::operator bool() rejects vacuum, zero thickness and zero density alike, and the consistency checker throws on any slab it rejects. Also stop pinning index_in_coll to the surface index. It is the position in the detector's material collection, and forcing the two to match made detray size that collection to the largest surface index and default-fill the gaps with invalid material. Leaving it unset packs the collection in payload order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZfKcVzPeWvRKHFGzVzXqc

github-actions · 2026-07-31T16:50:13Z

Public API surface diff

+1 added, 0 breaking.

➕ Added public API

New call signatures (incl. defaulted-arg overloads) (1)

Acts::AnyTrackStateProxy::addComponents(TrackStatePropMask)

github-actions · 2026-07-31T18:51:48Z

📊: Physics performance monitoring for `ab503df`

Full contents

physmon summary

❗️: Downstream build failure

eic-shell EICrecon (cc @acts-project/epic-contacts)

The DetrayGeometryConverter gating already handles metadata types without homogeneous material support, so ITk does not need it enabled.

paulgessinger · 2026-08-07T07:15:34Z

@niermann999 what do we do with this?

sonarqubecloud · 2026-08-07T08:14:11Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

niermann999

Looks good from the detray side, just some minor comments

niermann999 · 2026-08-07T12:24:37Z

+    // is indexed by the position in that sequence, while @c m_indices holds
+    // the index of the surface the material belongs to. The surfaces that
+    // carry material therefore do not have to be contiguous in the volume.
+    for (const auto [n, i] : detray::views::enumerate(m_indices)) {


Can we rename this, so that it becomes clearer which is the running index and which is the data? So, e.g.

Suggested change

for (const auto [n, i] : detray::views::enumerate(m_indices)) {

for (const auto [i, sf_idx] : detray::views::enumerate(m_indices)) {

and then current i->sf_idx and n->i

niermann999 · 2026-08-07T12:25:04Z

-    DETRAY_DEBUG_HOST("-> sf_offset=" << sf_offset);
+    // Add the material to the surfaces that the data links against.
+    //
+    // The n-th entries of @c m_indices, @c m_materials, @c m_thickness and


Suggested change

// The n-th entries of @c m_indices, @c m_materials, @c m_thickness and

// The i-th entries of @c m_indices, @c m_materials, @c m_thickness and

niermann999 · 2026-08-07T12:42:01Z

+/// Regression test: the material factory used to index its own data by
+/// (surface index - smallest surface index), which silently assumed that the
+/// surfaces carrying material form a contiguous block. For material on a
+/// non-contiguous set of surfaces that index ran past the end of the factory's
+/// material vector, so only detectors where every surface carried material
+/// could be built.


Not really relevant to know in the future?

Suggested change

/// Regression test: the material factory used to index its own data by

/// (surface index - smallest surface index), which silently assumed that the

/// surfaces carrying material form a contiguous block. For material on a

/// non-contiguous set of surfaces that index ran past the end of the factory's

/// material vector, so only detectors where every surface carried material

/// could be built.

niermann999 · 2026-08-07T12:44:14Z

+  using mat_factory_t = homogeneous_material_factory<detector_t>;
+
+  vecmem::host_memory_resource host_mr;
+  detector_t d(host_mr);


Can we add a dummy volume with a few surfaces here, so that we make sure the local surface indexing works?

niermann999 · 2026-08-07T12:48:57Z

+      }
+    } else {
+      // Surfaces that were not given material must not have any
+      EXPECT_NE(mat_link.id(), material_id::e_material_slab);


How about checking the same access that the navigator uses?

Suggested change

EXPECT_NE(mat_link.id(), material_id::e_material_slab);

EXPECT_FALSE(sf_desc.has_material());

paulgessinger marked this pull request as ready for review July 31, 2026 15:09

paulgessinger requested a review from niermann999 as a code owner July 31, 2026 15:09

paulgessinger requested review from paradajzblond and removed request for niermann999 July 31, 2026 15:09

paulgessinger changed the title ~~fix: allow Detray homogeneous material on a sparse set of surfaces~~ fix(detray): allow homogeneous material on a sparse set of surfaces Jul 31, 2026

github-actions Bot added Component - Plugins Affects one or more Plugins Component - Detray Affects the Detray project labels Jul 31, 2026

github-actions Bot added this to the next milestone Jul 31, 2026

paulgessinger requested a review from niermann999 July 31, 2026 16:36

paulgessinger force-pushed the detray-sparse-material branch from cf97d5b to ca4614c Compare July 31, 2026 16:39

fix(detray): keep ITk metadata without homogeneous material

ab503df

The DetrayGeometryConverter gating already handles metadata types without homogeneous material support, so ITk does not need it enabled.

github-actions Bot added the Public API label Aug 7, 2026

niermann999 approved these changes Aug 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(detray): allow homogeneous material on a sparse set of surfaces - #5815

fix(detray): allow homogeneous material on a sparse set of surfaces#5815
paulgessinger wants to merge 3 commits into
acts-project:mainfrom
paulgessinger:detray-sparse-material

paulgessinger commented Jul 31, 2026

Uh oh!

github-actions Bot commented Jul 31, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 31, 2026 •

edited

Loading

Uh oh!

paulgessinger commented Aug 7, 2026

Uh oh!

sonarqubecloud Bot commented Aug 7, 2026

Uh oh!

niermann999 left a comment

Uh oh!

niermann999 Aug 7, 2026 •

edited

Loading

Uh oh!

niermann999 Aug 7, 2026

Uh oh!

niermann999 Aug 7, 2026

Uh oh!

niermann999 Aug 7, 2026

Uh oh!

niermann999 Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	for (const auto [n, i] : detray::views::enumerate(m_indices)) {
	for (const auto [i, sf_idx] : detray::views::enumerate(m_indices)) {

	// The n-th entries of @c m_indices, @c m_materials, @c m_thickness and
	// The i-th entries of @c m_indices, @c m_materials, @c m_thickness and

	EXPECT_NE(mat_link.id(), material_id::e_material_slab);
	EXPECT_FALSE(sf_desc.has_material());

Uh oh!

Conversation

paulgessinger commented Jul 31, 2026

What

Why

Changes

Tests

Uh oh!

github-actions Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Public API surface diff

➕ Added public API

Uh oh!

github-actions Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊: Physics performance monitoring for ab503df

physmon summary

❗️: Downstream build failure

Uh oh!

paulgessinger commented Aug 7, 2026

Uh oh!

sonarqubecloud Bot commented Aug 7, 2026

Quality Gate passed

Uh oh!

niermann999 left a comment

Choose a reason for hiding this comment

Uh oh!

niermann999 Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niermann999 Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

niermann999 Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

niermann999 Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

niermann999 Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jul 31, 2026 •

edited

Loading

github-actions Bot commented Jul 31, 2026 •

edited

Loading

📊: Physics performance monitoring for `ab503df`

niermann999 Aug 7, 2026 •

edited

Loading