Repin qwen4exp (ggml-org#27742) to pick up the nextn draft head - #124
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1a0783f6b
ℹ️ 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".
| "https://github.com/unslothai/llama.cpp/pull/91/commits/c86ed269986f2dced6325c5c58bda966a2e2ead1", | ||
| "https://github.com/unslothai/llama.cpp/pull/95/commits/3db8cb5b2e9bf291057b9f19960e8601a162da81", | ||
| "https://github.com/unslothai/llama.cpp/pull/114/commits/c4ddc4805dbc12727897b354237bfd9225212b06", | ||
| "https://github.com/unslothai/llama.cpp/pull/114/commits/92ee7a5ce06ca4f8373dfec7de0a7729f391b7ff", |
There was a problem hiding this comment.
Include the required #108 repin
When this commit lands before #123, the manifest still uses 27278df700... for #108 even though the stated replay verification substituted 2c5b0007bc. The nightly merges pins sequentially and aborts on the first unresolved merge (.github/workflows/unsloth-prebuilt.yml:300-331), so on the current b10639 base it stops at #108 and never includes this new #114 head. Base this change on #123 or update #108 in the same manifest so the committed pin set is the one that was actually verified.
Useful? React with 👍 / 👎.
Second qwen4exp repin today.
#122pinnedc4ddc4805; the carry has moved on four commits since, and the upstream PR has gone from 27 to 30. The base tag is unchanged atb10639, so unlike#122this is purely a content refresh.scripts/unsloth/pr-set.json, one line:#114moves fromc4ddc4805to92ee7a5ce.What is in it
Three of the four new carry commits correspond one to one with the three new upstream commits:
qwen4exp: include llama-impl.h explicitly for llama_mul_mat_hadamardllama: fix the qwen4exp PLE history seq_rm(-1) iterator invalidation and the fatal-warning buildllama: segment the qwen4exp fused QKV for tensor splitThe fourth,
92ee7a5ce qwen4exp: multi-token prediction (nextn) draft head, is not in the upstream PR and has no counterpart there. It is 337 lines across 9 files, mostlysrc/models/qwen4exp.cpp(+235), and it adds two GGUF tensors to the existing NEXTN family:blk.{bid}.nextn.shared_head_downand..._up.I checked this rather than assuming, because it changes what a pin means. Below the nextn commit the carry is byte-identical to upstream
0b19188e9on every file the PR touches exceptsrc/llama-model.cpp, and that difference is entirely thefilter_attn/filter_recrreorder belowfilter_idxthat lets#118add its own without colliding. So: upstream-equivalent plus one reviewed-only-here feature.Verification
b10639in order:#107,#108,#70(additive),#91,#95,#114,#118.-DLLAMA_FATAL_WARNINGS=ON -DGGML_RPC=ON: 573/573, exit 0, zero warnings.test-llama-archsexits 0 with qwen4exp, glm5next and inkling registered.gguf-pyimports and exposes the new tensor enums.ca5d0a1dd1is still an ancestor, so#118composes unchanged, and it does.92ee7a5ceis in#114's commit list and mirrored torefs/pins/92ee7a5ce.One thing to decide
This probe used
2c5b0007bcfor pin#108, which is what#123proposes and master does not have yet. Merge#123first, or the nightly still stops at#108onb10639regardless of this PR.Worth being explicit that pinning
92ee7a5ceships the nextn draft head in the prebuilt binaries with no upstream review behind it, and that a qwen4exp GGUF converted with those two tensors present will not load in a stock build once#27742lands there. That is a call for whoever owns the feature, not a blocker for the pin; drop the last carry commit and repin atdbdcd49eaif the intent was upstream parity only.