fix(ops): propagate NaN through median - #4146
Conversation
median sorts and takes the midpoint. Sorting moves NaN to the end of the axis, so the midpoint slice never selects it and the NaN is silently dropped. Mask the result on any(isnan(...)) over the reduced axes for inexact dtypes, which matches max, min, mean, cummax and cummin, as well as NumPy and PyTorch.
|
Confirmed on an M5 Max, macOS 26.5, against The consistency argument holds up when it is measured, and it is a bit tidier Five ops disagree with numpy on main; with the four applied, none do. So these
One caveat on what that does and does not show: I applied them in the order 4146, |
Proposed changes
What's broken.
mx.mediansilently drops NaN — it returns a real number for input that contains NaN.Reproduced on CPU (
mlx 0.32.1.dev20260810+e78d894, source build,-DMLX_BUILD_METAL=OFF). Both NumPy and PyTorch returnnanhere:mx.mediantorch.mediannp.median[1.0, nan, 0.0][nan, 1.0, 0.0][-5.0, nan, 3.0][1.0, 0.0, nan]It is also inconsistent inside MLX:
max,min,mean,cummaxandcumminall propagate NaN, somedianis the odd one out among the reductions.The behaviour is shape-dependent, which makes it easy to miss.
medianover an axis of even length can average the NaN in by accident, so the same array gives a NaN along one axis and a plausible-looking number along another:Why.
mediansorts the reduced axes and slices the midpoint (mlx/ops.cpp).sortmoves NaN to the end of the axis, so for an odd-length axis the midpoint is always a non-NaN element and the NaN is never observed.The fix. After taking the midpoint, mask the result where the reduced axes contain a NaN. Guarded on
issubdtype(a.dtype(), inexact), so integer input (which is promoted to float but can never be NaN) keeps the original code path. Complex is covered too, matching NumPy's(nan+0j).The test.
test_median_naninpython/tests/test_ops.py, covering odd/even axis lengths, NaN in leading/middle/trailing position,float16/bfloat16/float32, per-axis and all-axes reductions,keepdims, complex, and negative controls (NaN-free float input and integer input are unchanged).Fails before, passes after:
python/tests/test_ops.py,test_autograd.pyandtest_reduce.pyare green (215 passed, 5422 subtests). The rest of the suite is green apart from 18 pre-existingMetal DLPack import is not availablefailures from my CPU-only build, which this change does not touch.Benchmark.
medianalready does an O(n log n) sort, so the added O(n)isnan+anypass is small. CPU, M4, best of 3 runs, usingbenchmarks/python/time_utils.py:median((1_000_000,))all axesmedian((1024,1024))all axesmedian((1024,1024), axis=1)median((256,256,64), axis=(0,2))median((64,64,64,64), axis=(1,3))median((1024,1024))int32, all axesThe int32 row takes the guarded path, so it runs byte-identical code in both builds; its +2.8% is the run-to-run noise floor on this machine. Everything except the largest 4-D multi-axis reduce sits inside that noise, and that case is +6.2%.
Checklist
pre-commit run --all-filesto format my code / installed pre-commit prior to committing changes