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

[Relay][Frontend][Onnx] Refactor where importer to support dynamic shapes. #7394

Merged
merged 2 commits into from
Feb 5, 2021

Conversation

jwfromm
Copy link
Contributor

@jwfromm jwfromm commented Feb 2, 2021

The current implementation of Where in our importer uses static shape analysis to do broadcasting. Unfortunately, this can break when inputs have dynamic shapes. This PR refactors the analysis to instead support dynamism.

@jwfromm
Copy link
Contributor Author

jwfromm commented Feb 2, 2021

@soiferj can you take a look at this PR?

Copy link
Contributor

@mbrookhart mbrookhart left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Any chance we can add a test where one of the inputs actually has dynamic shapes?

@jwfromm
Copy link
Contributor Author

jwfromm commented Feb 2, 2021

I would definitely like to but wasn't sure what the right way to force dynamic shapes in a model is. I think we would need to compose a composite graph of some dynamic op followed by where, which might be a very ugly test. Do you know a better way to do that?

@mbrookhart
Copy link
Contributor

My goto has been do throw in a shape_of op and do some computations on it to get dynamic inputs, even if it's just op(x) -> op(reshape(x, shape_of(x))

@mbrookhart mbrookhart merged commit 91e07e1 into apache:main Feb 5, 2021
@mbrookhart
Copy link
Contributor

Thanks @jwfromm

alexwong pushed a commit to alexwong/tvm that referenced this pull request Feb 11, 2021
…apes. (apache#7394)

* Refactor where importer to support dynamic shapes.

* Add a test for dynamic where.
Lokiiiiii pushed a commit to Lokiiiiii/tvm that referenced this pull request Mar 2, 2021
…apes. (apache#7394)

* Refactor where importer to support dynamic shapes.

* Add a test for dynamic where.
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Mar 2, 2021
…apes. (apache#7394)

* Refactor where importer to support dynamic shapes.

* Add a test for dynamic where.
@jwfromm jwfromm deleted the onnx_dynamic_where branch April 12, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants