-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Conversation
@luyaor @mbrookhart @masahi can you guys take a look at this tiny PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
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. |
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! |
Hi @luyaor you are welcome to poke at pytorch frontend too, I hope it is more robust than onnx frontend :) |
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.