Implement scatterElements #105
Conversation
|
@fdwr I've addressed your comments in the new commit, please take another look, thanks! |
There was a problem hiding this comment.
Thanks @huningxin!
I've addressed your comments, please take another review in new commit.
| let indicesValue = indices.getValueByIndex(indicesIndex); | ||
| indicesValue = indicesValue < 0 ? indicesValue + input.shape[axis] : indicesValue; | ||
| const outputLocation = | ||
| [...indicesLocation.slice(0, axis), indicesValue, ...indicesLocation .slice(axis + 1)]; |
There was a problem hiding this comment.
Should we restrict indices sizes are less than or equal to input sizes along every dimensions except axis? (indices.shape[i] <= input.shape[i] if i != axis). Otherwise, indicesLocation.slice() would contain invalid index value that is greater than output size.
BTW, DirectML requires "IndicesTensor.Sizes must match InputTensor.Sizes for every dimension except Axis." https://learn.microsoft.com/en-us/windows/win32/api/directml/ns-directml-dml_scatter_operator_desc. The current Chromium prototype follows this rule: https://chromium-review.googlesource.com/c/chromium/src/+/5921136/9/services/webnn/public/cpp/graph_validation_utils.cc#2563
CoreML only requires rank(indices)== rank(input): https://apple.github.io/coremltools/source/coremltools.converters.mil.mil.ops.defs.html#coremltools.converters.mil.mil.ops.defs.iOS15.scatter_gather.scatter_along_axis
@huningxin @fdwr @shiyi9801 PTAL, thanks.