-
Notifications
You must be signed in to change notification settings - Fork 85
[torchlib] Fix implementations for bitwise_* overloads #2618
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 <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
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 fixes the implementation of bitwise operations (bitwise_and
, bitwise_or
, bitwise_xor
, bitwise_left_shift
, bitwise_right_shift
) to properly handle scalar inputs by creating separate overloads for scalar operations. Previously, these operations only handled tensor-tensor operations, but PyTorch also supports tensor-scalar and scalar-tensor variants.
- Creates dedicated functions for scalar overloads (e.g.,
aten_bitwise_and_scalar
,aten_bitwise_and_scalar_tensor
) - Updates existing tensor-tensor functions to handle cases where one operand might not have a dtype
- Adds test coverage for bitwise_and with scalar input
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
onnxscript/function_libs/torch_lib/ops/core.py | Implements separate scalar overloads for all bitwise operations and updates dtype handling logic |
tests/function_libs/torch_lib/e2e_ops_tests.py | Adds test case for bitwise_and with scalar operand |
Comments suppressed due to low confidence (1)
onnxscript/function_libs/torch_lib/ops/core.py:1432
- After introducing dtype resolution logic,
self.dtype
should be replaced withdtype
to use the resolved dtype instead of the potentially None original dtype.
if self.dtype.is_integer():
return op.BitwiseXor(self, other)
if self.dtype == ir.DataType.BOOL:
Signed-off-by: Justin Chu <[email protected]>
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 6 comments.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2618 +/- ##
==========================================
- Coverage 70.32% 70.30% -0.02%
==========================================
Files 222 222
Lines 26224 26276 +52
Branches 2624 2624
==========================================
+ Hits 18442 18474 +32
- Misses 6865 6885 +20
Partials 917 917 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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 5 comments.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Some overloads for bitwise_* can accept scalar inputs which do not have the dtype. This PR creates implementations for the overloads.
Fix #2617