This is an automated email from the ASF dual-hosted git repository.
tlopex pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm.git
The following commit(s) were added to refs/heads/main by this push:
new ad0a225074 [Fix][Relax][Frontend][ONNX] Fix Scatter with indices
smaller than data (#20187)
ad0a225074 is described below
commit ad0a2250742d26948dd0ec50e980aeb33d5b8712
Author: HuEnwei <[email protected]>
AuthorDate: Fri Aug 28 04:56:58 2026 +0800
[Fix][Relax][Frontend][ONNX] Fix Scatter with indices smaller than data
(#20187)
Fixes: #20182
## Summary
The Relax ONNX frontend imported legal **opset-9/10 Scatter** models
whose
`indices`/`updates` are smaller than `data` (e.g. size-1 broadcast dims)
but
**silently produced wrong numeric output**. The spec iterates over
`indices'`
own shape — `output[idx[:axis] + (indices[idx],) + idx[axis+1:]] =
updates[idx]`
for each entry `idx` — while the frontend forwarded `indices`/`updates`
straight
to `relax.op.scatter_elements`, which only matches that semantics when
`indices.shape == data.shape`.
## Root cause
`Scatter._impl_v9` was a trivial forwarding to
`relax.op.scatter_elements`:
```python
@classmethod
def _impl_v9(cls, bb, inputs, attr, params):
axis = attr.get("axis", 0)
return relax.op.scatter_elements(inputs[0], inputs[1], inputs[2],
axis=axis)
```
`scatter_elements` (torch-`scatter_` semantics) writes for every
position of the
broadcast `indices`; ONNX Scatter instead iterates `indices'` own shape.
When
`indices` is smaller than `data`, the two disagree, and the lower-level
op
silently emits wrong values for the size-1-dim (broadcast) cases — e.g.
`data(2,3,4)`, `indices(1,3,1)`, `axis=0` writes updates to the wrong
cells
(`max|diff| = 298` in the minimal repro). A 72-case sweep (3 axes x
indices dims
in {1, dim} x 3 seeds) showed **12/72 wrong**, all with non-axis-1
broadcast dims.
## Fix
When `indices` is statically known and its shape differs from `data`'s,
lower the
per-entry semantics exactly as `scatter_nd` with explicit target
positions: a
constant coordinate grid of `indices'` own shape with the axis column
replaced by
the flattened `indices` values.
```python
@classmethod
def _impl_v9(cls, bb, inputs, attr, params):
...
# indices with a dynamic shape: keep the previous lowering.
if not all(isinstance(s, (tirx.IntImm, int)) for s in indices_shape):
return relax.op.scatter_elements(data, indices, updates, axis=axis)
# When indices has data's exact shape, scatter_elements is exact too.
if all(isinstance(s, (tirx.IntImm, int)) for s in data_shape) and list(
indices_shape
) == list(data_shape):
return relax.op.scatter_elements(data, indices, updates, axis=axis)
# per-entry targets: (n_entries, rank) grid of indices' own shape with
the
# axis column replaced by the flattened indices values -> exact
scatter_nd.
rank = len(data_shape)
axis = axis % rank
shape = tuple(int(s) for s in indices_shape)
n_entries = int(_np.prod(shape))
grid = _np.moveaxis(_np.indices(shape), 0, -1).reshape(n_entries, rank)
target = relax.op.where(
relax.const(
_np.broadcast_to(_np.eye(rank, dtype="bool")[axis], (n_entries,
rank)),
"bool",
),
relax.op.reshape(indices, (n_entries, 1)),
relax.const(grid.astype("int64"), "int64"),
)
return relax.op.scatter_nd(data, target, relax.op.reshape(updates,
(n_entries,)))
```
`scatter_nd` has no axis (coordinates are explicit), so negative axes
are
normalized with `axis % rank`. The exact-shape and dynamic-shape cases
keep the
original `scatter_elements` path, so conventional models are unchanged.
This also makes Scatter with mismatched `indices` correct for `axis=0/2`
(which
`test_scatter` previously skipped), so that skip is now restricted to
`ScatterElements` (whose `_impl_v11` still uses `scatter_elements`).
## Validation
Differential test (Relax `from_onnx` + `relax.build` + `VirtualMachine`
vs
onnxruntime) over 72 legal Scatter models: `data(2,3,4)` x `indices`
each dim in
{1, dim} x axes {0,1,2} x 3 seeds, plus the minimal repro, `(16,16,16)+
(8,8,8)` all axes, negative-axis and opset-10 cases. Verified on the
familyfuzz
locked build `262c6d2e0` via runtime monkey-patch (no source files
modified).
| Category | Cases | Before | After |
|---|---|---|---|
| broadcast / smaller `indices` (incl. size-1 dims) | 72 | **12 wrong
output** | match onnxrt, `max\|diff\| = 0` |
| `indices.shape == data.shape` (all axes incl. -1) | 4 | match | match
(no regression) |
| opset 10 broadcast | 2 | wrong | match |
| **Total** | **78** | 12 wrong | **0 wrong** |
In-tree tests added to `tests/python/relax/test_frontend_onnx.py`:
- `test_scatter_broadcast` — 4 axes x 4 broadcast `indices` shapes (16
cases),
`check_correctness` vs onnxruntime with `check_dtypes=True`: 16/16 pass.
- `test_scatter` — the `axis != 1` skip is now restricted to
`ScatterElements`;
Scatter runs for `axis=0/1/2` and passes.
Run:
```bash
pytest tests/python/relax/test_frontend_onnx.py::test_scatter \
tests/python/relax/test_frontend_onnx.py::test_scatter_broadcast
```
## Files changed
- `python/tvm/relax/frontend/onnx/onnx_frontend.py` — `Scatter._impl_v9`
lowers
shape-mismatched (broadcastable) `indices` as exact per-entry
`scatter_nd`.
- `tests/python/relax/test_frontend_onnx.py` — add
`test_scatter_broadcast`;
narrow the `test_scatter` skip to `ScatterElements`.
---
python/tvm/relax/frontend/onnx/onnx_frontend.py | 72 ++++++++++++++-
tests/python/relax/test_frontend_onnx.py | 117 +++++++++++++++++++++++-
2 files changed, 186 insertions(+), 3 deletions(-)
diff --git a/python/tvm/relax/frontend/onnx/onnx_frontend.py
b/python/tvm/relax/frontend/onnx/onnx_frontend.py
index a6a2a3152e..d078bdf9f6 100644
--- a/python/tvm/relax/frontend/onnx/onnx_frontend.py
+++ b/python/tvm/relax/frontend/onnx/onnx_frontend.py
@@ -1401,13 +1401,83 @@ class GatherND(OnnxOpConverter):
return relax.op.gather_nd(inputs[0], inputs[1], batch_dims)
+def _shapes_equal(a: list[tirx.Expr] | None, b: list[tirx.Expr] | None) ->
bool:
+ if a is None or b is None or len(a) != len(b):
+ return False
+ for x, y in zip(a, b):
+ x_static = isinstance(x, tirx.IntImm | int)
+ y_static = isinstance(y, tirx.IntImm | int)
+ if x_static and y_static:
+ if int(x) != int(y):
+ return False
+ elif (not x_static) and (not y_static):
+ if not x.same_as(y):
+ return False
+ else:
+ return False
+ return True
+
+
class Scatter(OnnxOpConverter):
"""Convert an onnx Scatter node into an equivalent Relax expression."""
@classmethod
def _impl_v9(cls, bb, inputs, attr, params):
+ data = inputs[0]
+ indices = inputs[1]
+ updates = inputs[2]
axis = attr.get("axis", 0)
- return relax.op.scatter_elements(inputs[0], inputs[1], inputs[2],
axis=axis)
+
+ indices_shape = indices.ty.shape
+ data_shape = data.ty.shape
+
+ if _shapes_equal(indices_shape, data_shape):
+ return relax.op.scatter_elements(data, indices, updates, axis=axis)
+
+ if indices_shape is None:
+ raise ValueError(
+ "Scatter with `indices` of unknown rank is unsupported, as the
per-entry "
+ "coordinate grid cannot be built"
+ )
+ if not all(isinstance(s, tirx.IntImm | int) for s in indices_shape):
+ raise ValueError(
+ "Scatter with dynamic `indices` whose shape is not provably
equal to "
+ "`data`'s shape is unsupported: the fallback lowering silently
produces "
+ "incorrect results for broadcast size-1 dims"
+ )
+ if data_shape is None:
+ raise ValueError(
+ "Scatter with `data` of unknown rank is unsupported, as the
per-entry "
+ "coordinate grid cannot be built"
+ )
+
+ # ONNX Scatter iterates over indices' own shape: for each entry idx,
+ # output[idx[:axis] + (indices[idx],) + idx[axis+1:]] = updates[idx]
+ # which is exactly scatter_nd with explicit per-entry target positions.
+ # The previous lowering passed a smaller indices (e.g. broadcastable
+ # size-1 dims) directly to scatter_elements, which silently produced
+ # wrong values.
+ rank = len(data_shape)
+ axis = axis % rank
+ # `StructInfo.dtype` is a PrimType, so coerce to a string for
numpy/relax.
+ indices_dtype = str(indices.ty.dtype)
+ shape = tuple(int(s) for s in indices_shape)
+ n_entries = int(_np.prod(shape))
+ # (n_entries, rank) coordinate grid of indices' own shape, C-order over
+ # its entries.
+ grid = _np.moveaxis(_np.indices(shape), 0, -1).reshape(n_entries, rank)
+ # Replace the axis column with the flattened indices values. The grid
is
+ # cast to the indices dtype (ONNX Scatter permits int32 indices) so
that
+ # both branches of `where` share a dtype.
+ target = relax.op.where(
+ relax.const(
+ _np.broadcast_to(_np.eye(rank, dtype="bool")[axis],
(n_entries, rank)),
+ "bool",
+ ),
+ relax.op.reshape(indices, (n_entries, 1)),
+ relax.const(grid.astype(indices_dtype), indices_dtype),
+ )
+ return relax.op.scatter_nd(data, target, relax.op.reshape(updates,
(n_entries,)))
@classmethod
def _impl_v11(cls, bb, inputs, attr, params):
diff --git a/tests/python/relax/test_frontend_onnx.py
b/tests/python/relax/test_frontend_onnx.py
index 4654a4082a..4e31015b10 100644
--- a/tests/python/relax/test_frontend_onnx.py
+++ b/tests/python/relax/test_frontend_onnx.py
@@ -1800,8 +1800,11 @@ def test_gather_nd(data_shape, indices_shape,
batch_dims):
@pytest.mark.parametrize("axis", [0, 1, 2])
@pytest.mark.parametrize(("name", "opset"), [("Scatter", 10),
("ScatterElements", 11)])
def test_scatter(axis: int, name: str, opset: int):
- if axis != 1:
- pytest.skip("The current topi impl is wrong, which only works for
axis=1")
+ if name == "ScatterElements" and axis != 1:
+ pytest.skip(
+ "ScatterElements with indices smaller than data is lowered via
scatter_elements, "
+ "which only works for axis=1"
+ )
input_shape = [16, 16, 16]
indices_shape = [8, 8, 8]
updates_shape = [8, 8, 8]
@@ -1822,6 +1825,116 @@ def test_scatter(axis: int, name: str, opset: int):
check_correctness(model, inputs={"indices": indices}, opset=opset)
[email protected]("indices_dtype", ["int32", "int64"])
+def test_scatter_broadcast(indices_dtype):
+ """Scatter (opset 9/10) whose indices/updates are smaller than data (size-1
+ broadcast dims) must follow the per-entry semantics of the ONNX spec:
+ output[idx[:axis] + (indices[idx],) + idx[axis+1:]] = updates[idx] for each
+ entry idx in indices' own shape. The previous lowering passed such indices
+ directly to scatter_elements, which silently produced wrong values. Both
+ ONNX-permitted indices dtypes (int32/int64) are covered, since the lowered
+ coordinate grid must match the indices dtype."""
+ data_shape = (2, 3, 4)
+ indices_proto = TensorProto.INT32 if indices_dtype == "int32" else
TensorProto.INT64
+ rng = np.random.RandomState(0)
+ for axis in [0, 1, 2, -1]:
+ for indices_shape in [(1, 3, 4), (2, 1, 4), (1, 3, 1), (1, 1, 1)]:
+ graph = helper.make_graph(
+ [
+ helper.make_node(
+ "Scatter", ["data", "indices", "updates"], ["output"],
axis=axis
+ )
+ ],
+ "scatter_broadcast_test",
+ inputs=[
+ helper.make_tensor_value_info("data", TensorProto.FLOAT,
data_shape),
+ helper.make_tensor_value_info("indices", indices_proto,
list(indices_shape)),
+ helper.make_tensor_value_info(
+ "updates", TensorProto.FLOAT, list(indices_shape)
+ ),
+ ],
+ outputs=[helper.make_tensor_value_info("output",
TensorProto.FLOAT, data_shape)],
+ )
+ model = helper.make_model(
+ graph,
+ producer_name="scatter_broadcast_test",
+ opset_imports=[helper.make_opsetid("", 9)],
+ )
+ inputs = {
+ "data": rng.randn(*data_shape).astype("float32"),
+ "indices": rng.randint(0, data_shape[axis % len(data_shape)],
indices_shape).astype(
+ indices_dtype
+ ),
+ "updates": rng.randn(*indices_shape).astype("float32"),
+ }
+ check_correctness(model, inputs=inputs, opset=9, check_dtypes=True)
+
+
+def test_scatter_dynamic_shape():
+ """Dynamic-shape Scatter: when indices is structurally provably equal to
data
+ (shared symbolic dims) it still lowers via scatter_elements and is correct;
+ a dynamic indices that is *not* provably equal (e.g. a broadcast size-1
dim)
+ must raise instead of silently emitting wrong values."""
+ n = tvm.tirx.Var("N", "int64")
+ rng = np.random.RandomState(1)
+ batch = 5
+
+ # data/indices/updates all share the symbolic batch dim N -> provably
equal,
+ # lowered via scatter_elements, correct per ONNX semantics.
+ shape_dict = {"data": [n, 3, 4], "indices": [n, 3, 4], "updates": [n, 3,
4]}
+ graph = helper.make_graph(
+ [helper.make_node("Scatter", ["data", "indices", "updates"],
["output"], axis=0)],
+ "scatter_dynamic_test",
+ inputs=[
+ helper.make_tensor_value_info("data", TensorProto.FLOAT, ["N", 3,
4]),
+ helper.make_tensor_value_info("indices", TensorProto.INT64, ["N",
3, 4]),
+ helper.make_tensor_value_info("updates", TensorProto.FLOAT, ["N",
3, 4]),
+ ],
+ outputs=[helper.make_tensor_value_info("output", TensorProto.FLOAT,
["N", 3, 4])],
+ )
+ model = helper.make_model(
+ graph, producer_name="scatter_dynamic_test",
opset_imports=[helper.make_opsetid("", 9)]
+ )
+ model.ir_version = 8
+ data = rng.randn(batch, 3, 4).astype("float32")
+ indices = rng.randint(0, batch, size=(batch, 3, 4)).astype("int64")
+ updates = rng.randn(batch, 3, 4).astype("float32")
+ ort_output = onnxruntime.InferenceSession(
+ model.SerializeToString(), providers=["CPUExecutionProvider"]
+ ).run(None, {"data": data, "indices": indices, "updates": updates})[0]
+ tvm_model = from_onnx(model, shape_dict=shape_dict, opset=9)
+ tvm_model = relax.transform.DecomposeOpsForInference()(tvm_model)
+ tvm_model = relax.transform.LegalizeOps()(tvm_model)
+ with tvm.transform.PassContext(opt_level=3):
+ ex = tvm.compile(tvm_model, target="llvm")
+ vm = relax.VirtualMachine(ex, tvm.cpu())
+ vm.set_input("main", data, indices, updates)
+ vm.invoke_stateful("main")
+ tvm_output = vm.get_outputs("main")
+ np.testing.assert_allclose(ort_output, tvm_output.numpy(), rtol=1e-6,
atol=1e-6)
+
+ # A dynamic indices that is not provably equal to data (size-1 broadcast
dim)
+ # must raise rather than silently produce wrong values.
+ shape_dict_bad = {"data": [n, 3, 4], "indices": [n, 1, 4], "updates": [n,
1, 4]}
+ graph_bad = helper.make_graph(
+ [helper.make_node("Scatter", ["data", "indices", "updates"],
["output"], axis=0)],
+ "scatter_dynamic_bad_test",
+ inputs=[
+ helper.make_tensor_value_info("data", TensorProto.FLOAT, ["N", 3,
4]),
+ helper.make_tensor_value_info("indices", TensorProto.INT64, ["N",
1, 4]),
+ helper.make_tensor_value_info("updates", TensorProto.FLOAT, ["N",
1, 4]),
+ ],
+ outputs=[helper.make_tensor_value_info("output", TensorProto.FLOAT,
["N", 3, 4])],
+ )
+ model_bad = helper.make_model(
+ graph_bad,
+ producer_name="scatter_dynamic_bad_test",
+ opset_imports=[helper.make_opsetid("", 9)],
+ )
+ with pytest.raises(ValueError, match="dynamic"):
+ from_onnx(model_bad, shape_dict=shape_dict_bad, opset=9)
+
+
@pytest.mark.parametrize(
"reduction, opset, data, indices, updates",
[