add GLM-5.3-Flash (GLM5-Next) support - #27773
Conversation
106ece6 to
9370c82
Compare
|
Hey @timkhronos great work on the PR! A few requests if possible:
Tagging @ngxson for visibility as well. I re-checked and if (1) + (2) is applied, the quants we uploaded work fine (+ the small shard-1 rewrite) and KLD / PPL are correct under this PR. Seems like a simple alias isn't possible actually :( It breaks the quants made with this PR |
|
Hmmm https://github.com/timkhronos/llama.cpp/pull/9/changes would alias the tensors but it looks a bit problematic hmmm |
|
Throwing up some performance numbers here from the lower end of consumer hardware (128GB DDR5 + 24GB VRAM (4090)). avar6 has some freshly converted imatrix quants from this PR up as of now if anyone else wants to give them a go: https://huggingface.co/avar6/GLM-5.3-Flash-BF16-gguf For the IQ3_S, I am getting roughly 300t/s prefill at 256K context and 2048 b/ub size. Generation speed starts off at around 9t/s and drops down considerably by mid window (~128K) to around 6t/s. This seems to track with the 'pooled indexer keys' issue. The model is fully coherent and seems to be working fine. I don't have PPL/KL numbers at the moment as I still need to generate a logit dump. I have noticed an interesting memory quirk, which I haven't seen before. This is the only model I have ever seen have inconsistent checkpoint sizes. As the context fills the checkpoints grow alarmingly fast in size. At ~90K they are already up to nearly 1.6GB. I don't know if this is an expected behavior for this model arch, or if this is a something which needs to be looked into. Also, something of note for you @danielhanchen which I found last night while looking over the three PRs for this arch. The vision towers between this PR and yours differ as well. This PR reuses the name clip.vision.projector_type = "glm4v" while you built a new one clip.vision.projector_type = "glm5next". Likely not much of an issue given how easy it is to regenerate mmproj files, but it will need to be delt with as well. |
|
Yes I'll re-do the vision! This is fine! @timkhronos I confirmed timkhronos#9 works fine and does not break your GGUFs. We will however need to do a cheap shard-1 update so that should be fine |
…ed up long context decode, fla, and slight MTP improvements.
|
@timkhronos I saw you changed the tensor naming - but my solution I provided was to allow everyone's quants to work - now your own ones you uploaded don't work haha. We still need to provide the shard rewrite for the naming (glm5-next) which we're fine with, but now the DeepSeek convention means you yourself have to reupload all shards or do a tensor rename inplace with a script - was this your intention? |
|
@danielhanchen Hey! I ended up going with the the indexer_compressor naming scheme, as it is closer to what's already there, and I was meaning to ask Avar to reconvert anyways, as his ggufs were made when we were missing quantization protection for some crucial tensors so they are not ideal. Your vision projectors will need reconverting though most likely, and your main model ggufs might be missing the |
|
@timkhronos Hey! I made some shard rewrites to https://huggingface.co/unsloth/GLM-5.3-Flash-GGUF/tree/main/Shard_Rewrite for in preparation! |
| if (mem->get_mem_idx() != nullptr && mem->get_kpool() > 0) { | ||
| kpool_states.push_back(kpool_build_state(nullptr)); | ||
| } |
There was a problem hiding this comment.
It is not clear what this does when the ubatch argumetn is nullptr.
There was a problem hiding this comment.
That was building a layout only state for the full cache context, so get n kpools has something to report during graph reserve and state ops. I split it into an explicit kpool_build_layout, and kpool_build_state takes the ubatch by ref now.
| // Sequence edits invalidate cached relative pools. | ||
| bool kpool_is_dirty () const { return kpool_dirty; } | ||
| void kpool_clear_dirty() { kpool_dirty = false; } |
There was a problem hiding this comment.
This is confusing. From the code, it looks like the kpools are owned by the llama_memory_hybrid_idx_context which lives only during the processing of the logical batch. They seem to be recreated for each logical batch - is this not correct? A sequence edit cannot occur inbetween ubatches of a logical batch. Or am I missing something?
Basically, it is not clear to me what is the lifetime of the kpools - try to clarify this.
There was a problem hiding this comment.
So basically, there are two things with different lifetimes. The per ubatch pool layout (kpool_state) is owned by the context and is rebuilt every logical batch.
But the pooled key values are not per batch, but are scattered into the idx_cache, and persist across batches, so each batch repools only the pools it's ubatches touch. Sequence edits between batches shifts the pool grid, which invalidates the cached values, but without touching the layout, and that is what the flag on the memory tracks. It is cleared only once the first ubatch of the next batch succeeds. I renamed it to kpool_cache_stale and split the state builder so the layout only build is explicit. Hope it reads a bit clearer this way.
There was a problem hiding this comment.
Just in case you forgot to push the commit - it's not visible atm.
There was a problem hiding this comment.
Sorry, It should be in 8c28939 now, I misscopied my Acctoken and didn't notice it rejected.
There was a problem hiding this comment.
Wouldn't it be clear to call kpool_cache_stale -> mem_idx_modified? Keep the "kpool" concept confined to the llama_memory_hybrid_idx_context.
| // Pools touched by this ubatch are re-pooled, shared cells cannot cache sequence relative pools. | ||
| const bool all_new = !st.cache_safe || (kpool_stale_batch && i_cur == 0); |
There was a problem hiding this comment.
This is where we use the information of kpool_stale_batch that tells us that the mem_idx was modified (for example due to seq_cp).
What is still not clear to me is what exactly is the impact to the kpool_state when this occurs. What would be the difference if we didn't keep track of kpool_stale_batch and why it would be incorrect?
(btw, I haven't read the paper about kpools, so I might be missing some concept about how they are supposed to work)
There was a problem hiding this comment.
The indexer itself scores pools of 4 tokens, and topk selects whole pools. Recomputing every pool over the full context each ubatch would cost O(n_ctx) per DSA layer, so as a performance optimization I cached the pooled key of each complete pool. It's the third slot in the indexer cache row key | gate | pooled of the pool's last member cell. Per ubatch, only pools that ubatch touches are repooled and rewritten, and everything else is gathered from cache.
The flag itself is what guards against incorrectness. Pool grouping is sequence relative, so seq_rm/seq_add can regroup every pool. e.g removing the head of a sequence shifts pos_min and re aligns all of them, all without the next ubatch touching any of those cells. So when mem_idx is modified, the cache pooled slots no longer describe the current grouping. Without the flag, we'd gather those stale pooled keys, and the indexer would score the wrong pools.
The per token key and gate stay valid, so the flag forces the first ubatch of the next batch to re-pool everything from them.
seq_cp is a bit different, since shared cells set cache_safe to false, which disables pool caching entirely. (One cell row can't hold two sequences differently pooled values), so everything is always repooled every ubatch while sharing is active.
Once cells become exclusive again, it triggers one full rebuild before the cache is trusted again.
So all in all, the pool cache itself is purely a speed optimization, and kpool_stale_batch is what it needs to stay correct.
There was a problem hiding this comment.
Thanks, this helps a bit. But I am still a little worried that the logic is not clear enough and either I am missing something, or there is an incorrect assumption. Is it possible to drop the pool cache optimization as a first iteration and simplify? We can consider to bring it back again - hopefully it would be clearer on the second pass.
There was a problem hiding this comment.
Hm... I can either strip it fully now, and re-add it in a follow up, or keep it and try to make the logic a bit more reviewable (comments, maybe a debug env var or ifdef that forces the pool caching logic off via all_new, so the two paths can be more closely compared).
I also did a quick test sweep at different context lengths, measuring the TG performance difference on my system, with pool caching disabled vs cached (I am heavily CPU offloaded, so I'd assume the delta is probably smaller on a fully GPU setup):
32k around 15% slower
64k around 24% slower
128k around 38% slower
256k around 57% slower
I can do whichever is preferred.
There was a problem hiding this comment.
I don't see what information the llama_memory_hybrid_idx_context persists. It is created on each batch, constructs some kpool states and then gets destroyed. Nothing ever remains permanently into the llama_memory_hybrid_idx. So it appears that there is no information being cached, and hence I don't understand why we need the kpool_cache_stale logic.
There was a problem hiding this comment.
Oh, I think I see where the confusion is coming from. So, the cached information isn't in the host side objects, but it's actual tensor data in mem_idx itself. Each indexer cache row is key | gate | pooled, and the pooled slot of a pool's last member cell holds that pool's pooled key. The graph writes it with ggml_set_rows into a view of get_k(il) and later batches read it back with ggml_get_rows via pool_cells. So it rides accross batches how K does.
The kpool_state structs in the context are only per-ubatch layout descriptions and are rebuilt and discarded on each batch. kpool_cache_stale guards the tensor data, as the pooled slots stay valid only while the sequence relative grouping is unchanged, so any seq_* edit flags them for a full re-pool on the next batch.
There was a problem hiding this comment.
So, the cached information isn't in the host side objects, but it's actual tensor data in mem_idx itself.
Ok, I understand now. Try to express this better in the code. For starts, rename bool kpool_cache_stale to mem_idx_stale.
There was a problem hiding this comment.
So basically, the full lifecycle is:
Per ubatch:
- cpy_k writes each new token's key and gate into it's indexer cache row with the pooled slot zero initialized
- kpool_build_state scans mem_idx and derives the current grouping, which cells form which complete pools of 4 per sequence. It marks only the pools this ubatch touches or completes as is_new, and during decode that's one pool every 4th token, zero otherwise.
- The graph computes pooled keys for just the is_new pools, and set_rows writes each into the pooled slot og that pool's last member row.
- All pools are then gathered in one ggml_get_rows via pool_cells, fresh ones just written, older ones from whatever batch last wrote them. The incomplete tail is never pooled or cached, just appended per token via tail_idxs pointing at the raw cells.
Upon invalidation:
- Any seq_* edit sets kpool_cache_stale on llama_memory_hybrid_idx
- The next batch's first ubatch treats every pool as is_new, the grouping is re derived from the edited mem_idx, and everything is recomputed from the still valid key | gate rows and the (possibly different) rep rows are rewritted. The flag is cleared once the ubatch succeeds, then normal operation is resumed.
So essentially the pooled slots are trusted only while the sequence relative grouping that produced them is unchanged, and pool cells is always derived from the current grouping, with old pooled slots orphaned, since a slot is only ever read through pool_cells.
There was a problem hiding this comment.
Thanks, it's clear now. Such mechanism would even be useful for regular KQ masks because currently we always compute the entire KQ mask from scratch on the host side and upload it to the device. Instead, we could be updating only the elements that have changed relative to the previous state which would reduce the H2D traffic on each ubatch. But it's much simpler to implement the logic without this optimization.
The idea is clear now. Correctness of the implementation would be a bit difficult to validate by just analyzing the code. So for now let's hope it works.
Have you tested the implementation with parallel sequences? And unified/non-unified cache enabled?
There was a problem hiding this comment.
Okay, I ran some of it before, but retested everything to make sure it still holds after the latest changes:
-
Unified with parallel works, pools are laid out per sequence, and any shared cell sets cache_safe = false, which disables pool caching entirely and repools everything each ubatch, and caching resumes once cells are exclusive again, with the stale flag forcing one full rebuild first.
-
Concretely, I also tested -np 4 wih --kv-unified, a few times with different prompts where each process produced coherent and correct output, and a few times with fixed seed and greedy sampling, same prompt in all slots across different length prompts, and all slots produced identical output.
-
Non unified multi sequence is explicitly unsupported for now, the kpool path asserts n_stream == 1, so it fails cleanly rather than misbehaving. I'd rather defer multi stream to a follow up if possible, rather than grow this PR.
-
For pool caching, I made a small local self check build, that recomputes every pool fresh each ubatch and compares against the cached pooled slot's in graph. Over a long llama-server session, with chunked prefil and repeated seq_rm + checkpoint restores, across several long decodes, the max relative divergence across all DSA layers stayed at the F16 roundtrip(~7e-8 max_rel on squared norms) floor, with zero spikes.
I also did the code renames/comments to make the whole thing a bit clearer still, pushed as 2b533e0
| kpool_state kpool_build_layout() const; | ||
| kpool_state kpool_build_state(const llama_ubatch & ubatch) const; | ||
| const kpool_state & kpool_cur() const; | ||
| std::vector<kpool_state> kpool_states; |
There was a problem hiding this comment.
Do we actually need to keep all kpool states? Seems like we only need the last state for every ubatch?
There was a problem hiding this comment.
The vector shape came from the fact that the state is per-ubatch data, but in reality I think nothing actually reads an earlier state, so the vector only ever served it's final element, Collapsed in fe3187d
| // Whether this context tracks k-pool states. | ||
| bool kpool_track = false; |
There was a problem hiding this comment.
This also seems redundant - no need to duplicate the information when it is directly available from the llama_memory_hybrid_idx * mem. Can be a helper method instead.
There was a problem hiding this comment.
It's not entirely a duplicate, since the flag was also serving as a batch context marker. The update context has mem set too and gets apply()d from kv_self_update, but carries no ubatches to build a state from, so deriving purely from mem would send it into kpool_build_state(get_ubatch()). Setting that flag only in the constructor was an implicit way of ensuring that. Turned it into a helper, with the condition explicit
This comment was marked as spam.
This comment was marked as spam.
|
I tried this PR over RPC. Machine 1 with 4x 9060 XT (GFX1200) and Machine 2 with Strix-Halo (GFX1151) + 7600 XT (GFX1102). All in all ~185GB VRAM to use. I tried the Quant Q3_XL-3.86bpw: At first i could not run it with ROCM or VULKAN. Edit: Just tested VULKAN with -b 256 and -ub 256 there it crashes also. `ROCM -b 1024 -ub 1024: On the ROCM RPC side: Vulkan -b 1024 -ub 1024: RPC side: /home/lunarbuntu/Downloads/llama.cpp-GLM5.3-Flash/ggml/src/ggml-rpc/ggml-rpc.cpp:1386: GGML_ASSERT(tensor->data >= buffer_start && tensor->data + tensor_size <= buffer_start + buffer_size) failed` |
|
@timkhronos Heads up, recent change to |
|
@CISC Thank you, adapted in 1b564d2. @ggerganov I ended up adding the multi stream support in ff6be95, so both --kv-unified and --no-kv-unified work properly now. |
…-org#27773 - add LLM_ARCH_GLM5_NEXT (glm5-next) with legacy glm5next alias normalized to new arch - add LLM_KV kpool/select_tail/index_share_mtp and LLM_TENSOR kpool gate/ape - extend hparams with indexer_kpool fields - add LLM_TYPE 320B_A18B and layer kpool tensors - model mapping for both glm5next and glm5-next to new class - model saver, context graph_max_nodes, graph swiglu clamp for GLM5_NEXT - deepseek4 templated mHC helpers for sharing with glm5-next (preserve with_zero_dep) - add src/models/glm5-next.cpp from PR with block_size fallback for legacy GGUF - replace old src/models/glm5next.cpp runtime with alias to new class - kv-cache get_stream/get_k_storage helpers - hybrid idx kpool state merge with checkpoint idx_row_size Source: timkhronos/llama.cpp GLM5.3-Flash 5c4bd50
…ng native ggml-org#27773 path - KDA head dim: primary KDA_HEAD_DIM, fallback SSM_STATE_SIZE - KDA gate lower bound: default -5.0f when missing (legacy semantics) - recurrent layout: prefer explicit RECURRENT_LAYERS, else native n_head_kv==0 if any zero present, else legacy FULL_ATTENTION_INTERVAL default 4
Native ggml-org#27773 with KDA_HEAD_DIM present but no KDA_GATE_LOWER_BOUND should keep -INFINITY (softplus branch). -5.0f applies only when KDA_HEAD_DIM was missing and fell back to SSM_STATE_SIZE (legacy GLM-5.3 format).
|
Data point for the pool-cache discussion (#discussion_r3914969359) and for anyone sizing this on a single 24 GB card + CPU offload. Setup: EPYC 9355P, 1.5 TB DDR5, 1× RTX 4090, CUDA build. 1. Decode vs depth, this PR vs #27754 (same weights and flags,
Prefill ~123 t/s on both, flat with depth. The gap is the O(n_kv) term the pooled-index cache removes; on a CPU-offloaded box the cache is what makes long context usable. 2. Current head (5c4bd50),
Speeds are the same (the CPU expert stream dominates here). The win of 3. Long context,
No degradation with depth and no sign of the repeat-collapse reported on the other branch; 4. A real agentic session on the same build at |
Overview
Add support for GLM -5.3-flash a 320B hybrid model, supporting both text and vision.
Additional information
Architecture
GLM 5.3 flash mixes 34 KDA linear layers with 11 DSA laters, with mHC and Deepseek style Moe. Most of the parts are already in llama.cpp so I reused whatever I could:
What I implemented new:
llama_memory_hybrid_dsa: recurrent state + DSA cache, cloned from hybrid ISWA.Rebased onto llama_memory_hybrid_idx instead of the earlier ISWA clone.the encoder is the same family as glmv4 with per head qk-norm, clamped Swiglu and no post conv norm. It reuses glm4v projector with a swiglu_limit key and an optional image token budget.Added as glm5v as GLM 5.3 Flash requires a different pre processing method than what glm4v uses.Tests
Limitations
Quantized GGUFs converted with this PR are available here.
Requirements