Skip to content

Validate ScatterND index depth - #32034

Merged
Akshay Sonawane (apsonawane) merged 4 commits into
mainfrom
fix/scatternd-zero-index-depth
Aug 20, 2026
Merged

Validate ScatterND index depth#32034
Akshay Sonawane (apsonawane) merged 4 commits into
mainfrom
fix/scatternd-zero-index-depth

Conversation

@apsonawane

Copy link
Copy Markdown
Contributor

This pull request improves input validation for the ScatterND operator and adds a corresponding unit test to ensure that invalid input is properly rejected.

Input validation:

  • Added a check in scatter_nd.h to ensure that the last dimension of the indices tensor is at least 1, returning an error if it is not.

Testing:

  • Added a unit test in scatter_nd_op_test.cc (ScatterND_rejects_zero_index_depth) to verify that the operator rejects indices tensors with a zero-sized last dimension.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request strengthens runtime input validation for the CPU ScatterND operator by rejecting indices tensors whose last dimension (index depth) is zero, and adds a unit test to ensure that invalid case is properly rejected.

Changes:

  • Add ValidateShapes guard to require indices.shape[-1] >= 1, returning INVALID_ARGUMENT otherwise.
  • Add a unit test that constructs indices with shape {1, 0} and asserts the operator fails with the expected error substring.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
onnxruntime/core/providers/cpu/tensor/scatter_nd.h Adds a shape-validation check to reject zero (or negative) index depth in indices.
onnxruntime/test/providers/cpu/tensor/scatter_nd_op_test.cc Adds a regression test ensuring the new validation triggers for {1, 0} indices.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@titaiwangms

Copy link
Copy Markdown
Contributor

Major review finding

onnxruntime/core/providers/cpu/tensor/scatter_nd.h:34

This rejects indices.shape[-1] == 0, but the ONNX ScatterND contract only requires the tuple length k to be at most rank(data) and defines updates.shape = indices.shape[:-1] ++ data.shape[k:]. It does not impose a lower bound of 1.

For example, data=[2,3], indices=[1,0], and updates=[1,2,3] represent one empty index tuple updating the entire tensor. The new validation rejects this contract-valid model across every provider using the shared helper.

Please handle k == 0 safely rather than declaring it invalid.

auto-merge was automatically disabled August 20, 2026 17:53

Base branch was modified

@apsonawane
Akshay Sonawane (apsonawane) merged commit d63396d into main Aug 20, 2026
96 of 97 checks passed
@apsonawane
Akshay Sonawane (apsonawane) deleted the fix/scatternd-zero-index-depth branch August 20, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants