-
Notifications
You must be signed in to change notification settings - Fork 354
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
Adding max import #1769
Adding max import #1769
Conversation
b6547ee
to
946bf92
Compare
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.
Thanks for helping on the ONNX track!
If you look at the Max operator spec, it actually corresponds to the element-wise maximum between two tensors. This is equivalent to tensor.max_pair(other)
or torch.maximum(tensor, other)
.
Going through your implementation, it seems to correspond to ReduceMax
which is already implemented.
Oh, you're right, my bad, let me fix that |
It's all good 🙏 Honestly the ONNX spec can be quite confusing.. and there's a bunch of ops 😅 |
Should be fixed! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1769 +/- ##
==========================================
- Coverage 86.61% 86.41% -0.21%
==========================================
Files 700 734 +34
Lines 83423 85607 +2184
==========================================
+ Hits 72258 73978 +1720
- Misses 11165 11629 +464 ☔ View full report in Codecov by Sentry. |
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 for the addition 🙂
Pull Request Template
Checklist
run-checks all
script has been executed. -- run-check all gives a bunch of syntax errorsRelated Issues/PRs
Provide links to relevant issues and dependent PRs.
Changes
Followed the steps described in contributor-book
Testing
Ran the tests manually
cargo test max