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

[TF frontend] add support for StridedSlice to input a single constant #6949

Merged
merged 3 commits into from
Dec 17, 2020

Conversation

alter-xp
Copy link
Contributor

@@ -1599,6 +1599,9 @@ def _impl(inputs, attr, params, mod):
data_shape = get_const_tuple(in_type.checked_type.shape)
data_dim = len(data_shape)
stride_dim = len(stride)
if data_dim == 0 and isinstance(inputs[0], _expr.Constant):
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect this is data_dim==1, right? Otherwise, the data would just be empty. Also, could you add a test case to check this situation? In general, every new feature should come with an appropriate test (see the guidelines : https://tvm.apache.org/docs/contribute/pull_request.html)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I didn't describe it clearly, the input here is a single number. So data_dim here is 0. This situation is the same as np.array(1). len(np.array(1).shape) == 0

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, I didn't know that! I would still add a test to show when this situation arises (just to make sure that branch is covered). What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👌add test for that.

@alter-xp
Copy link
Contributor Author

alter-xp commented Dec 8, 2020

@giuseros hi, tests have been added.

Copy link
Contributor

@giuseros giuseros left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments @alter-xp ! LGTM

@alter-xp
Copy link
Contributor Author

eh, this branch has not been merged? Is there any problem?

@giuseros
Copy link
Contributor

Hi @alter-xp ,
I don't have write privileges. I guess that @FrozenGene , @anijain2305 , @comaniac , @mbaret might help to merge this in

@FrozenGene FrozenGene merged commit d5eb1da into apache:main Dec 17, 2020
@FrozenGene
Copy link
Member

Thanks @alter-xp @giuseros

TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Jan 20, 2021
…apache#6949)

* [TF frontend] add support for StridedSlice to input a single constant

* add test for strideslice with a single number input

* fix bug
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jan 21, 2021
…apache#6949)

* [TF frontend] add support for StridedSlice to input a single constant

* add test for strideslice with a single number input

* fix bug
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
…apache#6949)

* [TF frontend] add support for StridedSlice to input a single constant

* add test for strideslice with a single number input

* fix bug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants