Repository navigation
Carry what the dropped GLM-5-Next, Qwen MTP and readahead pins had beyond upstream - #241
danielhanchen wants to merge 4 commits into
Conversation
The graph cache is keyed on cgraph->nodes[0] alone, so two evaluations that share a first node but differ in shape collide on one entry. Warmup needs two consecutive calls with unchanged node properties, so a workload whose batch shape varies resets warmup on nearly every call and falls back to eager launch. Speculative decoding is exactly that workload. The qwen4exp verify batch is distributed 2:13 percent, 3:11 percent, 4:75 percent as the accepted count varies, where qwen35 sits at 4:98 percent and is effectively constant. Host launch time for the qwen4exp target decode was 1.52 ms with the draft head disabled and 12.35 ms with it enabled, while GPU time was unchanged, so the regression was entirely host side. The key now mixes the first node, the last node and the node count. This is O(1) rather than a walk over every node: the existing uid early return fires on 127 of 128 decodes, so the hot path must not touch node data. An earlier all-nodes hash reintroduced exactly the per-node walk a CUDA graph exists to avoid. Measured overhead against the previous key is 0.2 to 0.6 percent, with both variants built into one binary to avoid comparing across runs. Capture churn on Qwen3.8-27B UD-Q2_K_XL drops from 52 captures and 50 destroys to 4 and 0, with identical output md5 and an unchanged speculative ratio. Across 14 distinct prefill shapes the cache instantiates 16 entries against 14 before, with no destroys and no growth, and is capped at 64 by LRU on top of the existing sweep. test-backend-ops passes 13646 of 13646 on CUDA0, and Llama-3.2-1B-Instruct Q8_0 is byte identical with no throughput change. (cherry picked from commit 5a08a71)
llama_prefetch_rows only issued madvise(MADV_WILLNEED) on Linux, so the macOS prebuilt read per-layer embedding rows (Gemma 4 PLE, the qwen4exp n-gram table) one page fault at a time. madvise and MADV_WILLNEED exist there with the same meaning. Carries the macOS half of #137.
The glm5-next indexer cache rows hold the compressor gates next to the keys, and the gates feed a softmax. Passing -ctk q8_0 through to that cache quantized them too. Keep it F16 then, as unslothai's earlier GLM-5-Next support did; qwen4exp rows carry no gate and are unchanged.
The NextN block was loaded but build_arch_graph threw for LLM_GRAPH_TYPE_DECODER_MTP. Build it: eh_proj([enorm(e) ; hnorm(h)]), then one DSA layer with a plain residual (the block has no mHC tensors), the MoE + shared expert FFN, and the head norm and head (nextn shared ones when present). It reuses the trunk's k-pool input and DSA layer; the MTP context memory already holds only the NextN block. h_nextn is the post-norm hidden state, the same kind the trunk exports.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b96a713a48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // a captured cuda graph hard-codes the shapes, so a caller that alternates shapes - a | ||
| // speculative verify batch, for example - needs a separate instance per shape. with a | ||
| // single key per split, every shape change resets the warmup and no graph is ever used. |
There was a problem hiding this comment.
These newly added lines hard-wrap sentences across several comment lines solely to fit a column width. Join each sentence onto one line; the repository explicitly prohibits fixed-width wrapping and splitting sentences mid-line because it makes reviews and future edits harder.
AGENTS.md reference: AGENTS.md:L74-L81
Useful? React with 👍 / 👎.
| // NextN draft head: one DSA layer with a plain residual (no mHC), fed by eh_proj([enorm(e) ; hnorm(h)]) | ||
| llama_model_glm5_next::graph_mtp::graph_mtp(const llama_model & model, const llm_graph_params & params) : | ||
| graph(model, params, no_build{}) { | ||
| GGML_ASSERT(hparams.n_layer_nextn == 1 && "GLM5-Next MTP has a single NextN block"); |
There was a problem hiding this comment.
When a GLM5-Next GGUF declares more than one NextN block and the user selects MTP drafting, this assertion terminates the entire process. This is reachable because conversion/glm.py preserves an arbitrary positive num_nextn_predict_layers value and the generic loader only checks that it does not exceed the total block count; reject unsupported counts with a recoverable model/graph error instead of GGML_ASSERT.
Useful? React with 👍 / 👎.
|
|
||
| res->t_embd = cur_out; | ||
|
|
||
| cur = ggml_mul_mat(ctx0, layer.nextn.shared_head_head ? layer.nextn.shared_head_head : model.output, cur_out); |
There was a problem hiding this comment.
When a loaded adapter targets blk.N.nextn.shared_head_head, the adapter loader accepts and associates those weights with this tensor, but the raw ggml_mul_mat bypasses the LoRA lookup, so the draft logits silently ignore that adapter. Use the existing build_lora_mm path, as the other MTP output heads do, so speculative predictions remain consistent with the adapted model.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
…ed heads (unslothai#242) Drop ggml-org#27754, unslothai#144 and unslothai#137: upstream merged GLM-5-Next (ggml-org#27773), Qwen4Exp MTP (ggml-org#29761) and row prefetch (ggml-org#29599), and the old pins no longer merge. Old-GGUF loading moves to unslothai#239, shared MTP heads to unslothai#240, and what the dropped pins had beyond upstream to unslothai#241. Repin ggml-org#24423, ggml-org#25731, unslothai#61 and unslothai#176 to their conflict-fixed heads.
This branch has not been deployed
1 participant
Summary
The nightly pin set drops three pins whose features landed upstream in a different form: ggml-org#27754 (GLM-5-Next, superseded by ggml-org#27773), #144 (Qwen3.8-Flash-Next MTP, superseded by ggml-org#29761) and #137 (lazy-table readahead, superseded by ggml-org#29599). The upstream versions do not carry everything those pins did. This PR brings the missing pieces back as four small commits on b11368 (
base/upstream-1fb7ef3e3), so the pin set can drop the old pins without losing speed.ggml-cuda: key the CUDA graph cache by shapellama: prefetch lazily read gather rows on macOS toollama_prefetchonly issuesmadvise(MADV_WILLNEED)on Linux and Windows; #137 also covered macOS. One-line platform guard change.llama: keep the glm5-next indexer cache F16 under a quantized -ctk-ctk q8_0quantized them too; keep that cache F16 as ggml-org#27754 did. qwen4exp is unchanged.glm5-next : NextN (MTP) draft graphLLM_GRAPH_TYPE_DECODER_MTP. This builds the draft graph (eh_proj, one DSA layer, MoE + shared expert, head norm and head), reusing the trunk's k-pool input and DSA layer.6 files, +153/-18. No new flags.
Testing
GLM-5.3-Flash UD-IQ1_S and Qwen3.8-Flash-Next UD-IQ1_S, one exclusive B200, interleaved before/after runs (before = last good nightly, b11160 + the old pins; after = b11368 + the refreshed pin set including this PR):
--spec-type draft-mtp)macos-15runner (M1, Metal) with gemma-4-E2B and-lzm on: greedy output is byte-identical with and without it.additive_merge.py) and builds with-DLLAMA_FATAL_WARNINGS=ON.