Accumulate low-precision float reductions in float32 on the CPU#3909
Open
v-code01 wants to merge 1 commit into
Open
Accumulate low-precision float reductions in float32 on the CPU#3909v-code01 wants to merge 1 commit into
v-code01 wants to merge 1 commit into
Conversation
Sum/Prod reductions of float16/bfloat16 accumulated in the input dtype (reduction_op<InT, InT, ...>), so a long reduction saturates the accumulator: on the CPU a float16 sum of ones stalls around 16384 and a bfloat16 one at 256, each far from the true value and worse than numpy (which reduces in a wider type). Because mean and var are built on sum, they inherit the error - e.g. mx.mean of a large bfloat16 array of ones returns 0 instead of 1. Integer reductions already widen to int32; do the same for low-precision floats. Reduce into a float32 accumulator via the existing reduction_op machinery (which already supports a wider accumulator than the input, as used for int8/int16 -> int32) and narrow back to the output dtype once at the end. Every other dtype is unchanged, so float32/int/complex results stay bit-identical. After the fix the CPU float16/bfloat16 sums match numpy within the output dtype's range (a float16 sum still overflows to inf once the true value exceeds float16's ~65504 max, which is unavoidable for a float16 output), and mean/var are correct. Adds a regression test for float16/bfloat16 sum/mean/var (all-reduce and a strided axis reduction), pinned to the CPU stream.
Member
|
Thanks for putting this together |
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.
Problem
Sum/Prodreductions offloat16/bfloat16accumulate in the input dtype (reduction_op<InT, InT, ...>), so a long reduction saturates the accumulator:The CPU result is both wrong and worse than numpy, which reduces in a wider type. Because
meanandvarare built onsum, they inherit the error (a large bfloat16 mean of ones returns 0). This is the reduce-path sibling of the scan issue in #3907.Fix
Integer reductions already widen to
int32; do the same for low-precision floats. Reduce into afloat32accumulator through the existingreduction_opmachinery — which already supports an accumulator wider than the input (used forint8/int16 -> int32) — and narrow back to the output dtype once at the end. Every other dtype is untouched, sofloat32/ int / complex results stay bit-identical.A
float16sum still overflows toinfonce the true value exceeds float16's ~65504 max — that is unavoidable for afloat16output — but within range it now matches numpy, andmean/varare correct.Tests
Added
test_low_precision_float_accumulation(float16/bfloat16sum/mean/var, all-reduce and a strided axis reduction), pinned to the CPU stream. It fails on the current backend (stall/saturation) and passes with the fix. Fulltest_reduce.py+test_ops.py(157 tests / 5371 subtests) stay green; float32/int reductions remain bit-identical to numpy.Verified with a CPU-only build (
MLX_BUILD_METAL=OFF) on an M4.