Repository navigation
fix(flex): avoid overflow in boolean select updates - #5629
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (69.07%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #5629 +/- ##
==========================================
- Coverage 69.08% 69.07% -0.01%
==========================================
Files 1283 1283
Lines 201222 201265 +43
==========================================
+ Hits 139005 139031 +26
- Misses 62217 62234 +17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation restores OR semantics without overflow and includes focused regression coverage.
Pull request overview
Fixes boolean select updates overflowing u8 when many duplicate indices target the same element.
Changes:
- Replaces addition-and-clamping with direct boolean OR updates.
- Preserves the input boolean dtype.
- Adds regressions for duplicate indices, rows, columns, and transposed views.
File summaries
| File | Description |
|---|---|
crates/burn-flex/src/ops/gather_scatter.rs |
Adds the shared OR update kernel. |
crates/burn-flex/src/ops/bool.rs |
Routes boolean select updates through OR. |
crates/burn-backend-tests/tests/tensor/bool/ops/select.rs |
Covers overflow and indexing paths. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
laggui
approved these changes
Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
cargo run-checkshas been executed successfully.Related Issues/PRs
Fixes #5605.
Changes
Flex::bool_select_oraccumulated boolean updates in au8before clamping to 0/1. With 256 true updates to the same index, debug builds panic on overflow and unchecked arithmetic can wrap the result to false.Use OR in the existing indexed-update kernel instead. This keeps its shape and index validation, 2D fast paths, and view materialization, while retaining the original boolean dtype on the result. Duplicate updates now remain true regardless of their count; existing true values and unselected elements are preserved.
Add shared backend regressions for 256/512 duplicate indices, duplicate rows, and duplicate columns from transposed inputs.
Testing
All three new regression tests fail on the original implementation:
With the fix, the complete boolean select group passes:
cargo run-checks: passed, including formatting, typos, dependency audit, workspace Clippy, no-std compilation, Flex backend tests, and linear algebra tests (3,604 passed, 43 ignored, 0 failed in total). Its embedded Git downloader could not reach GitHub for RustSec. The current official advisory database (bf25f6575a93a35f30796c65c0ed91bee7fa19fd) was fetched through GitHub's API, and the unchanged audit rules passed against that snapshot. The full validation command uses a temporary local database path with fetching disabled; the repository audit configuration is restored afterward.AI-assisted development
Codex assisted with investigation, implementation, regression testing, and an independent review of the change. The review checked the OR semantics, input/result dtype, storage ownership, and general/2D indexing paths. The tests above exercise the real backend.
Local toolchain note: the release build emitted a
rust-objcopydebug-info stripping warning because its LLVM shared library is missing; compilation and all test commands still exited successfully.