Repository navigation
Round floating-point floor_divide to a whole number like ATen - #23685
Merged
Merged
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23685
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 69a3b5e with merge base 6cc8edf ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
Contributor
Author
|
@pytorchbot label "release notes: ops & kernels" |
pytorch-bot
Bot
added
the
release notes: ops & kernels
executorch-triage
Bot
added
the
community: contribution
JakeStevens
approved these changes
Oct 13, 2026
JakeStevens
left a comment
JakeStevens
left a comment
Contributor
There was a problem hiding this comment.
can you rebase and re run the tests? a lot got skipped. LGTM once we are green
utils::floor_divide returned (a - fmod(a, b)) / b, which can be off from a whole number by a rounding error (4.2 // 1.4 gave 2.9999999999999996), and returned +-inf by the sign of a for a zero divisor. Follow ATen's div_floor_floating: floor the quotient and round it up if it is more than 0.5 above, and return a / b for a zero divisor. Fixes pytorch#23684
usmanmateen
force-pushed
the
fix/floor-divide-rounding
branch
from
October 13, 2026 19:22
293bfe2 to
69a3b5e
Compare
Contributor
Author
|
Rebased on main (6cc8edf). Locally, |
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.
Summary
Fixes #23684.
For floating-point inputs,
utils::floor_divideinkernels/portable/cpu/util/math_util.hreturned(a - fmod(a, b)) / b, minus 1 when the signs differ. That quotient can be off from a whole number by a rounding error. For example,4.2 // 1.4gave2.9999999999999996in float64, so.long()on the result gives 2 instead of 3. About 7% of random float64 inputs and 6% of random float32 inputs were affected. For a zero divisor it returned±infby the sign ofa, so0.0 // 0.0gaveinfand the sign of a-0.0divisor was ignored.This follows ATen's
div_floor_floating. It floors the quotient and adds 1 if the quotient was more than 0.5 above the floor. It returns a signed zero when the quotient is 0, anda / bfor a zero divisor. The helper is used byfloor_divide.outand bydiv.out_mode/div.Scalar_mode_outwithrounding_mode="floor"in the portable kernels, and by the Cadence HiFi and Fusion G3divoperators.Test plan
Added two tests to
kernels/test/op_floor_divide_test.cpp:FloatResultIsWholeNumbercovers float64 and float32 cases whose quotient was off from a whole number, including negative results.FloatDivideByZerocovers0/0,±1/0and±1/-0.Expected values match eager PyTorch. I built
portable_kernels_testwith CMake (EXECUTORCH_BUILD_TESTS=ON) on Linux x86_64 (WSL2). Before this change, both new tests fail. After it, allOpFloorDivideTestandOpDivOutTesttests pass, and the fullportable_kernels_testrun passes (1650 tests).clang-format18.1.3 reports no changes on the two files.This PR was authored with AI assistance (Claude Code).
cc @nil-is-all