-
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] Add RNN operation for ONNX frontend #12213
Conversation
I'll take a look today |
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.
I think it looks fine. My main concern is about the high amount of similar code between this and LSTM. Can you try a refactor to share relevant parts with each other instead of copy paste?
Also q about tests
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!
…2213) * Add RNN operation for ONNX frontend. * link checks * rm test_rnn_batchwise in unsupported_onnx_tests * merge similar codes to class methods * implement opset 14 and refactor test_forward * reformat verify_rnn_helper Co-authored-by: 张亦驰 <zhangyichi1@corp.netease.com>
Add RNN operator for ONNX frontend. Note that currently onnxruntime (<= 1.12.0) has wrong default values (all zeros) for all activations so I removed default value tests for HardSigmoid.
cc @mbrookhart @AndrewZhaoLuo