Repository navigation
Fix mx.remainder floored-mod for float16/bfloat16 on CPU - #3976
Conversation
0d8d2e3 to
d27d606
Compare
zcbenz
left a comment
There was a problem hiding this comment.
The correct way to fix this is to define our own is_signed_v utility that works with the half types.
Also there are too many unrelated changes in docs.
d27d606 to
830c0ef
Compare
|
Done. Added an |
|
You can check the definition of |
|
Thanks. Reworked |
830c0ef to
c6f1605
Compare
mx.remainderreturns the wrong result forfloat16andbfloat16on the CPU backend.The SIMD kernel sign-corrects the remainder so it takes the sign of the divisor, like
float32and the GPU backends. The correction is gated onstd::is_signed_v<T>, which is false for the customfloat16_tandbfloat16_t, so the half types skip it and return the raw IEEE remainder.Example with
a = [-1, -1, -1],b = [3, 5, 7]:float32:[2, 4, 6](correct)float16andbfloat16:[-1, -1, -1](wrong)The correction now gates on
!std::is_unsigned_v<T> && !is_complex<T>, in the scalar path (base_simd.h) and the Accelerate SIMD path (accelerate_simd.h). For every type that was already correct this is the same branch, so only the half types change.test_remainderis extended tofloat16andbfloat16.Verified on macOS (M3 Ultra, Metal + Accelerate) and Linux (CPU):
float16/bfloat16remainder now matchesfloat32on both CPU and GPU, andtest_ops.pypasses.complex64remainder is also wrong on CPU (it drops the imaginary part), which I'll address separately.