Skip to content

Handle broadcasted bool masks in aten.index_put#2966

Open
justinchuby with Copilot wants to merge 2 commits into
mainfrom
copilot/follow-up-bincount-index-put
Open

Handle broadcasted bool masks in aten.index_put#2966
justinchuby with Copilot wants to merge 2 commits into
mainfrom
copilot/follow-up-bincount-index-put

Conversation

Copilot AI commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

aten.index_put's bool-mask lowering handled the covered single-mask path correctly, but the multi-mask path assumed equal True counts and failed on valid broadcasted cases. It also rejected mixed bool/None indexing such as x[:, mask] = value, even though the generic advanced-index path can represent that form.

  • What changed

    • Restrict the dedicated bool-mask lowering to the single [mask] case.
    • Convert bool indices in mixed / multi-index cases into integer position tensors via NonZero, then reuse the existing advanced-index ScatterND lowering.
    • Make advanced-index rank bookkeeping tolerant of these converted bool indices, whose symbolic shape metadata may be absent during export.
  • Behavioral impact

    • Multi-mask bool indexing now supports PyTorch-valid broadcasted cases where one mask selects a single position and another selects multiple positions.
    • Mixed indexing with None and a 1-D bool mask now lowers through the generic path instead of failing early.
  • Regression coverage

    • Add an e2e case for broadcasted multi-mask bool indexing.
    • Add an e2e case for bool mask indexing mixed with None.

Example of a newly covered case:

x = torch.zeros((3, 4), dtype=torch.float32)
mask0 = torch.tensor([False, True, False], dtype=torch.bool)
mask1 = torch.tensor([True, False, True, True], dtype=torch.bool)
update = torch.tensor([10.0, 20.0, 30.0], dtype=torch.float32)

torch.ops.aten.index_put(x, [mask0, mask1], update)
# updates x[1, [0, 2, 3]]

Copilot AI changed the title [WIP] Fix multi-mask index_put equal True counts assumption Handle broadcasted bool masks in aten.index_put Jul 16, 2026
Copilot AI requested a review from justinchuby July 16, 2026 00:03
@justinchuby
justinchuby requested a review from Copilot July 16, 2026 15:20
@justinchuby
justinchuby marked this pull request as ready for review July 16, 2026 15:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 aten.index_put lowering for boolean masks by avoiding incorrect assumptions in the multi-index bool-mask path and routing mixed/multi boolean indexing through the existing advanced-index (ScatterND) lowering.

Changes:

  • Restricts the dedicated boolean-mask lowering to the single-index form ([mask]).
  • Converts 1-D boolean indices appearing in mixed/multi-index cases into integer position tensors via NonZero, then reuses the advanced-index ScatterND lowering (including broadcast handling).
  • Adds end-to-end export coverage for broadcasted multi-mask boolean indexing and for boolean indexing mixed with None (slice-like) indices.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/function_libs/torch_lib/e2e_ops_tests.py Adds new e2e export test cases covering broadcasted multi-mask bool indexing and bool mask mixed with None.
onnxscript/function_libs/torch_lib/ops/core.py Updates aten_index_put lowering to route mixed/multi bool masks through the advanced-index path by converting bool masks to integer indices, and adjusts rank bookkeeping accordingly.

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 31.57895% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.65%. Comparing base (35134aa) to head (65ce600).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
onnxscript/function_libs/torch_lib/ops/core.py 31.57% 11 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2966      +/-   ##
==========================================
+ Coverage   72.56%   72.65%   +0.09%     
==========================================
  Files         263      265       +2     
  Lines       32069    32189     +120     
  Branches     3020     3038      +18     
==========================================
+ Hits        23270    23388     +118     
+ Misses       7775     7769       -6     
- Partials     1024     1032       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Follow up on bincount + index_put

3 participants