Skip to content

fix(attention): cast SDPA masks to the attention dtype, not q's - #2417

Merged
inureyes merged 3 commits into
mainfrom
fix/issue-2336-gemma4-turbo4-sdpa-mask-dtype
Oct 16, 2026
Merged

inureyes merged 3 commits into
mainfrom
fix/issue-2336-gemma4-turbo4-sdpa-mask-dtype

Conversation

@inureyes

@inureyes inureyes commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Cause

The issue's hypothesis (a float32 q/k/v) was wrong. Logging the SDPA call on the crashing layer showed q bf16 [1,16,1980,256], k/v fp16 [1,8,1980,256], mask f32 [1980,1980]. TurboQuant caches always return fp16 K/V, so a bf16 model hands SDPA a bf16 query over fp16 K/V. SDPA attends in result_type(q, k, v), which is float32 for bf16 + fp16. The bridge cast the additive mask to the query's dtype (bf16), and the CUDA overlay's patched promotion table keeps bf16 + float32 at bf16, so the mask no longer promoted into the float32 output and SDPA threw. Below the window Gemma 4 attends maskless, which is why only prompts past 1024 tokens crashed. Metal's unpatched table promotes bf16 + float32 to float32, so it was CUDA-only.

Fix and why at this boundary

The bridge (fast_scaled_dot_product_attention, the sinks variant, the Metal 4 bridge) is the one place that already adapts mask dtypes to MLX's contract, and its stated purpose was to satisfy "must promote to output type". It picked the wrong target. A shared sdpa_mask_for_inputs now casts floating masks to result_type(q, k, v). The sinks bridge passes attention sinks through the same helper, because SDPA applies the same promotion rule to them (review finding; GPT-OSS greedy output on gpt-oss-20b-mxfp4-q4 is byte-identical before and after). When q/k/v share a dtype this is the same cast as before. mask_in_score_dtype on the chunked path now casts to the query dtype only when K and V share it.

Rejected: casting K/V back to bf16 in Gemma 4 (fixes one model, adds a per-layer K/V copy, and changes turbo4 numerics everywhere), and making turbo caches return the input dtype (changes the cache's fp16 contract that fused turbo kernels rely on).

Validation

  • New tests layers::tests::additive_mask_with_bf16_query_over_fp16_kv_is_accepted and gemma4_unified_mask_tests::turbo4_past_window_sliding_mask_is_accepted_by_sdpa (Turbo4Asym RotatingKVCache with bf16 K/V, past-window create_sliding_window_prefill_mask, trim_mask_to_keys, attention_from_ptr, compared with float32 attention). With the bridge change reverted both abort with Mask type must promote to output type float32. With it, they pass.
  • layers::tests (100) and models::gemma4 (120) pass. Clippy (-p mlxcel-core --lib --tests, root --lib --tests) and fmt are clean, and so are the scripts/ci checks.
  • GB10, ~/models/mlx/gemma-4-26b-a4b-it-4bit, mlxcel-server, 1987-token prompt, 300 tokens at temperature 0, MLXCEL_SDPA_DETERMINISTIC=1. On main, --kv-cache-mode fp16+turbo4 aborts. With the fix it completes, both with no drafter and with --model-draft ~/models/mlx/gemma-4-26b-a4b-it-assistant-bf16 --draft-kind mtp (MLXCEL_MTP_ALLOW_INEXACT=1). The turbo4 output is coherent and matches the FP16 KV arm for the first 287 characters, then diverges on wording, as expected from 4-bit V quantization.
  • Follow-ups from review, not in this PR: the CUDA chunk gate (cuda_sdpa_materializes_scores, materializing_sdpa_query_chunk) judges flash eligibility and score bytes from q's dtype, which understates mixed bf16/fp16 attention that runs in float32. The softcap composite still casts its mask to q's dtype and returns v's dtype. The CUDA dtype.cpp overlay header says bf16 + fp16 gives bf16, while its table gives float32.
  • A CHANGELOG.md entry is added under Fixed.
  • Acceptance criterion 4 (fix(cache): bound the turbo4 sliding append to visible_len #1793) was re-checked with a reverted-fix(cache): bound the turbo4 sliding append to visible_len #2330 build. Results are on fix(cache): bound the turbo4 sliding append to visible_len #1793.

Closes #2336

@inureyes inureyes added type:bug Bug fixes, error corrections, or issue resolutions priority:medium Medium priority area:models Model architectures, weights, loading, metadata area:inference Generation, sampling, decoding (incl. speculative, DRY) platform:linux Linux (CUDA / packaging) specific status:review Under review labels Oct 16, 2026
@inureyes inureyes added status:done Completed and removed status:review Under review labels Oct 16, 2026
Gemma 4 with `--kv-cache-mode fp16+turbo4` aborted in SDPA (`Mask type must promote to output type float32`) once a prefill ran past the 1024-token sliding window. Logging the call showed a bf16 query over fp16 keys and values (TurboQuant caches always return fp16) with an f32 additive mask. SDPA attends in `result_type(q, k, v)`, which is float32 for bf16 + fp16, and the bridge cast the mask to the query's bf16. The CUDA overlay's promotion table keeps bf16 + float32 at bf16, so that mask no longer promoted into the float32 output. Inside the window Gemma 4 attends maskless, which is why only longer prompts hit it.

The three SDPA bridges (`fast_scaled_dot_product_attention`, the sinks variant and the Metal 4 bridge) now cast floating masks through one helper, `sdpa_mask_for_inputs`, to `result_type(q, k, v)`. With matching q/k/v dtypes this is the cast they always did. `mask_in_score_dtype` on the query-chunked path now casts to the query dtype only when K and V share it.

Two regression tests (mlxcel-core layers, Gemma 4 past-window turbo4 mask) abort with the production error when the bridge change is reverted.

Closes #2336
SDPA rejects attention sinks that do not promote into `result_type(q, k, v)`, the same rule that aborted Gemma 4 on a TurboQuant cache for masks. The sinks bridge now passes sinks through `sdpa_mask_for_inputs`. SDPA casts them to that dtype itself, so numerics are unchanged: GPT-OSS greedy output on gpt-oss-20b-mxfp4-q4 is byte-identical before and after on GB10. Also corrects the `metal4_attention` doc comment and adds a CHANGELOG entry for #2336.

Refs #2336
@inureyes
inureyes force-pushed the fix/issue-2336-gemma4-turbo4-sdpa-mask-dtype branch from 1477214 to 48d5191 Compare October 16, 2026 01:53
@inureyes
inureyes merged commit c71403c into main Oct 16, 2026
27 checks passed
@inureyes
inureyes deleted the fix/issue-2336-gemma4-turbo4-sdpa-mask-dtype branch October 16, 2026 03:06
inureyes added a commit that referenced this pull request Oct 17, 2026
)

The four mixed bf16/fp16 defects that review of #2417 found. A bf16 model on an fp16 TurboQuant KV cache attends in `result_type(q, k, v)`, which is float32. Same-dtype attention is unchanged.

Closes #2422

## Changes

- **Chunk gate (CUDA).** New bridge helper `sdpa_attention_dtype(q, k, v)`, next to `sdpa_mask_for_inputs`. It reads the backend's promotion table. cuDNN eligibility and score row bytes now use it, and the decision is factored into `sdpa_query_chunk_within_budget(.., budget)`. bf16 over fp16 now chunks at head_dim 128 and is sized at 4 bytes per score at head_dim 256. The maskless causal path in `lib.rs` shares the gate.
- **Softcap composite (Gemma 2).** `compiled_softcap_sdpa` and `_gqa`, including the grouped decode branch, cast masks through `sdpa_mask_for_inputs` and return the attention dtype instead of q's and v's.
- **Overlay header.** `dtype.cpp` no longer says bf16 + fp16 gives bf16. The new test exposed one more fact, now noted there: the patched fold is order-dependent on CUDA, so `result_type(fp16, bf16, bf16)` is bf16. TurboQuant never produces that order.
- **Snapshot skip reason.** `snapshot_standard` and `snapshot_rotating` refuse a populated non-FP16 cache by mode before the keys/values pairing check. This also closes a silent gap. On main, a populated symmetric `turbo4` `KVCache` (keys and values both `None`, offset 8) returned `Ok` and stored nothing, which a probe test confirmed. Empty quantized layers (KV-shared) are still skipped. Every `kv_snapshot` user gets the fix: Gemma 3, AFMoE, Gemma 4, Llama 4 and Muse Glimmer. Qwen3-Next/3.5, LFM2 and Bailing already check the mode first. Jamba, Granite MoE Hybrid, Nemotron-H, Plamo2 and Falcon-H1 get no injected KV mode.

## Tests and revert check

New tests: `layers::tests::query_chunk_gate_sizes_mixed_half_kv_from_the_attention_dtype` (it also asserts the same-dtype decisions), `layers::tests::sdpa_attention_dtype_follows_the_backend_promotion_table`, `ffi_tests::test_compiled_softcap_sdpa_mixed_half_kv_attends_in_float32`, `models::kv_snapshot::tests::{populated_turbo_caches_are_refused_by_mode, empty_turbo_caches_are_skipped_not_refused}`, and `gemma4_unified_mask_tests::turbo4_snapshot_donation_names_the_mode` (through `Cache::snapshot_into`).

With the fixes reverted, the chunk-gate, softcap, populated-turbo and Gemma 4 tests fail (`None` vs `Some(16)`, output dtype fp16, `has only one of keys/values`). The softcap test's mask carries finite biases, so reverting only the mask cast also fails it, on the float32 comparison. The promotion-table test and the empty-cache test pass on main by design.

Run on GB10 (`--features cuda`, `--test-threads=1`, all passing): `-p mlxcel-core` `layers::tests` (102), `softcap` (18) and `ffi_tests` (112), plus root `models::kv_snapshot` (25), `models::gemma4` (121), `gemma3`, `gemma2`, `afmoe`, `llama4` and `muse_glimmer`. Clippy `-D warnings` (core and root `--lib --tests`), fmt and the `scripts/ci` checks are clean.

## Real checkpoints

All runs use `MLXCEL_SDPA_DETERMINISTIC=1` and temperature 0. "Before" is main `cadc6366`, and each before-arm showed self-parity.

- `mlxcel-server -m ~/models/mlx/gemma-4-26b-a4b-it-4bit --kv-cache-mode fp16+turbo4` with a 1987-token prompt and 300 tokens: coherent, and the response is byte-identical to before. The skip reason changes from `rotating cache has only one of keys/values` to `rotating cache mode Turbo4Asym is not supported by model-state snapshots`.
- `mlxcel generate`, 200 tokens: byte-identical before and after on `qwen3-0.6b-4bit`, and on `gemma-2-2b-it-4bit` with default KV and with `fp16+turbo4`. That checkpoint is fp16, so both of its arms are same-dtype.
- The mixed softcap path needs a bf16 Gemma 2, and every mlx-community Gemma 2 is fp16. A local bf16 conversion of `gemma-2-2b-it-4bit` (not committed) is byte-identical with default KV. With `fp16+turbo4` it diverges at character 645 and stays coherent, which is expected from float32 masked scores.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:inference Generation, sampling, decoding (incl. speculative, DRY) area:models Model architectures, weights, loading, metadata platform:linux Linux (CUDA / packaging) specific priority:medium Medium 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: Gemma 4 with fp16+turbo4 KV aborts in SDPA past the sliding window

1 participant