[TOPI][Relay][ONNX] Replace scatter_add by scatter_elements(reduction="add")#14008
Merged
AndrewZhaoLuo merged 19 commits intoapache:mainfrom Feb 23, 2023
Merged
[TOPI][Relay][ONNX] Replace scatter_add by scatter_elements(reduction="add")#14008AndrewZhaoLuo merged 19 commits intoapache:mainfrom
AndrewZhaoLuo merged 19 commits intoapache:mainfrom
Conversation
Collaborator
|
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
vvchernov
force-pushed
the
vc/rem_scatter_add
branch
3 times, most recently
from
55ff794 to
171a8cb
Compare
vvchernov
changed the title
vvchernov
force-pushed
the
vc/rem_scatter_add
branch
from
c9abb97 to
20c1b5e
Compare
vvchernov
changed the title
vvchernov
force-pushed
the
vc/rem_scatter_add
branch
3 times, most recently
from
4b0d49b to
ca239fe
Compare
| from .nms import atomic_add | ||
|
|
||
|
|
||
| def gen_scatter_add_1d_atomic(data, indices, updates, out, axis, _): |
Contributor
There was a problem hiding this comment.
Ah this is something I think I might have missed. Within indices along a particular axis, do we expect all indices to be unique? Otherwise I believe something similar must be done with all operators.
Contributor
Author
There was a problem hiding this comment.
Yes, potentially we can. The problem is that quick search did not give me any atomic operation like atomic_add, moreover it is not supported by some architectures ("vulkan", "metal"). I postponed this task due to it requires more time for investigation and implementation but does not have any priority.
Contributor
Author
|
@tvm-bot rerun |
vvchernov
force-pushed
the
vc/rem_scatter_add
branch
from
e108505 to
f710d79
Compare
Contributor
Author
|
Hello @AndrewZhaoLuo! Could you see this PR again? |
added 11 commits
February 22, 2023 23:04
added 8 commits
February 22, 2023 23:04
vvchernov
force-pushed
the
vc/rem_scatter_add
branch
from
f710d79 to
4877b72
Compare
AndrewZhaoLuo
approved these changes
Feb 23, 2023
Contributor
AndrewZhaoLuo
left a comment
AndrewZhaoLuo
left a comment
There was a problem hiding this comment.
Thanks for the refactor @vvchernov !
yongwww
pushed a commit
to yongwww/tvm
that referenced
this pull request
Feb 27, 2023
…="add") (apache#14008) * update paddlepaddle * update tensorflow * update pytorch * remove scatter_add strategy * remove ScatterAddAttrs registration * remove scatter_add from front-end * remove scatter_add schedule and strategy * remove back-end API of scatter_add * update test_op_level3 * add checks for scatter_add to pytorch front-end * remove cpu topi implementation of scatter_add * remove scatter_add strategy for cuda-gpu * transfer reduction_func definition on higher level * use atomic_add in cude scatter_elements. remove cuda implementation for scatter_add * fix lint * remove ScatterAddAttrs * fix condition for using of atomic_add * last upstream of scatter_add with pytorch description. add test case * upstream scatter_elements for CPU with CUDA approach --------- Co-authored-by: Valery Chernov <valery.chernov@deelvin.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Operation scatter_add duplicates one of functionality of scatter_elements. It was replaced, but useful code was transferred to scatter_elements (particularly cuda implementation for 1d input)
cc @AndrewZhaoLuo