Fix Downsample and Log crashing on MLX arrays - #222
Merged
Conversation
Both nodes advertise Array API support but could not run on MLX at all.
Downsample selected the kept samples with a NumPy integer index array, and
MLX rejects that outright ("Cannot index mlx array using the given type").
The kept samples are always an arithmetic sequence -- first at (-s_idx) % q,
then every q -- so this is now a strided slice. That is a view rather than a
gather on every backend, and 1.9-3.5x faster on MLX (M4 Pro, 30x256 through
512x1024) where it worked at all. The NumPy arange per message goes away too.
Log(clip_zero=True) reached xp.isdtype and finfo.smallest_normal, neither of
which MLX exposes. It also guarded the clip on bool(xp.any(data <= 0)), which
needs the answer on the host: a full device round-trip every message, stalling
the pipeline exactly where a lazy backend would otherwise run ahead. Clipping
unconditionally is one fused elementwise pass and 4.6-6.9x cheaper than the
branch it replaces. Only positive subnormals change, and raising those is what
clip_zero is for.
Adds util.array.np_finfo to resolve dtype limits by name against NumPy, since
backends disagree on what their own finfo carries (MLX's has no
smallest_normal) and the float formats are IEEE-identical anyway.
Also adds the two benchmark scripts these numbers come from. The A/B harness
walks the rules in Awni Hannun's "Writing Fast MLX" guide against what this
package ships; the end-to-end one measures whole nodes under jittered chunk
lengths, since run-to-run spread (~6%) is larger than several of the effects
under test and the isolated result does not always survive.
This was referenced Aug 22, 2026
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.
Two nodes that advertise Array API support could not run on MLX at all. Both turned up while benchmarking this package against the rules in Awni Hannun's Writing Fast MLX guide — the "before" case simply raised.
DownsampleSelected the kept samples with a NumPy integer index array:
The kept samples are always an arithmetic sequence — first at
(-s_idx) % q, then everyq— so this is now a strided slice. A view rather than a gather on every backend, and the per-messagenp.arangegoes away too.Guide rule: prefer
take/slicing over fancy indexing.Log(clip_zero=True)Reached
xp.isdtypeandfinfo.smallest_normal, neither of which MLX exposes. It also guarded the clip on a host read:That stalls the pipeline exactly where a lazy backend would otherwise be running ahead. Clipping unconditionally is one fused elementwise pass. The only behavioral difference is that positive subnormals are now also raised to
smallest_normalwhen nothing was<= 0— which is whatclip_zerois for.Guide rule: avoid accidental frequent evaluation.
Measurements
M4 Pro, MLX 0.31.2, streaming shapes,
async_evalper message with one synchronize at the end (i.e. how a streaming graph actually pays for it):Log(clip_zero=True)Downsampleselection, q=2Downsampleselection, q=8mx.takewas also measured as a candidate for the downsample gather and was not better than the gather (0.85–1.14x); the slice is the win.Also here
util.array.np_finfo— resolves dtype limits by name against NumPy. Backends disagree on what their ownfinfocarries (MLX's has onlymin/max/eps), and the float formats are IEEE-identical, so one lookup serves all of them. ReturnsNonefor dtypes NumPy cannot identify (e.g.bfloat16) rather than guessing.benchmarks/benchmark_mlx_gist_lessons.py— A/B micro-benchmarks walking each guide rule against what this package ships. Several candidates in it are measured and rejected (see below); it is kept so the constants can be re-derived on other hardware.benchmarks/benchmark_mlx_end_to_end.py— whole-node throughput under jittered chunk lengths, reporting min-of-repeats. Run-to-run spread here is ~6%, larger than several effects under test, so the isolated micro-result does not always survive.Candidates the benchmarks rejected, recorded so nobody re-litigates them:
abs(x)**2→real²+imag²inspectrum(0.68–0.89x, i.e. slower),x @ W→x @ W_contig.T(no effect at our shapes), and caching the per-launchcoef/valid_lengthMetal kernel constants (no effect).Tests
Regression tests for both crashes, each asserting MLX agrees sample-for-sample with the NumPy path across chunked streaming. 4061 pass.