[Relay, TOPI] Support dynamic slicing on first few axes, keeping the rest static #8068
Conversation
This file contains 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
masahi
commented
May 18, 2021
| @@ -25,16 +25,19 @@ | |||
|
|
|||
| @tvm.testing.uses_gpu | |||
| def test_dynamic_strided_slice(): | |||
| def verify(dshape, begin, end, strides, output, slice_mode="end", test_ref=True, dtype="int32"): | |||
There was a problem hiding this comment.
output is not used in the test, so I removed it.
masahi
changed the title
masahi
force-pushed
the
dyn-slice-few-axes
branch
from
0784e59
to
4f1742a
Compare
|
This looks good to me. Do we need to do anything to support this behavior in the standard strided slice op? |
|
No, I believe it is already supported, although the static version doesn't have a constraint that (No idea why CI is running another job, it already passed). |
|
thanks @mbrookhart |
trevor-m
pushed a commit
to trevor-m/tvm
that referenced
this pull request
Jun 17, 2021
…rest static (apache#8068) * Supporting dynamic slice on first few axes * fix index normalization * update dynamic slice tests * pylint fix * fix loop index dtype * fix more dtype issue
trevor-m
pushed a commit
to neo-ai/tvm
that referenced
this pull request
Jun 17, 2021
…rest static (apache#8068) * Supporting dynamic slice on first few axes * fix index normalization * update dynamic slice tests * pylint fix * fix loop index dtype * fix more dtype issue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR relaxes the constraint that
begin,endandstridesparams of dynamic strided slice to have the same length as input rank, and enable slicing on only first few axes (in particular, slicing along batch dim only).For example, the output shape of ONNX NMS is
(num_detection, 3). However, when translated to relay, it becomes(?, ?), since current dynamic stride slice always slices on all axes, even if some axes are conceptually static. This posed a problem whenGatherNDis applied to the output of ONNX NMS, since it requires the second dimension to be static:tvm/src/relay/op/tensor/transform.cc
Lines 3348 to 3349 in c999a84
ready for review @mbrookhart @jwfromm @kevinthesun @yongwww