Carry unslothai#116 (GLM-5-Next) composed onto the qwen4exp b10632 carry - #118
Conversation
The PR branches off fork master, so its head carries 77 fork CI paths (.github/ and scripts/unsloth/, including pr-set.json itself) that have no business in a release tree. This is b10632 with the PR's 42 paths applied and nothing else. Squashed rather than replayed: the PR head is a merge commit, and the 20 commits are on the PR for review. What matters for a pin is that the tree is exact, which is checkable and checked: the diff against b10632 is exactly the PR's 42 paths, and those 42 are byte-identical to the PR head.
Three changes, none of them behavioural:
llama-model.cpp declare filter_idx/type_idx above the other filters, off
the line the inkling arch edits, and give GLM5NEXT its
own else-if instead of sharing the qwen condition that
qwen4exp also appends to
llama-kv-cache.cpp park the two context accessors with type_v rather than
at the get_n_kv anchor inkling also appends to
tensor_mapping.py put the GLM5NEXT map at the end of the dict rather than
where qwen4exp's lands
The last two are pure reorderings, asserted as such (identical multiset of
lines). Without these the nightly hits conflicts additive_merge.py cannot prove
safe, and the whole build stops.
These two pins are not independent: both add a sparse-attention indexer cache and both declare the same `filter_idx` in create_memory, so merging them side by side would declare it twice. No resolver rule can fix that safely, and the nightly would stop on it. Building glm5next's carry on top of qwen4exp's makes the composition explicit and resolved once, here, rather than attempted nightly. One declaration is kept, with each arch's own extra alongside it: needs_mem_idx for qwen4exp, type_idx for glm5next. Everything else was a pure add/add union. Consequence worth knowing: dropping the qwen4exp pin no longer drops its code, because this carry contains it. Drop both together, or rebuild this carry.
![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: 3766b41229
ℹ️ 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".
| const int64_t n_kv = kbuf->ne[2]; | ||
| const int64_t n_stream = kbuf->ne[3]; | ||
| const int64_t n_tps = n_tokens/n_stream; |
There was a problem hiding this comment.
When a non-unified server batch contains non-contiguous sequence streams, such as active sequence IDs 0 and 2, get_k() exposes the entire stream range (3 streams), while build_inp_kpool() deliberately packs pool_cells and masks using ubatch.n_seqs_unq (2 streams). Deriving n_stream from kbuf->ne[3] therefore makes n_tokens/n_stream truncate and triggers the assertion at line 456 during ordinary two-sequence decoding; if the division happens to be exact, the subsequent gathers still associate the packed pool maps with the wrong cache streams. Use the packed input/mask stream count and explicitly map those streams into the cache view.
Useful? React with 👍 / 👎.
| // cannot reach some vocab entries: " 王" (Ġçİĭ, 102322) needs (Ġ,çİĭ)=242943 | ||
| // but (Ġ,ç)=27944 wins first, so it stops three tokens short. triggered by |
There was a problem hiding this comment.
This new comment includes a Chinese character and several non-ASCII tokenizer glyphs, contrary to the repository's explicit requirement that code avoid Unicode characters. Rewrite the example using ASCII descriptions or byte/code-point notation.
AGENTS.md reference: AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
2 participants
Carries #116 (GLM-5-Next / GLM-5.3-Flash) so the nightly prebuild can pin it.
Based on
qwen4exp-27742-b10632, the #114 carry, not onb10632directly. That is deliberate and is the main thing to review here.Why it is not pinned straight from #116
glm5next/publicbranches off fork master, so its head carries 77 fork CI paths (.github/andscripts/unsloth/, includingpr-set.jsonitself) that have no business in a release tree. This branch isb10632with the PR's 42 paths applied and nothing else. Those 42 are byte-identical to the PR head, and the diff againstb10632is exactly those 42 paths, both asserted.Why it sits on top of the qwen4exp carry
These two pins are not independent. Both add a sparse-attention indexer cache, and both declare the same
filter_idxincreate_memory. Merged side by side, the union declares it twice and the build stops; no resolver rule can fix that safely, and it is not something the nightly should be attempting unattended.So the composition is resolved once, here: a single
filter_idxdeclaration carrying each arch's own extra next to it,needs_mem_idxfor qwen4exp andtype_idxfor glm5next. Everything else was a pure add/add union.Consequence: dropping the qwen4exp pin no longer drops its code, because this carry contains it. Drop both together, or rebuild this carry against whatever remains.
Decoupling commit
One commit exists purely so the set composes, and changes no behaviour:
llama-model.cpp- declarefilter_idx/type_idxabove the other filters, off the line the inkling arch edits, and giveGLM5NEXTits ownelse ifrather than sharing the qwen condition that qwen4exp also appends tollama-kv-cache.cpp- park the two context accessors withtype_vrather than at theget_n_kvanchor inkling also appends totensor_mapping.py- put theGLM5NEXTmap at the end of the dict rather than where qwen4exp's landsThe last two are pure reorderings, asserted as such: identical multiset of lines before and after.
Verification against
b10632#70viaadditive_merge.pyLLAMA_BUILD_TESTS=ONtest-llama-archsexits 0 withglm5nextOK andqwen4expOK, and inkling's documented skip intactfilter_idxdeclarations that remain increate_memoryare different types in disjoint branches (llama_kv_cache::andllama_memory_hybrid::); the compile is what settles that, and it passesrefs/pins/3766b41229c20249fd4d83d7ba297499d50e9b80Do not delete this branch while it is referenced by
scripts/unsloth/pr-set.json.