onnx / onnx

Open standard for machine learning interoperability
https://onnx.ai/
Apache License 2.0
17.76k stars 3.67k forks source link

ScatterND's doc gives wrong result for 3d indice case #6019

Open haowang5128 opened 7 months ago

haowang5128 commented 7 months ago

Bug Report

Describe the bug

In ScatterND doc , it shows scatternd computation as:

output = np.copy(data)
update_indices = indices.shape[:-1]
for idx in np.ndindex(update_indices):
    output[indices[idx]] = f(output[indices[idx]], updates[idx])

For a 3d indices case like this:

input = [[[0., 1.],  [2., 3.]], [[4., 5.], [6., 7.]]]
indices = [[[0, 0]]]
updates = [[[100., 101.]]]
reduction  = None

It provides results different from ONNXRuntime.

In the doc's method, indices[idx] gives a numpy array, output[indices[idx]] will not select the correct element. Convert indices[idx] to tuple will fix this.

output = np.copy(data)
update_indices = indices.shape[:-1]
for idx in np.ndindex(update_indices):
    output[tuple(indices[idx])] = f(output[tuple(indices[idx])], updates[idx])
liqunfu commented 7 months ago

@haowang5128 , thank you for pointing out this. Would you like to create a PR for the fix. If so, please also correct sample implementation at: https://github.com/onnx/onnx/blob/9cc907f78e91600ab43bfda125592ec34edda0c4/onnx/backend/test/case/node/scatternd.py#L26. The code does produce correct test data. However, to have extra tuple at the right hand side makes more sense.

xadupre commented 6 months ago

Is it on CPU or GPU? This PR addresses one issue on GPU: https://github.com/microsoft/onnxruntime/pull/19540.