Skip to content

Add MTP support for Step 3.5 Flash - #901

Closed
janhilgard wants to merge 1 commit into
ml-explore:mainfrom
janhilgard:feat/step3p5-model
Closed

Add MTP support for Step 3.5 Flash#901
janhilgard wants to merge 1 commit into
ml-explore:mainfrom
janhilgard:feat/step3p5-model

Conversation

@janhilgard

Copy link
Copy Markdown

Summary

  • Add MTP (Multi-Token Prediction) speculative decoding support to the existing Step 3.5 Flash model
  • 3 MTP prediction layers with per-layer shared_head (dense MLP, not MoE)
  • Backward compatible — MTP only instantiated when num_nextn_predict_layers > 0

Changes

Component Description
Step3p5SharedHead Per-layer prediction head: norm + output projection
Step3p5MTPLayer hnorm + enorm → eh_proj → sliding attention + dense MLP → shared_head
Step3p5MTP Container for multiple prediction layers
Model.__call__ Added return_hidden parameter for MTP integration
Model.mtp_forward Single-step MTP prediction (hidden + next token → logits)
Model.make_mtp_cache KV cache factory for MTP layers
sanitize Handle MTP weight loading (instead of filtering)
quant_predicate Exclude MTP norm layers from quantization

Also fixes two pre-existing ruff E741 lint warnings (llayer in generator expressions).

MTP Architecture

hidden_states ──→ hnorm ──→ ┐
                             ├──→ concat [e, h] ──→ eh_proj ──→ attn + MLP ──→ shared_head ──→ logits
next_token_ids ──→ embed ──→ enorm ──→ ┘

Each MTP layer uses sliding attention with dense SwiGLU MLP (not MoE), matching the original StepFun architecture.

Test plan

  • Basic inference without MTP (backward compatibility)
  • return_hidden=True returns prenorm hidden states
  • mtp_forward() produces correct-shaped logits
  • make_mtp_cache() returns KVCache list matching MTP layer count
  • ruff format and ruff check pass cleanly
  • End-to-end test with mlx-community/Step-3.5-Flash-4bit (111 GB, requires Apple Silicon)

🤖 Generated with Claude Code

janhilgard added a commit to janhilgard/vllm-mlx that referenced this pull request Feb 16, 2026
Step 3.5 Flash is a 196B MoE model (288 experts, top-8 routing, ~11B
active params) with 3 MTP prediction layers. The MLX community 4-bit
conversion strips MTP weights and lacks MTP-aware modeling code.

This adds:
- scripts/add_mtp_weights_step3p5.py: Downloads BF16 MTP shards from
  the original model, extracts layers 45-47, remaps to mtp.layers.*,
  quantizes to 4-bit, and installs the MTP modeling file
- scripts/modeling_step3p5_mtp.py: Full MLX-native model implementation
  with MTP support (Step3p5MTP, Step3p5MTPLayer, Step3p5SharedHead)
- Reasoning parser alias "step3p5" (reuses deepseek_r1 <think> parser)
- Documentation updates in README.md and docs/reference/models.md

Note: The custom modeling file is a workaround until
ml-explore/mlx-lm#901 is merged upstream.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Step 3.5 Flash ships MTP weights, but `sanitize()` dropped every `.mtp` key,
so the head was discarded at load and speculative decoding had nothing to
draft with. This adds the head and the plumbing to use it:

- `Step3p5MTPLayer`: eh_proj over the concatenated normalized hidden state and
  next-token embedding, then a decoder block. It borrows a real layer's
  attention config rather than inventing one, picking the first
  `sliding_attention` entry in `layer_types` so RoPE and the attention
  settings match what the checkpoint was trained with.
- `Step3p5SharedHead`: per-MTP-layer norm plus output projection.
- `mtp_forward()` / `make_mtp_cache()`, and `sanitize()` keeps the MTP weights
  instead of dropping them.

Rebased onto current main. ml-explore#949 landed in the meantime and gave sliding
attention layers a `RotatingKVCache`, which the MTP cache has to follow: the
MTP layer runs sliding attention, so a plain `KVCache` there grows without
bound across a generation while the main model's equivalent layers stay capped
at `sliding_window`. `make_mtp_cache()` now keys off `self_attn.is_sliding`
rather than assuming, because which layer it borrows depends on `layer_types`.

Covered by `test_step3p5_mtp_cache_follows_the_main_model_policy`, which
asserts the class, the window, and that the cache actually stays bounded over
4x sliding_window steps — a plain `KVCache` fails it. `tests/test_models.py`:
78 passed, 1 skipped, 52 subtests.
@janhilgard

Copy link
Copy Markdown
Author

Rebased onto current main and refreshed. Status update, since this has been sitting a while and the ground has moved under it.

The base model landed upstream in the meantimemlx_lm/models/step3p5.py is in 0.31.3. MTP still is not: there is no Step3p5MTPLayer or Step3p5SharedHead, and sanitize() still drops the head outright:

for k, v in weights.items():
    if ".mtp" in k:
        continue

So the checkpoint ships MTP weights that are discarded at load, and speculative decoding has nothing to draft with. This PR is not stale — it just never got picked up.

One real conflict the rebase surfaced, and it was semantic rather than textual. #949 gave sliding attention layers a RotatingKVCache. The MTP layer runs sliding attention (it borrows the first sliding_attention layer's config so RoPE matches the checkpoint), but make_mtp_cache() was handing it a plain KVCache. The rebase applied cleanly and hid that completely — the MTP cache would have grown without bound across a generation while the main model's equivalent layers stayed capped at sliding_window.

Fixed by following the same policy, keyed off self_attn.is_sliding rather than assumed, since which layer the head borrows depends on layer_types:

return [
    RotatingKVCache(max_size=self.args.sliding_window)
    if layer.self_attn.is_sliding else KVCache()
    for layer in self.mtp.layers
]

test_step3p5_mtp_cache_follows_the_main_model_policy covers it — class, window size, and that the cache actually stays bounded over 4× sliding_window steps, plus the full-attention config keeping a plain KVCache. A plain KVCache fails it, which I checked by reverting the change.

tests/test_models.py: 78 passed, 1 skipped, 52 subtests.

One caveat I would rather state than have found in review: I no longer run Step 3.5 Flash, so this round is structural verification only — I cannot re-measure MTP acceptance against real weights the way the original numbers were taken. The cache fix is checked against behaviour, not just types, but if you want live acceptance figures before merging, someone with the model would need to take them.

@janhilgard

Copy link
Copy Markdown
Author

Closing this — not because it went stale, but because I can no longer stand behind it.

State as of today, for anyone who finds this later:

  • The base model landed upstream in the meantime; mlx_lm/models/step3p5.py is in 0.31.3.
  • MTP is still not there. There is no Step3p5MTPLayer or Step3p5SharedHead, and sanitize() still drops the head outright (if ".mtp" in k: continue), so checkpoints ship MTP weights that are discarded at load and speculative decoding has nothing to draft with.
  • The branch is rebased onto current main and green: tests/test_models.py 78 passed, 1 skipped, 52 subtests.
  • The rebase surfaced a semantic conflict worth recording. step3p5: use rotating cache for sliding attention layers #949 gave sliding-attention layers a RotatingKVCache, and the MTP layer runs sliding attention — it borrows the first sliding_attention layer's config so RoPE matches the checkpoint — while make_mtp_cache() was still handing it a plain KVCache. Textually the rebase was clean, so nothing flagged it; the MTP cache would have grown without bound across a generation while the main model's equivalent layers stayed capped at sliding_window. Fixed by keying off self_attn.is_sliding, with a regression test that checks the cache actually stays bounded rather than just having the right class.

What I cannot do is the part that matters for merging it: I no longer run Step 3.5 Flash, so I cannot re-measure MTP acceptance against real weights. The original numbers came from a live deployment that no longer exists, and shipping a speculative-decoding path whose acceptance rate nobody can currently verify is not something I want to advocate for.

The branch stays up. If someone with the model wants to pick it up, the code and tests are here and the cache fix above is the non-obvious part.

@janhilgard janhilgard closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant