-
Notifications
You must be signed in to change notification settings - Fork 357
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
Remainder operator #1726
base: main
Are you sure you want to change the base?
Remainder operator #1726
Conversation
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.
Thank you for enhancing the code.
The code looks good, but I have noticed we are missing an entry in Burn book. Can you please add this documentation here: burn-book/src/building-blocks/tensor.md and you also mention about your new %
operator as well.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1726 +/- ##
=======================================
Coverage 86.47% 86.47%
=======================================
Files 698 698
Lines 82874 82888 +14
=======================================
+ Hits 71667 71681 +14
Misses 11207 11207 ☔ 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
Once the book is updated we can merge it!
Pull Request Template
Checklist
run-checks all
script has been executed.Related Issues/PRs
Related PR: 1597
Related Issue: 1510
Changes
Problem: The
tensor.remainder_scalar()
method has been implemented in a previous PR. However, its not possible to use the%
operator, e.g.,tensor % 2.0
.Changes: Added
ops::Rem
trait implementation for Tensor.Testing
Added a test for Tensor that makes use of the
%
operator.