Skip to content

Conversation

justinchuby
Copy link
Collaborator

Some overloads for bitwise_* can accept scalar inputs which do not have the dtype. This PR creates implementations for the overloads.

Fix #2617

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]>
Copy link
Contributor

@Copilot Copilot AI left a 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 with dtype 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]>
@justinchuby justinchuby requested a review from Copilot October 9, 2025 17:04
@justinchuby justinchuby added the module: torchlib Related to the torch/aten function lib in development label Oct 9, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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.

Copy link

codecov bot commented Oct 9, 2025

Codecov Report

❌ Patch coverage is 56.33803% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.30%. Comparing base (cb6f873) to head (f763cd0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
onnxscript/function_libs/torch_lib/ops/core.py 56.33% 22 Missing and 9 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

@justinchuby justinchuby requested a review from Copilot October 9, 2025 17:38
Copy link
Contributor

@Copilot Copilot AI left a 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.

@justinchuby justinchuby added this to the 0.5.4 milestone Oct 9, 2025
@justinchuby justinchuby requested a review from Copilot October 9, 2025 18:47
@justinchuby justinchuby enabled auto-merge (squash) October 9, 2025 18:48
@justinchuby justinchuby added the merge at lgtm Reviewers can merge when they approve label Oct 9, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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.

@justinchuby justinchuby merged commit 59c3d32 into main Oct 9, 2025
32 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in ONNX Script Review Board Oct 9, 2025
@justinchuby justinchuby deleted the justinchu/bitwise-fix branch October 9, 2025 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge at lgtm Reviewers can merge when they approve module: torchlib Related to the torch/aten function lib in development

Projects

Development

Successfully merging this pull request may close these issues.

bitwise_and cannot have int argument

2 participants