unsloth: pin ggml-org#27941 so the qwen4exp fixes reach the nightlies - #148
Closed
danielhanchen wants to merge 1 commit into
Closed
unsloth: pin ggml-org#27941 so the qwen4exp fixes reach the nightlies#148danielhanchen wants to merge 1 commit into
danielhanchen wants to merge 1 commit into
Conversation
We ship Qwen3.8-Flash-Next GGUFs and the qwen4exp path in the nightlies still has the defects this fixes. The fixes exist, in two places, and neither of them currently reaches a user. ggml-org#27941 is a strict superset of our own #143: same four files, nine more lines in llama-memory-hybrid-idx.cpp, plus a llama-kv-cache.cpp change #143 does not carry. Both are mine. Pinning the upstream one means one copy to maintain and a clean retirement once a base tag contains it; #143 is closed as superseded. Scope is narrow. llama_memory_hybrid_idx is reached from exactly one model, so nothing outside qwen4exp can regress on this pin, and the defects are backend independent, which is what explains the reporter who saw the same failure on Vulkan where the ROCm-specific issues do not apply. Open and MERGEABLE upstream at the pinned commit. Drop this entry once a base tag carries the work.
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. |
danielhanchen
added a commit
that referenced
this pull request
Aug 31, 2026
* unsloth: pin ggml-org#27941, #152 and #154 ggml-org#27941 at 02eb201, no longer a draft and MERGEABLE upstream. It fixes four qwen4exp correctness defects and is the only account of the Flash-Next reporter who is on Vulkan only, where the allocator variable is never set. The previous pin PR #148 carried the stale 8161d11 and is closed. #152 at 258345e and #154 at 31e432e both merge onto b10708 on their own. #137 is already pinned at 4e1865e, which is still its head, so no change. #142 and #144 are NOT pinned here: both predate an upstream refactor that folded lazy_mode and model_shared into a lazy struct, so they conflict on src/llama.cpp and src/llama-model-loader.cpp against b10708. b10708 has 'ml.lazy.mode = params.lazy_mode' where those branches still write 'ml.lazy_mode' and 'ml.model_shared'. They need rebasing onto current upstream before they can be pinned; pinning them now would fail the resolve. * unsloth: repin ggml-org#27941 to 6b2b85c The PR moved on at 10:44Z, after 02eb201 was pinned. Verified to merge onto the current base tag: 6 files changed, 480 insertions, 138 deletions. * unsloth: pin #144, rebased, and leave #142 out as contained in it #144 rebased onto b10709 at 6fc8df1. Two of its nine commits were dropped rather than carried: 'qwen4exp: fix QSA correctness defects and harden metadata loading' and the comment tidy on top of it. That work is what ggml-org#27941 supersedes, and keeping both copies is what made the two pins conflict in llama-kv-cells.h and llama-memory-hybrid-idx.cpp. Listed after ggml-org#27941 so the upstream version of that work lands first. #142 is deliberately NOT pinned: its single commit is byte-identical to #144's c7bd6f2 apart from the lazy API spelling, so #144 already contains it and pinning both would apply the same change twice. Verified: b10709 + ggml-org#27941 + #144 merges clean, 24 files, +612/-94. * unsloth: repin ggml-org#27754 to 949f7ef The PR moved on at 10:39Z; 5796547 was pinned by #159 earlier today. Verified to merge onto b10709: 44 files changed, 2673 insertions, 38 deletions. Every entry in the set is now at its PR's current head. --------- Co-authored-by: danielhanchen <elliegouldingstuff@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Pins ggml-org#27941 into the nightly build set.
We ship Qwen3.8-Flash-Next GGUFs, the qwen4exp path in our nightlies still has the defects this fixes, and the fixes exist in two places, neither of which currently reaches a user. #143 sits on this fork unmerged; ggml-org#27941 sits upstream unmerged; the nightly tree is the upstream tag plus
scripts/unsloth/pr-set.json, so it has neither.Why the upstream one and not #143
ggml-org#27941 is a strict superset of #143:
src/llama-kv-cells.hsrc/llama-memory-hybrid-idx.cppsrc/llama-memory-hybrid-idx.hsrc/models/qwen4exp.cppsrc/llama-kv-cache.cppBoth are mine. Pinning the upstream one means one copy to maintain, and the entry retires cleanly once a base tag contains the work rather than becoming a conflicting re-application of code the base already has. #143 is closed as superseded.
Pinning both is not an option; they would conflict on every shared line.
Blast radius
Narrow.
llama_memory_hybrid_idxis reached from exactly one model, so nothing outside qwen4exp can regress on this pin. The defects are backend independent, which is what accounts for the reporter who hit the same failure on Vulkan, where the ROCm-specific problems tracked in #147 do not apply.Open and
MERGEABLEupstream at the pinned commit8161d11. Drop the entry once a base tag carries it, per_doc.Note on ordering
This and #147 both append to the end of
prs, so whichever merges second needs a one-line rebase. They are independent decisions and I would rather they be judged separately than bundled.