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] Fix mismatch between Onnx Prelu definition and importer. #7208

Merged
merged 1 commit into from
Jan 5, 2021

Conversation

jwfromm
Copy link
Contributor

@jwfromm jwfromm commented Jan 5, 2021

Our current prelu converter assumes that incoming data is in NCHW format and that the slope will have C total elements. Neither of these are actual requirements for ONNX PreLu. As pointed out in #7202, our converter fails in other cases. This PR makes our importer prelu compliant with the onnx spec.

@jwfromm
Copy link
Contributor Author

jwfromm commented Jan 5, 2021

@luyaor @mbrookhart @masahi can you guys take a look at this tiny 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.

LGTM. Thanks!

@luyaor
Copy link
Contributor

luyaor commented Jan 5, 2021

Hi @jwfromm @mbrookhart , thanks for the response and effort on this case. The PR looks good to me.

I am currently working on a research project related to TVM, would like to make more contributions to TVM and also looking forward to the feedback from community.

@jwfromm
Copy link
Contributor Author

jwfromm commented Jan 5, 2021

These two errors that you generated were excellent real bugs with the importer and were very easy to understand and replicate with your post. If they're being auto-generated they look excellent!

@masahi
Copy link
Member

masahi commented Jan 5, 2021

Hi @luyaor you are welcome to poke at pytorch frontend too, I hope it is more robust than onnx frontend :)

@masahi masahi merged commit 1812060 into apache:main Jan 5, 2021
tkonolige pushed a commit to tkonolige/incubator-tvm that referenced this pull request Jan 11, 2021
masahi pushed a commit to masahi/tvm that referenced this pull request Jan 14, 2021
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Jan 20, 2021
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jan 21, 2021
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
@jwfromm jwfromm deleted the onnx_prelu_fix branch April 12, 2023 15:55
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.

4 participants