Repository navigation
unsloth: drop the pins upstream superseded and repin the conflict-fixed heads - #242
Conversation
…ed heads Drop ggml-org#27754, #144 and #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 #239, shared MTP heads to #240, and what the dropped pins had beyond upstream to #241. Repin ggml-org#24423, ggml-org#25731, #61 and #176 to their conflict-fixed heads.
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 664babefa4
ℹ️ 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".
| "owner": "unslothai#239", | ||
| "checks": [ | ||
| { "kind": "arch", "arch": "glm5next" }, | ||
| { "kind": "arch", "arch": "glm5-next" }, |
There was a problem hiding this comment.
Preserve the legacy GLM-5-Next architecture probe
When unslothai#239 is needed to keep existing GGUFs whose general.architecture is glm5next loadable, replacing this probe with glm5-next exercises only the new upstream path. A regression in the compatibility path added by #239 could therefore ship while this feature remains green; keep a separate glm5next check alongside the upstream-name check.
Useful? React with 👍 / 👎.
| }, | ||
| "qwen4exp-mtp": { | ||
| "owner": "unslothai#144", | ||
| "owner": "unslothai#240", |
There was a problem hiding this comment.
Exercise the shared Qwen MTP-head path
When #240 is responsible for making the shared MTP sidecars work, assigning it this feature check provides no coverage for that behavior: test-llama-archs -a qwen4exp constructs and decodes one synthetic model, without loading a target plus shared sidecar or creating an MTP speculative context, while the TOPK_QSA probe tests only the isolated operator. A regression in #240's shared-weight loading or borrowing path can therefore leave this feature green; add an integration probe for the shared head or record the pin as unchecked instead.
Useful? React with 👍 / 👎.
Summary
The last three nightly runs of
unsloth-prebuilt.ymlfailed at "Resolve tag": since upstream merged its own GLM-5-Next and Qwen4Exp MTP, 7 of the 13 pins no longer merge onto a current base tag. This updatespr-set.jsonandfeature-checks.jsonso the full set merges again on b11368, b11404 and b11417 (the newest tag).Dropped
What those pins had beyond upstream is not lost:
glm5nextarch,swiglu_limitmmproj) next to the new upstream naming, so nobody has to re-download.mtp-*-shared-*.gguf) and the original self-contained heads.-ctk, and the macOS half of llama: batched readahead for lazily read gather tables #137's readahead.On #137 specifically: interleaved cold-cache runs on b11368 (Qwen3.8-Flash-Next UD-IQ1_S, lazy table) show upstream's readahead and #137 at the same throughput and the same major-fault count (95 vs 96, against 10,815 with neither), with byte-identical greedy output.
Repinned to conflict-fixed heads
3dac51dc8643189144(merges b11417)3870aa48befd2b13f8(merges b11417, opts Inkling out of mixed token/embd batches added by ggml-org#29622)46cbf0e95ef45f212c(merges b11368)09ce1a4d2e2519ebc0(merges b11368)#70, #95, #149, #152, #157 and #158 are unchanged.
feature-checks.json
glm5next: owner Load GLM-5-Next GGUFs converted with the earlier glm5next arch name #239, arch probe renamed to upstream'sglm5-next.qwen4exp-mtp: owner qwen4exp: run MTP heads that share the target's token_embd and output #240.unchecked; Carry what the dropped GLM-5-Next, Qwen MTP and readahead pins had beyond upstream #241 added there with its reason.Testing
additive_merge.py: every pin merges on b11368, b11404 and b11417; Add TML Inkling architecture ggml-org/llama.cpp#25731 and kimi-k3 : the MoonViT-3d vision tower and full-size loading fixes #70 need the additive resolver, as before.-DLLAMA_FATAL_WARNINGS=ON.test-llama-archspasses all 374 tests andtest-mtmd-implpasses.feature_matrix.py(CPU, as in CI) demonstrates all 7 features.Shard_Rewriteshard 1 + mmproj.