Skip to content

Fix mx.remainder floored-mod for float16/bfloat16 on CPU - #3976

Merged
zcbenz merged 2 commits into
ml-explore:mainfrom
sashko-zakharchuk:fix-remainder-half
Aug 5, 2026
Merged

zcbenz merged 2 commits into
ml-explore:mainfrom
sashko-zakharchuk:fix-remainder-half

Conversation

@sashko-zakharchuk

@sashko-zakharchuk sashko-zakharchuk commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

mx.remainder returns the wrong result for float16 and bfloat16 on the CPU backend.

The SIMD kernel sign-corrects the remainder so it takes the sign of the divisor, like float32 and the GPU backends. The correction is gated on std::is_signed_v<T>, which is false for the custom float16_t and bfloat16_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)
  • float16 and bfloat16: [-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_remainder is extended to float16 and bfloat16.

Verified on macOS (M3 Ultra, Metal + Accelerate) and Linux (CPU): float16/bfloat16 remainder now matches float32 on both CPU and GPU, and test_ops.py passes.

complex64 remainder is also wrong on CPU (it drops the imaginary part), which I'll address separately.

@sashko-zakharchuk
sashko-zakharchuk force-pushed the fix-remainder-half branch 5 times, most recently from 0d8d2e3 to d27d606 Compare August 2, 2026 14:30

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sashko-zakharchuk

Copy link
Copy Markdown
Contributor Author

Done. Added an is_signed_v in base_simd.h (next to is_complex) that treats non-complex floating types as signed, covering float16_t/bfloat16_t, and gated the floored-mod correction on it in base_simd.h and accelerate_simd.h. Dropped the docs changes and rebased on main.

@zcbenz

zcbenz commented Aug 5, 2026

Copy link
Copy Markdown
Member

You can check the definition of is_array_v or is_vector_v for an idea how to do it correctly.

@sashko-zakharchuk

Copy link
Copy Markdown
Contributor Author

Thanks. Reworked is_signed_v to an explicit trait like is_array_v: std::is_signed_v<T> || std::is_same_v<T, float16_t> || std::is_same_v<T, bfloat16_t>, with half_types.h included in base_simd.h for the type names.

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch and good fix!

@zcbenz
zcbenz merged commit 2c46b95 into ml-explore:main Aug 5, 2026
28 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Aug 18, 2026
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants