Skip to content

fix(cuda): restore DeepSeek-V2 output by reverting the regressed RMSNorm kernel#829

Merged
inureyes merged 4 commits into
mainfrom
fix/issue-824-mlx-bump-deepseek-v2
Jul 20, 2026
Merged

fix(cuda): restore DeepSeek-V2 output by reverting the regressed RMSNorm kernel#829
inureyes merged 4 commits into
mainfrom
fix/issue-824-mlx-bump-deepseek-v2

Conversation

@inureyes

@inureyes inureyes commented Jul 20, 2026

Copy link
Copy Markdown
Member

Summary

Bumps the vendored MLX pin to the latest upstream (b7c3dd6d) and adds a CUDA RMSNorm overlay that fixes the DeepSeek-V2-Lite generation regression tracked in #824. The MLX pin stays at the latest commit; only the one regressed kernel file is overlaid back to its last-known-good version.

Root cause (pinned by a deterministic bisect)

Upstream MLX #3792 ("Fix CUDA RMSNorm small-row dispatch", commit a5a684d, first shipped in the 0.32.1 pin) regressed the small-axis CUDA RMSNorm. DeepSeek-V2's MLA kv_a_layernorm normalizes over kv_lora_rank (512), which with N_READS == 8 for half-precision activations lands on the multi-warp block_dim == 64 dispatch config that #3792 changed (groups_per_block 2 → 1). The later #3850 rewrite ("RMSNorm forward speed up") kept the same axis broken. The result was a deterministically-wrong normalizer that degenerated DeepSeek-V2 generation into repeated punctuation. Models whose norm axis is larger (qwen3 hidden_size 1024+, etc.) take a different dispatch path and were unaffected, which is why only DeepSeek-V2 broke.

MLX_USE_CUDA_GRAPHS=0 exposes the fault as deterministic; compute-sanitizer initcheck and memcheck are both clean, so it is a numerical kernel error, not a memory-safety bug. Bisect boundary: 1700b39 (commit before #3792) is coherent, a5a684d (#3792) is garbage. This corrects #817's earlier "memory-safety nondeterminism, not bisectable" reading (the graphs-on layer masked the determinism).

Fix

Overlay only mlx/backend/cuda/rms_norm.cu with the last-known-good upstream version (1700b39a, immediately before #3792), via the existing patches/mlx/backend/cuda/ overlay mechanism. Its helper dependencies (load_vector, BlockBroadcastReduce, dispatch_float_types, contiguous_copy_gpu) are unchanged between that commit and the pin, so it compiles as-is against the latest MLX. This trades the #3850 RMSNorm speedup for correctness on the affected small-axis path. The pin is updated in all three synchronized locations (mlx-cpp/CMakeLists.txt GIT_TAG, mlxcel-core/build.rs MLX_EXPECTED_COMMIT, .github/workflows/release.yml).

Verification (GB10)

  • deepseek-v2-lite-4bit greedy, MLX_USE_CUDA_GRAPHS=0: 4/4 coherent (was 0/4 garbage before the fix).
  • No regression: qwen3-0.6b-4bit, qwen2.5-0.5b-4bit (answers "Paris"), llama-3.2-1b-4bit all coherent and unchanged.

Scope / follow-ups

Closes #824


Update: second commit adds the CUDA-graph-capture fix (required for the server)

The RMSNorm overlay above fixes the deterministic forward, but DeepSeek-V2 also has the same CUDA-graph decode hazard as Gemma 4 (#688): with graph capture on, incremental decode degenerates into repeated tokens even though the identical forward is coherent with capture off. This is pre-existing (present before the RMSNorm regression) and dominates the mlxcel-server path (graph capture on by default), so the kernel fix alone left the server returning garbage.

Second commit extends the established MLX_USE_CUDA_GRAPHS=0 startup lever to the DeepSeek-V2 family (maybe_disable_cuda_graphs_for_gemma4 generalized to maybe_disable_cuda_graphs_for_model, now covering Gemma 4 / DeepSeek-V2 / DeepSeek-VL2). An explicit operator MLX_USE_CUDA_GRAPHS still overrides.

End-to-end deployment verification (GB10): mlxcel-server -m deepseek-v2-lite-4bit at default settings now answers coherently 5/5 ("The capital of France is Paris."), where it returned The!!!! garbage before. Startup log: "DeepSeek-V2 detected: disabling CUDA graph capture ... issue #824".

Both layers of #824 are now resolved: the RMSNorm numerical regression (overlay) and the pre-existing graph-capture hazard (startup lever). The upstream small-axis RMSNorm bug report to ml-explore/mlx remains a follow-up.

…orm kernel

Bump the vendored MLX pin to the latest upstream (b7c3dd6d) and add a CUDA RMSNorm overlay that fixes the DeepSeek-V2-Lite generation regression tracked in #824.

Root cause, pinned by a deterministic bisect: upstream MLX #3792 ("Fix CUDA RMSNorm small-row dispatch", commit a5a684d, first shipped in the 0.32.1 pin) regressed the small-axis CUDA RMSNorm. DeepSeek-V2's MLA kv_a_layernorm normalizes over kv_lora_rank (512), which with N_READS == 8 for half-precision activations lands on the multi-warp block_dim == 64 dispatch config that #3792 changed (groups_per_block 2 -> 1); the later #3850 rewrite ("RMSNorm forward speed up") kept the same axis broken. The result was a deterministically-wrong normalizer that degenerated DeepSeek-V2 text generation into repeated punctuation. Models whose norm axis is larger (qwen3 hidden_size 1024+, etc.) take a different dispatch path and were unaffected, which is why only DeepSeek-V2 broke.

The MLX_USE_CUDA_GRAPHS=0 run exposes it as deterministic (compute-sanitizer initcheck and memcheck are both clean, so it is a numerical kernel error, not a memory-safety bug). Bisect: 1700b39 (the commit before #3792) is coherent, a5a684d (#3792) is garbage.

Fix: keep the MLX pin at the latest commit and overlay only mlx/backend/cuda/rms_norm.cu with the last-known-good upstream version (1700b39a, i.e. immediately before #3792). The overlay's helper dependencies (load_vector, BlockBroadcastReduce, dispatch_float_types, contiguous_copy_gpu) are unchanged between that commit and the pin, so it compiles as-is against the latest MLX. This trades the #3850 RMSNorm speedup for correctness on the affected small-axis path; re-sync or drop the overlay once the upstream small-axis RMSNorm bug is fixed (an upstream report is a follow-up).

Verified on GB10: deepseek-v2-lite-4bit is deterministically coherent again with graphs off (4/4, was 0/4), and qwen3-0.6b / qwen2.5-0.5b / llama-3.2-1b are unchanged (no RMSNorm regression). The residual graphs-on nondeterminism for this model is the pre-existing CUDA-graph-capture layer noted in #824 (present even before this regression) and is out of scope here.

The pin is updated in all three synchronized locations (mlx-cpp/CMakeLists.txt GIT_TAG, mlxcel-core/build.rs MLX_EXPECTED_COMMIT, .github/workflows/release.yml).

Closes #824
@inureyes inureyes added type:bug Bug fixes, error corrections, or issue resolutions priority:high High priority area:models Model architectures, weights, loading, metadata status:review Under review labels Jul 20, 2026
inureyes added 2 commits July 20, 2026 09:50
…ecode (#824)

The RMSNorm overlay in the previous commit fixes the deterministic forward, but DeepSeek-V2 also has the same CUDA-graph decode hazard that Gemma 4 has (issue #688): with graph capture on, incremental decode degenerates into repeated tokens even though the identical forward is deterministically coherent with capture off. This is pre-existing (present since before the RMSNorm regression) and dominates the server path, which runs graph capture on by default, so without this the model still returns garbage under `mlxcel-server` even with the kernel fixed.

Extend the established `MLX_USE_CUDA_GRAPHS=0` startup lever to the DeepSeek-V2 family. Generalize `maybe_disable_cuda_graphs_for_gemma4` to `maybe_disable_cuda_graphs_for_model`, matching Gemma 4 (#688) and DeepSeek-V2 / DeepSeek-VL2 (#824) and logging the detected family and issue. An explicit operator `MLX_USE_CUDA_GRAPHS` still overrides. Unaffected families (Gemma 3, Qwen 3.5, etc.) keep graph capture on.

Verified on GB10: `mlxcel-server -m deepseek-v2-lite-4bit` at its default settings now answers coherently (5/5 "The capital of France is Paris."), where it returned `The!!!!` garbage before. The startup log shows "DeepSeek-V2 detected: disabling CUDA graph capture ... issue #824".

Refs #824
maybe_disable_cuda_graphs_for_model (src/loading/mod.rs) now covers both Gemma 4 (#688) and DeepSeek-V2 (#824), but its hot-swap Invariant paragraph and the two call-site comments in src/server/startup.rs and src/distributed/pipeline/stage_executor/mod.rs still read as Gemma-4-only, which both the implementation and security reviews flagged as stale.

Reworded the Invariant paragraph and the two call-site comments to hazard-family / non-hazard-family wording and referenced issue #824 alongside #688. The detailed Gemma-4 mechanism prose in the loading/mod.rs doc comment is untouched since it remains accurate for that family; no behavior changed.

Validation:
- cargo fmt --all -- --check: clean
- cargo clippy --lib --features cuda -- -D warnings: aborted per the watchdog constraint after it began a cold cutlass/MLX C++ rebuild under a fresh build-script fingerprint; killed the process tree before any object files landed and left the pre-existing green build untouched. Comment-only edits, so no warning surface changed.

Refs #824
@inureyes inureyes added status:done Completed and removed status:review Under review labels Jul 20, 2026
`expected_rows = (t * gh * gw) as i32` casts an already-i32 expression to i32, which trips `clippy::unnecessary_cast` under `-D warnings`. Pre-existing since the MiniMax-M3 processor landed; surfaced now because this branch touches `src/distributed/pipeline`, which triggers the pipeline-parallel-ci "Clippy on PP modules" step (the only CI job that lints this path). Drop the redundant cast.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:models Model architectures, weights, loading, metadata priority:high High priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(models): dedicated root-cause debugging of the DeepSeek-V2-Lite greedy nondeterminism (follow-up to #817)

1 participant