-
Notifications
You must be signed in to change notification settings - Fork 85
[torchlib] Support integers in logical_and/or ops and update other logical ops #2582
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
Conversation
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
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.
Pull Request Overview
This PR adds support for integer inputs to PyTorch's logical operators (logical_and, logical_or, logical_not, logical_xor) in the torchlib implementation. The changes modify these operators to accept generic tensor types and handle both boolean and integer inputs by casting integers to boolean when needed.
- Separates bitwise operations from logical operations for better type safety
- Updates logical operators to accept TTensor instead of just BOOL types
- Adds test coverage for the newly supported logical operators
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
tests/function_libs/torch_lib/ops_test_data.py | Adds test entries for logical_and, logical_not, logical_or, and logical_xor operators |
onnxscript/function_libs/torch_lib/ops/core.py | Refactors bitwise and logical operators to handle different data types properly, adding integer support to logical ops |
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
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.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2582 +/- ##
==========================================
+ Coverage 70.04% 70.07% +0.02%
==========================================
Files 223 223
Lines 26215 26249 +34
Branches 2583 2597 +14
==========================================
+ Hits 18363 18393 +30
Misses 6946 6946
- Partials 906 910 +4 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Is this PR fixing some bugs or just moving code around to improve code quality? I see it combines some code to avoid function-signature dispatching? It would be better we put motivation and result in the description, since the linked PR (original) did not specify either.
@titaiwangms Updated, thanks |
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
This PR
bitwise_*
functions so that thelogical_*
functions are no longer handling bool overloads of the bitwise ops.logical_*
implementations.Replacement of #2579.