Repository navigation
fix(cuda): add MLXCEL_SDPA_DETERMINISTIC for reproducible decode - #2162
Merged
Merged
Conversation
Qwen3 temp-0 output differed from run to run on GB10, and within one process, once the context passed 256 tokens (#2128). From 256 KV positions a one-row decode SDPA goes to cuDNN over the padded cache buffer, and cuDNN 9.27's first engine for the head_dim 128 graph uses stream-K work distribution (knob 38), which combines partial key reductions in completion order: about 1 call in 100 rounded differently (ULP level), and over 28 layers and hundreds of steps a near-tied argmax flipped. cuDNN does not tag the engine NONDETERMINISTIC, so a numerical-note filter keeps it. Stream-K is also what keeps that decode fast at long context, so the default stays as it is and determinism is opt-in, by maintainer decision. Barring stream-K on cuDNN cost 0.67x to 0.92x decode at 3K to 16K keys on GB10; sdpa_vector, whose KV split reduces in a fixed order, cost 0.87x to 1.00x. - MLXCEL_SDPA_DETERMINISTIC=1 (CUDA): one-row decode never takes cuDNN, so it runs sdpa_vector, and every other cuDNN SDPA graph bars stream-K candidates when a stream-K-off one exists. Off by default. - tests/cuda_sdpa_determinism.rs reproduces the defect without a checkpoint: 2000 identical decode SDPA calls over a padded 512-position cache must give one output with the switch on; with MLXCEL_SDPA_DETERMINISTIC=0 it fails (16 to 46 calls in 4000 differ). - tests/cuda_qmm_determinism.rs gains a Qwen3 case that decodes past the 256-position threshold, both tests set the switch unless the environment does, and the default checkpoint path now points at models/mlx/, where checkpoints live, so the tests run instead of skipping on a host that has them. - docs: the environment variable and an upstream report draft with the engine tags and the cost table. Closes #2128
Review follow-ups for MLXCEL_SDPA_DETERMINISTIC. - With the switch on, a forward graph whose stream-K-off candidates all fail support or build (the behavior-note filter, shared memory, workspace) no longer aborts: it is rebuilt on a fresh graph with the default selection, and a one-time warning names the switch. The same warning covers a graph where every candidate uses stream-K or an engine tag cannot be read. Only that case falls back, through a dedicated exception, so genuine cuDNN refusals still reach the bucketed-path handling. - The switch-on tests live in their own binary, which sets the switch before main with ctor, so cuda_qmm_determinism keeps guarding the default dispatch. A second SDPA case covers sdpa_vector's two-pass kernel (1500 of 2048 keys); on the default path that shape differed in 446 of 2000 calls. - Docs and comments: knob 38 is stated as the measured correlate (the compared plans also differ in tile knobs), the contributor's analysis on #2128 (an unordered cross-warp reduction of the softmax denominator) is cited, the stream-K bar on other forward graphs is called a precaution, ranges match the measurements, the overlay header and the MLXCEL_SDPA_FALLBACK_MAX_QUERIES row no longer say decode never enters cuDNN, the switch's integer parsing is documented, and the server's paged decode is recorded as measured deterministic across restarts in both modes.
4 tasks
This was referenced Oct 6, 2026
3 tasks done
inureyes
added a commit
that referenced
this pull request
Oct 6, 2026
) ## Summary Gemma 3 decodes on the lookahead pipeline again, now with a teardown that rewinds its model-owned state exactly, including a sliding window the decode has wrapped. On GB10 the issue's protocol (gemma-3-4b-it-4bit, 200 tokens, default flags) runs at 87.3 tok/s, against 87.6 for the pre-#2139 binary and 76.2 for force-sync, with greedy output byte-identical to `MLXCEL_FORCE_SYNC=1`. - `RotatingKVCache` gets an opt-in undo log (`cache/decode_undo.rs`): per single-token write the starting cursors, plus a copy of the overwritten `[B, H, 1, D]` rows when the write lands on a valid slot inside a `DecodeLookaheadAppendScope` (the scheduler's prime forward). `rewind_decode_writes(n)` restores rows newest first and the exact `offset`/`idx`, and refuses with the cache untouched under speculative buffering, non-FP16 storage, a disabled log, `n > offset`, or a write the log does not cover. Any other mutation clears the log. - `LanguageModel::supports_decode_lookahead_rewind` / `rewind_decode_appends`, delegated in `LoadedModel` and the VLM wrapper. Gemma 3 opts in when every sliding layer is FP16, logs depth 2 on scheduler sequences only (the CLI slot keeps none), and trims global layers with `KVCache::trim`. - Scheduler: `lookahead_params` admits a model-owned family only with that capability, including the `ModelOwned` allocation at `--parallel 1`. `apply_lookahead_trim` calls the rewind before `sync_sequence_storage`; on `Err` it logs, finishes the request with an error (overriding a same-tick `length`/`stop` so nothing is donated), and drops the id from the re-dispatch. The row copies are lazy MLX nodes reading the pre-write buffer. Left unevaluated they pin that buffer, so the write's `slice_update` cannot reuse it in place and copies the whole window every step. Measured on a 1906-token prompt (every write overwrites a valid slot): unflushed 64.5 tok/s, below force-sync; flushed in one `async_eval` per forward 72.4; no copies at all (unsound) 76.9. This also rules out the issue's rejected alternative of retaining pre-write handles: it pins the same buffers. Copies are taken only inside the prime forward, so force-sync and requests the pipeline does not admit pay nothing. ## Measurements (GB10, CUDA, `mlxcel-server`, default flags) Interleaved rounds, rotated order, fresh server per arm and round, median of 3 requests per arm, host-idle gate before each arm (CPU busy < 12%; an ungated first pass on a loaded host showed 40-60 tok/s outliers in both lookahead arms). `null` is a byte-identical copy of the `new` binary, `ref` is `12601f86` (pre-#2139), `base` is `0ee13dfe`. | 200 tokens, short prompt (n=4) | new | null | ref | sync | base | |---|---|---|---|---|---| | median tok/s | 87.3 | 87.2 | 87.6 | 76.2 | 75.7 | | paired new vs arm | | -0.2 to +0.6% | -1.1 to -0.2% | +13.8 to +16.7% | +13.1 to +23.1% | | 200 tokens after a 1906-token prompt, window wrapped (n=4) | new | null | ref | sync | base | |---|---|---|---|---|---| | median tok/s | 72.7 | 72.9 | 77.1 | 67.3 | 67.4 | | paired new vs arm | | -0.5 to +0.7% | -6.2 to -5.3% | +7.8 to +8.9% | +7.1 to +15.7% | The issue's bar (within 3% of 84.1-84.5) is met with margin, and the null arm puts the method's floor under 1%. Past the window the restored pipeline costs 5.7% against the pre-#2139 binary: that is the price of the row copies plus their extra `async_eval`, which the unsound `ref` never paid (its output differs there). It is still 8% faster than sync. Llama 3.2 1B (dense KV, gated, n=4): new 270.4, null 268.0, base 270.0, sync 210.0 tok/s; paired new vs base -0.3 to +0.5%; identical output in every arm. ## Correctness - Server, Gemma 3 4B, greedy, every round: short, the 1906-token prompt, and 4 concurrent requests give identical md5 for new, null, sync and base at `--parallel 4` (default) and at `--parallel 1`. `ref` differs on the long and concurrent requests (the #1754 desync). Concurrent requests are queued behind a long blocker prefill so both arms see the same batch history; without that, force-sync alone produced two different outputs across runs. - `scheduler_model_owned_lookahead_tests` (new) uses the pad-trim fixture (one sliding, one global layer), not the `scheduler_model_owned_cache_tests.rs:75` fixture the issue names: that fixture's `sliding_window_pattern: 1` makes its only layer a `Cache::Standard`, so it cannot wrap. A real Gemma 3 with a window of 8 runs pipelined and force-sync in lockstep, single sequence (`ModelOwned` allocation) and four concurrent (paged override), with an admission teardown and two-position finish teardowns after the wrap. At every tick without a live lookahead the offsets, cursor, whole sliding buffer and live global K/V are bit-equal, and tokens match. With the row restore removed both tests fail. Also: the capability gate (INT8 sliding layer stays sync), the trait default, and the error path. - `cache::decode_undo_tests`: rewinds of one and two writes after the wrap, across the cursor rollover, after an over-window prefill, across the warmup boundary, refusals untouched, scope behaviour. - `model_owned_sequence_never_primes_the_decode_lookahead` is replaced by `model_owned_sequence_primes_the_decode_lookahead_only_with_a_rewind`; the #1754 `length`, one-token and EOS snapshot tests pass unchanged on the pipelined path. ## Test plan - [x] `mlxcel-core` `cache::` (556), `server::batch` (495), `models::gemma3`, `vision::` pass with `--test-threads=1` under `gpu-lock` - [x] `cargo clippy --release --features cuda --lib --tests -- -D warnings` (both crates) and `cargo fmt --check` - [x] GB10 server runs above ## Not verified on this host - Metal: the row copy and flush were measured on CUDA only. - The branch is based on `0ee13dfe`; `e466f511` (#2162, CUDA SDPA switch) landed after and was not rebuilt here. Closes #2159
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.
Summary
Qwen3 temp-0 output differed from run to run on GB10 once the context passed 256 tokens (#2128), and it differed within one process too. The cause is the cuDNN engine the heuristics pick for the one-row decode step: its output depends on the order of an internal reduction, which a contributor's analysis on #2128 locates in an unordered cross-warp reduction of the softmax denominator. This PR adds
MLXCEL_SDPA_DETERMINISTIC=1, which makes decode bitwise reproducible. The default is unchanged, because stream-K is also the fast path at long context (decision recorded on #2128).Root cause
use_cudnn_for_decodingsends a one-row SDPA to cuDNN with k and v unsliced to the cache buffer and the true lengths in a padding mask.eng8_k40=0_k41=2_k24=2_k38=1_k27=0. Knob 38 isCUDNN_KNOB_TYPE_STREAM_K.[1,16,1,128]and k/v 300 rows of a 512-row buffer, 4000 identical calls gave 2 distinct outputs, differing by at most 7.6e-6 (ULP level). At 1500 of 2048 rows, 446 of 2000 calls differed.eng8_k40=2_k41=1_k24=1_k38=0_k27=0(stream-K off), gave 1 distinct output. It also differs in its tile and kernel-config knobs, so knob 38 is the measured correlate, not an isolated cause.NONDETERMINISTIC, sodeselect_numeric_noteskeeps it (tried).MLX_USE_CUDA_GRAPHS=0) and the KV in-place write (MLXCEL_KV_INPLACE_WRITE=0) left it unchanged.MLX_CUDA_USE_CUDNN_SDPA=0removed it.Changes
src/lib/mlx-cpp/patches/mlx/backend/cuda/scaled_dot_product_attention.cpp: withMLXCEL_SDPA_DETERMINISTIC=1:use_cudnn_for_decodingreturns false, so one-row decode runssdpa_vector, whose KV split reduces in a fixed order.build_sdpa_graphbars stream-K candidates (_k38=<non-zero>in the engine tag) on the remaining forward graphs (prefill, a bucketed verify block). Those graphs were not shown to be nondeterministic and were not separately measured.main.tests/cuda_sdpa_determinism.rs(new binary; sets the switch beforemainwithctorunless the environment does):sdpa_vector's two-pass kernel. Neither needs a checkpoint.tests/cuda_qmm_determinism.rskeeps guarding the default dispatch. Its default checkpoint path now points atmodels/mlx/, where checkpoints live; the old path never existed, so the bug: Gemma 4 26B A4B temp-0 decode is non-deterministic run-to-run and corrupts near-tied tokens independently of the fused MoE kernel #910 test always skipped. fix(cuda/quant): quantized prefill is not bitwise reproducible on sm_70, and its determinism test never runs #1558 still owns making a missing checkpoint fail instead of skip.docs/environment-variables.md: the variable, with the measured costs and what it does not cover. TheMLXCEL_SDPA_FALLBACK_MAX_QUERIESrow no longer says decode never enters cuDNN.docs/upstream/mlx-cuda-sdpa-stream-k-nondeterminism.md: upstream report draft for MLX and cuDNN.Cost of the two deterministic options
Decode throughput on GB10, relative to the default. Median of 3 interleaved runs on a quiet host, release build, 4-bit checkpoints.
sdpa_vector(this PR's switch)Prefill throughput was within noise (0.95x to 1.05x) with stream-K barred. Barring stream-K alone, which was the first version of this branch, costs more than moving decode to
sdpa_vector, so the switch does the latter.Verification
cuda_sdpa_determinismpasses 3 of 3 with the switch.MLXCEL_SDPA_DETERMINISTIC=0it fails 3 of 3: 11 of 2000 calls differ at 300 keys, 446 of 2000 at 1500 keys, and the Qwen3 logits diverge at decode step 204.cuda_qmm_determinismpasses on the default dispatch.mlxcel-server(Qwen3-1.7B, pool-backed paged decode, which does not use MLX SDPA) gave the same output for one greedy request across server restarts with the switch on and off. A repeat served from the prompt-prefix cache differed from the first, uncached run, identically in both modes. That is a separate path difference, not addressed here.cargo clippy --profile test-fast --features cuda -D warningson both test targets: clean.cargo fmt --check: clean. CI on the first head: all jobs green.Changes during review
Closes #2128