Skip to content

[tmva][sofie] Do not mutate the indices tensor in Gather codegen - #23231

Merged
guitargeek merged 1 commit into
root-project:masterfrom
guitargeek:sofie-gather-runtime-indices
Sep 3, 2026
Merged

[tmva][sofie] Do not mutate the indices tensor in Gather codegen#23231
guitargeek merged 1 commit into
root-project:masterfrom
guitargeek:sofie-gather-runtime-indices

Conversation

@guitargeek

Copy link
Copy Markdown
Contributor

When the gather indices are not a known constant, the generated code corrected possible negative indices by writing them back into the indices tensor:

if (tensor_receivers[i] < 0)
tensor_receivers[i] += 5;

If the indices tensor is a model input, the generated pointer is "int64_t const*" and the emitted code does not compile. Runtime gather indices that are graph inputs are the norm for graph neural networks, where the edge sender/receiver lists are fed at inference time.

Emit the correction at the read site instead, leaving the input tensor untouched.

Covered by the new GatherRuntimeNegativeIndices test model, whose gather indices are a graph input instead of an initializer. The existing Gather tests all pass the indices as initializers, so the generated code took the constant-index branch and never exercised the correction.

🤖 Done with the help of AI

When the gather indices are not a known constant, the generated code
corrected possible negative indices by writing them back into the
indices tensor:

   if (tensor_receivers[i] < 0)
      tensor_receivers[i] += 5;

If the indices tensor is a model input, the generated pointer is
"int64_t const*" and the emitted code does not compile.  Runtime gather
indices that are graph inputs are the norm for graph neural networks,
where the edge sender/receiver lists are fed at inference time.

Emit the correction at the read site instead, leaving the input tensor
untouched.

Covered by the new GatherRuntimeNegativeIndices test model, whose gather
indices are a graph input instead of an initializer.  The existing
Gather tests all pass the indices as initializers, so the generated code
took the constant-index branch and never exercised the correction.

🤖 Done with the help of AI
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 9h 43m 50s ⏱️
 3 864 tests  3 862 ✅ 0 💤 2 ❌
75 926 runs  75 923 ✅ 1 💤 2 ❌

For more details on these failures, see this check.

Results for commit e4b49be.

@lmoneta lmoneta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Thank you Jonas for the fix!

@guitargeek
guitargeek merged commit 8faaf73 into root-project:master Sep 3, 2026
57 of 66 checks passed
@guitargeek
guitargeek deleted the sofie-gather-runtime-indices branch September 3, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants