Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ScatterND operator in ONNX #2220

Merged
merged 13 commits into from
Aug 29, 2019
Merged

Conversation

BowenBao
Copy link
Contributor

@BowenBao BowenBao commented Aug 6, 2019

This is the inverse of GatherND #2106

This will support the exporting of torch.masked_scatter, torch.index_put, torch.copy_, tf.scatter_nd_update, etc.

@BowenBao
Copy link
Contributor Author

Copy link
Contributor

@spandantiwari spandantiwari left a comment

Choose a reason for hiding this comment

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

Overall, Looks good. @BowenBao, Could you please address the comment related to TF reference?

docs/Changelog.md Outdated Show resolved Hide resolved
@gramalingam
Copy link
Contributor

A question: I believe indices should not have a repeated value, is that correct? That is, we cannot have two or more updates for the same index-location. If so, let us add that to the documentation. (I guess this is common with scatter too, I presume.)

@BowenBao
Copy link
Contributor Author

@gramalingam Thanks for your detailed suggestions! I have updated the doc with clarification. You are right duplicate entries of indices is not supported.

Copy link
Contributor

@gramalingam gramalingam left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@spandantiwari
Copy link
Contributor

@gramalingam - thanks for the review!

@spandantiwari
Copy link
Contributor

@bddppq - for his review.

@BowenBao
Copy link
Contributor Author

@prasanthpul could you include this in ONNX 1.6 milestone as well? Thanks.

@prasanthpul prasanthpul added this to the 1.6 milestone Aug 19, 2019
@postrational postrational added the operator Issues related to ONNX operators label Aug 22, 2019
docs/Operators.md Outdated Show resolved Hide resolved
@BowenBao BowenBao force-pushed the onnx_scatternd branch 2 times, most recently from be2d6e1 to 0548023 Compare August 27, 2019 20:30
@ebarsoum ebarsoum merged commit 0e330e9 into onnx:master Aug 29, 2019
jcwchen pushed a commit to jcwchen/onnx that referenced this pull request Sep 23, 2020
Support ScatterND operator in ONNX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
operator Issues related to ONNX operators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants