qwen4exp: add Qwen3.8-Flash-Next support - #111
Conversation
Adds the GGUF-side plumbing for HF model_type qwen4_exp: - MODEL_ARCH.QWEN4EXP plus tensors for the low-rank hyper-connection variant (hc_*_norm/down/up/inject) and the PLE n-gram hash embeddings. The DeepSeek-V4 hc_*_fn/base/scale tensors are a different parameterisation, so these are separate entries rather than reuse. - Reuses the existing indexer, per_layer_token_embd, SSM and compress_ratios keys unchanged. - conversion/qwen4exp.py inherits the Qwen3.5 linear-attention V-head reorder and interleaved mrope, concatenates the 128 PLE embedding shards, and splits index_qk_proj into separate indexer q/k tensors. The PLE hash multipliers reach ~2.4e13. prepare_tensors() casts every non-float dtype to float32 before modify_tensors() runs, and GGUF array writes infer INT32 from Python ints, so both paths are bypassed: the constants are read from the pre-cast lazy tensors and written as explicit UINT64 arrays. Additive only; no existing arch changes behaviour.
Adds LLM_ARCH_QWEN4EXP with its hparams and tensor loading. The graph comes in the next commit; this makes the model load and report correct metadata. - hyper-connections set n_embd_out_impl = hc_count * n_embd, so the residual stream is 4x wide and there is no output_norm: the final mixer's hc_norm is the last norm in the model. - registered as hybrid and given the same recurrent/attention memory filters as Qwen3-Next and Qwen3.5. - reuses the existing indexer, per_layer_token_embd, SSM and compress_ratios keys as-is. - the PLE table row count is read back from the file rather than recomputing the vocab padding rule. llama-model-loader gains UINT64 array support. That branch previously threw, so no existing caller changes behaviour; it is needed because the PLE hash multipliers do not fit in int32.
Implements the decode graph for Qwen3.8-Flash-Next: the hyper-connection residual stream, gated delta net layers, the MoE block with its gated shared expert, and dense full attention. The QSA indexer and the PLE n-gram embedding are not wired up yet and land in later commits. Hyper-connections are implemented here rather than shared with deepseek4.cpp. The two formulations agree on the [n_embd, hc, n_tokens] layout and little else: DeepSeek-V4 mixes with a full-rank projection and Sinkhorn-normalises it, whereas this model uses a low-rank down/silu/up sigmoid gate and collapses by a plain mean. Only the ~10 line stream mean is genuinely common, so sharing would mean touching DSV4's hot path and its three fused CUDA ops to reuse very little. What is reused is the substantive part: the LLM_KV_HYPER_CONNECTION_* keys, the n_embd_out_impl wide-residual support already in the loader, and the layout convention. Also allows a checkpoint to carry no PLE layers at all, which makes it possible to bring the graph up and validate it in stages. Validated against vLLM, the only working reference implementation. On a scaled-down model with an init scale large enough to give non-uniform logits, agreement with vLLM sits at the numerical noise floor: llama.cpp f32 against its own bf16 gives 84.3% top-1 agreement over 255 positions, and this graph against vLLM gives 85.1%. The comparison was calibrated by seeding three deliberate bugs (silu instead of sigmoid on the delta net gate, dropping the 1/hc scale in the mix, dropping the 2x in the combine); each drops top-1 to between 0% and 11%, an order of magnitude below the floor.
Adds the per-layer embedding: a custom I32 graph input hashes each token with its ngram_size-1 predecessors host-side and the result is a plain row gather over the shared table, the same shape gemma3n's per-layer embedding uses. The hash has to run on the host because the splitmix64-derived multipliers reach 2^45, so the products need 64-bit integers and an xor, neither of which ggml has. Predecessors that fall outside the ubatch come from a small per-sequence history on the model, mirroring the per-request ngram_context the reference carries. It is only trusted when contiguous with the incoming position, so a fresh prompt or a rewound cache falls back to EOS padding rather than hashing against stale tokens. The depthwise conv is written out as a sum of shifted, per-channel-scaled copies rather than through ggml_conv_1d_dw, which carries a correctness warning upstream. Verified two ways. The row indices match a transcription of the reference's tensor formulation exactly, 1024 of 1024 rows, including sequences with EOS tokens sprinkled through them to exercise the segment reset. Separately, with PLE placed on layer 0 so its input is just the token embedding, ple_embd and ple_gated_value match a PyTorch computation from the same checkpoint to every printed digit. End to end over 1023 scored positions the port sits the same distance from vLLM with PLE as without it, 6.3 points of top-1 against 6.0, so PLE costs no accuracy relative to the rest of the model. That common offset is vLLM's bf16 activations, which cannot be removed: its QSA kernel refuses float32. Two bugs found along the way, both caught by the row-index check. The history was read and updated in the same pass, so a token early in a ubatch could pick up an earlier token of that same ubatch as prior context; it is now snapshotted first. And an EOS token was cutting its own context, where the reference takes the last EOS strictly before the position, so a boundary only hides tokens from the positions after it. Known gap: the conv carries no state across ubatches, so it is exact only for a prefill that starts at position 0. Chunked prefill and decode need the conv state wired into the recurrent memory, and the conv branch itself is still numerically unverified because the fixture zeroes its weights.
The PLE depthwise conv was zero-padding on the left, which is only right for a prefill that starts at position 0. Decode and chunked prefill saw a truncated history for the first (kernel-1)*ngram_size positions of every ubatch. The PLE module sits on a layer that is also a delta-net layer, so both need a conv history in the same recurrent row. Rather than plumb a per-layer state size through build_rs and build_conv_state, the row is widened once and each convolution addresses its own slice through a local helper. n_embd_r() gains the extra span, which is zero for every other architecture because it is derived from ple_n_heads. Verified by feeding the same 1024 token sequence in chunks instead of one shot: at 64 tokens per decode the logits are bit-identical to the single-shot run, 1023 of 1023 top-1 and a maximum logprob deviation of exactly zero. At one token per decode they differ slightly, but the no-PLE model differs more under the same test (94.6% against 97.1%), so that is the usual gemv-versus- gemm accumulation difference and not the state. The conv branch is also no longer unverified. With non-zero conv weights the port sits 6.3 points of top-1 below the numerical floor, the same distance as with the weights zeroed and as the model with no PLE at all, so the branch adds no error of its own. test-llama-archs passes every existing architecture at 0.00e+00, including the delta-net models that share this code path.
build_rs writes into the state tensor in place, zeroing one row and copying the carried-over states, so calling it twice for the same layer let the second call clobber the first write-back. The PLE layer is also a delta-net layer, so that is exactly what happened: both convolutions gathered the same row. They now share a single gather per layer. The earlier claim that the conv state was carried correctly was tested on a fixture whose conv weights are zero, where the branch contributes nothing and chunking matches trivially. Re-running with non-zero conv weights showed the divergence, growing with the number of ubatch boundaries: 97.1% top-1 at one boundary down to 90.2% at seven. With the shared gather it is bit-identical to the single-shot run at every chunk size tried, 512, 128 and 64, with a maximum logprob deviation of exactly zero over 1023 positions. The delta-net-only model stays bit-identical too, so nothing regressed there. Also derive the delta-net conv channel count the way load_arch_tensors sizes wqkv instead of from ssm_d_inner. The two agree for this model, but n_embd_r() only bounds the row and the convolution has to match the tensor feeding it. test-llama-archs previously aborted on this architecture and took every later architecture with it. qwen4exp is marked MoE-only, given the hyper-connection keys and an ssm_d_inner consistent with its tensor derivation, and skipped for now: the hyper-connection keys written by get_gguf_ctx are not reaching the synthesised file, which needs a separate look. The suite completes again, 124 architectures at 0.00e+00.
Groundwork for qwen4exp's QSA sparse attention. Its indexer needs a per-token key history for the full-attention layers, but a hybrid model cannot use llama_kv_cache_dsa: that class derives from llama_memory_i rather than llama_kv_cache, and llama_memory_hybrid constructs its attention cache directly. No existing architecture pairs recurrent state with a sparse indexer, so there was nothing to reuse wholesale. llama_memory_hybrid therefore gains a third, optional cache, shaped the same way llama_kv_cache_dsa shapes its lightning-indexer cache: a copy of hparams with n_head_kv forced to 1 and n_embd_head_k_full set to indexer_head_size. It is built only when a filter_idx callback is passed, which defaults to nullptr, so every existing architecture gets exactly what it got before. The per-sequence operations and the batch preparation forward to it under a null check, matching how the DSA cache prepares its two caches over the same ubatches. test-llama-archs passes all 124 architectures at 0.00e+00, including the 12 in the hybrid family that share this code. The qwen4exp fixtures are unchanged: same logits against vLLM, and chunked evaluation still bit-identical to single-shot.
The full-attention layers of this model do not attend to everything. An
indexer scores one mean-pooled key per block of compress_ratio tokens and
keeps a budget of the best blocks, plus the tail of tokens that do not yet
form a complete block. Below indexer_top_k + compress_ratio - 1 cached
tokens every block fits in the budget, so the result is exactly dense.
What is reused rather than rebuilt:
- the mask machinery. build_attn's DSA overload already turns a list of
token indices into a KQ mask via ggml_set_rows, so that block is lifted
out verbatim into build_attn_mask_top_k and shared with a new overload
on llm_graph_input_attn_kv. DSA's node sequence is unchanged; the new
overload exists because llama_kv_cache_dsa assumes MLA and cannot be
dropped into a hybrid model.
- the indexer key cache, which is the optional third cache added to
llama_memory_hybrid in the previous commit. It holds raw keys, because
pooling happens before the norm and the rotation.
The graph expands block scores rather than block indices: giving every
token of a block its block's score needs only a gather, where expanding
indices would need an integer multiply-add that ggml has no op for. Since
the budget is a whole number of blocks and a block's members tie exactly,
the cut still lands on a block boundary.
Everything that depends on cache layout is computed host-side in
set_input_qsa. Blocks are cuts of the position line rather than of the cell
array, so nothing assumes the cache is contiguous.
Measured on the tiny fixture against vLLM, comparing the selected token
indices directly rather than the logits:
below the budget selection identical, and 1024-token logits are
bit-identical to the pre-QSA dense path
above the budget mean jaccard 0.975
The direct index comparison is what made this correct. The reference
rectifies each head's dot product before summing over heads, which an
earlier reading of it had missed; on logits alone the resulting port looked
fine, because on a randomly initialised fixture the known-correct dense
path already disagrees with vLLM by more than the bug did. Comparing the
indices showed 0.794, and fixing the ReLU moved it to 0.975.
The indexer cache found its own slots, independently of the attention cache. Both are the same size and see the same ubatches, so in a straight-through prefill they agree, which is why every fixture and every single-shot parity run passed. They drift once the context is being rewritten between turns, and then the QSA top-k indices, which are applied against the attention mask, point at the wrong cells. The seven-turn chat test caught it on the third turn: llama-server aborted on the assertion that the two caches report the same n_kv. The cache is a side buffer addressed by the attention cache's cells, so it now takes that cache's slot layout instead of computing one. Applying that layout also marks its cells identically, so the two agree cell for cell by construction rather than by coincidence, and the assertion can no longer fire. Inert where the caches already agreed: test-llama-archs green at 126 archs and 0.00e+00, and the 4096-token tiny fixture is unchanged at max logit delta 0.0.
The old note guessed that the hyper-connection keys never reach the file. They do: dumping the gguf_context handed to llama_model_init_from_user shows both among its 67 KVs, and the loader still reports one missing.
The arch was skipped with a note guessing that the hyper-connection keys
never reached the synthesised file. They did. The suite builds a model, then
saves and reloads it, and llama_model_saver did not re-emit those keys, so
the failure was in the roundtrip leg rather than the first load. Three gaps,
all in shared code and all additive:
- add_kv_from_model wrote no hyper-connection, compress-ratio or PLE keys.
The PLE group only means anything whole, so it is written or omitted
together; the rest follow the file's existing style of writing every key
unconditionally, since an architecture that does not read one is
unaffected by a zero.
- the saver had no uint64 path at all, which the PLE hash constants need.
- add_tensors_from_model enumerates model-level tensors by hand and was
missing per_layer_tok_embd and the three final-mixer tensors.
Two smaller fixes on the qwen4exp side, both found by running the test:
- build_qsa_top_k divided by the compression ratio before asserting it was
non-zero, so a file without the key crashed instead of reporting.
- a layer with no compression ratio now falls back to dense attention,
which is what the model computes below the budget anyway. The test then
has to write a ratio to reach QSA at all, and an indexer key length no
narrower than n_rot, since the indexer ropes with the main attention's
rotary width.
Full suite: 126 archs, qwen4exp at 0.00e+00 with roundtrip OK. The tiny
fixture is unchanged, max logit delta 0.0 against the pre-QSA dense run.
The n-gram table arrives as 128 shards that were held in a dict and then torch.cat-ed, so the peak was the shards plus the concatenation: around 300 GB of RSS on the real checkpoint, which rules out machines that could otherwise convert this model. Each shard is now written straight into a memory-mapped file at its final row offset and dropped, so the resident set is one shard and the rest is the page cache's problem. The temporary file sits beside the output and is removed once the write finishes, including on failure. Shards other than the last must be uniform for direct placement, which is asserted rather than assumed, and a shard arriving before the stride is known is held instead of misplaced. Verified on the tiny fixture: the resulting GGUF is byte-identical to the one the concatenating path produced (md5 2d274efac91ad1e9a6007efb0687e597).
tensor_type_fallback demotes a tensor whose ncols is not a multiple of the target's block size, but its switch only enumerates the 256-block types. A target that is already a 32-block type (iq4_nl, q4_0, q5_0, q8_0, ...) falls into default: and throws, even though the function already knows how to answer that case: the ncols check right below the switch resolves an unrepresentable shape to F16. Route those types into that check instead of throwing. Only paths that abort today change, so no quantization that currently succeeds is affected. Found on a 4-wide depthwise conv kernel. llama-quantize reported nothing but "failed to quantize model from ...", with no tensor name and no exception text, which made a quant recipe that had simply not pinned the tensor look like a corrupt model. It now names the tensor and continues.
per_layer_token_embd shares the TOKEN_EMBD category with token_embd.weight, so --token-embedding-type is returned for it before any --tensor-type pattern is consulted, and there is no way to give it a tier of its own. That grouping is fine as a default and stays the default. It is a poor fit for the size, though: on qwen4exp the table is 97.7 GiB of a 337.6 GiB BF16 file and about 46% of a 4-bit one, roughly eighty times token_embd.weight, and it is read by ggml_get_rows rather than a matmul so no imatrix ever covers it. Allow an explicit --tensor-type pattern to name it, and only it. Nothing changes unless such a pattern is passed, and token_embd.weight keeps the old precedence in either case. Measured on Qwen3.8-Flash-Next, Q4_K_M with an imatrix: the table lands at q8_0 (51.9 GiB, 113.5 GiB total) by following --token-embedding-type, and pinning it q4_1 gives 30.5 GiB for 92.1 GiB total, 19% off the file.
The per-tensor output buffer was sized `nelements * 4`, described as an upper bound. It is a very loose one: the output is at most 2 bytes per element (f16/bf16) and usually well under 1.1 (q8_0 and below), so between 2x and 4x of it is never touched. The exact size is already known here, since it is what the quantization loop writes, what new_size sums to, and what the GGUF metadata is asserted against a few lines later. On a model whose largest tensor is a few GB none of this matters. On Qwen3.8-Flash-Next it does: per_layer_token_embd is 51.2 G elements, so the buffer was 205 GB where 54 GB is needed at q8_0 and 32 GB at q4_1. Measured on that model, VmHWM of a live llama-quantize was 485 GB per process. Three of them fit in 2 TB and five did not, which is what an OOM-killed quant ladder looks like. This removes about 150 GB of that. Byte-identical output, verified against the same binary built at the parent commit: q4_K, q8_0, q5_K, q6_K and IQ4_XS, over BF16 and F32 sources, with and without a PLE table present. Six cases, six matching md5s.
The PLE row indices are computed host-side from ubatch->token, and set_input
returned early when that was null. A multimodal ubatch is exactly that case:
the mtmd layer consumes the image placeholder ids and hands llama_decode
embeddings instead. The early return left the I32 index tensor uninitialised,
so ggml_get_rows indexed a 320 M row table with whatever the buffer happened to
contain, and aborted:
GGML_ASSERT(i01 >= 0 && i01 < ne01) failed
ggml_compute_forward_get_rows
mtmd_helper_decode_image_chunk -> llama_decode
Every image request crashed. Nothing caught it because the vision work had only
ever been verified by converting an mmproj, never by running one.
The reference computes the hash over input_ids, where those positions still
hold the image placeholder, so carry that id through as qwen4exp.ple.image_token_id
and hash it. The key is optional: a file converted before it existed falls back
to the PLE EOS token, which is defined and treats the image as a segment
boundary rather than crashing.
Verified end to end with llama-mtmd-cli, a Q4_K_M base and the F16 mmproj, on a
generated image with known content. The model names the red circle, the blue
square, the inverted green triangle and reads "UNSLOTH 42", each with the right
position.
set_input_qsa asserted n_stream == 1, so llama-server could not serve this model with more than one slot unless -kvu was passed. With a non-unified cache each sequence owns its own cells, and a cell index means a different token in each stream, so a single shared mapping is wrong. - cell_blk, blk_cells and bias gain a stream dimension. At n_stream == 1 these collapse to the shapes they had, so the unified path is unchanged. - Scoring is now batched over streams. ggml_mul_mat matches ne[2] on both operands, so stream s's queries only ever meet stream s's blocks; without this sequences would score against each other's context. - set_input_qsa loops per stream and resolves cells through v_cells[seq_to_stream[seq_id]], following set_input_kq_mask_impl, instead of hardcoding v_cells[0]. - llama_kv_cache_context::get_n_stream() is added, mirroring the ns that get_k and get_v already derive from the slot info. build_attn_mask_top_k needed no change: it already expects [n_top_k, n_batch, 1, n_stream], so the top-k result is reshaped to meet it. set_input_qsa has exactly one caller, so the blast radius is qwen4exp only. Validation, UD-Q4_K_XL on one B200: - unified cache unchanged within noise: 1802.9/68.85 -> 1807.2/69.11 t/s at batch 1, 2262.5/192.43 -> 2270.1/193.75 at batch 4. - non-unified now runs at npl 1, 4, 16 where it previously aborted, and is 22% faster than the -kvu workaround at batch 16 (1205 vs 984 t/s total), since per-stream cells avoid the cross-sequence masking a unified cache pays for. - no cross-stream contamination: four concurrent sequences each carrying a distinct secret all recall their own and no other, on both cache modes. - test-llama-archs green on qwen4exp, deepseek2, gemma3n, qwen3next, llama. Note on testing: comparing concurrent output against solo output exactly is not a valid check. It failed 0/4 with no bug present, and the unified-cache control failed the same way, because batch composition changes the floating-point reduction order and near-tied tokens flip. The contamination test above is what the exit code gates on.
![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: bea3b12dae
ℹ️ 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 back = s - i; | ||
| const int64_t k = (int64_t) hist.size() - back; | ||
| if (back > 0 && k >= 0 && k < (int64_t) hist.size() && pos - s >= 0) { |
There was a problem hiding this comment.
When an ubatch contains multiple sequences, i is the global token index, so for the first token of every sequence after the first, back = s - i is non-positive and the saved predecessor history is never used. With the default non-unified cache, split_equal() concatenates each sequence's tokens, making continued batched decoding hash EOS padding for all but the first sequence and produce incorrect PLE activations. Compute this offset from the token's sequence-local index instead.
Useful? React with 👍 / 👎.
| ctx_idx(mem->get_mem_idx() == nullptr ? nullptr : | ||
| new llama_kv_cache_context(mem->get_mem_idx(), std::move(sinfos_idx), this->ubatches)), |
There was a problem hiding this comment.
The indexer context is created only by this batch constructor; the full/update constructors above still create contexts for mem_attn and mem_recr only. Consequently, a cross-stream seq_cp queues buffer copies in mem_idx, but the following memory_update() never applies them, while the attention cache is copied. Subsequent QSA then addresses the destination using copied cell metadata but stale indexer key rows. Include the indexer cache in the update lifecycle, handling its raw, unrotated keys appropriately.
Useful? React with 👍 / 👎.
| void llama_memory_hybrid::clear(bool data) { | ||
| mem_attn->clear(data); | ||
| if (mem_idx) mem_idx->clear(data); | ||
| mem_recr->clear(data); |
There was a problem hiding this comment.
Although the new indexer cache is cleared here, state_write() and state_read() below still serialize only mem_attn and mem_recr. Loading a Qwen4Exp sequence into a fresh context therefore restores the attention cells but leaves all historical indexer keys and their cell metadata empty, so QSA cannot select the restored prefix correctly on subsequent decoding. Serialize mem_idx alongside the attention cache for non-partial state operations.
Useful? React with 👍 / 👎.
| struct ple_history { | ||
| llama_pos next_pos = -1; | ||
| std::vector<llama_token> toks; | ||
| }; | ||
| mutable std::unordered_map<llama_seq_id, ple_history> ple_hist; |
There was a problem hiding this comment.
This history is stored on the model rather than in the memory module, so sequence lifecycle operations do not update it. In particular, after llama_memory_seq_cp(src, dst, ...), the attention, recurrent, and indexer states are copied but ple_hist[dst] is absent; continuing the destination sequence makes the first n_gram - 1 positions use EOS predecessors instead of the copied prefix. Move this state into the per-context memory lifecycle or explicitly handle copy and state restoration.
Useful? React with 👍 / 👎.
| blk_of[j] = (int32_t) b; | ||
| cur_blk_cells[b*r + (p%r)] = (int32_t) j; | ||
| filled[b]++; |
There was a problem hiding this comment.
With kv_unified enabled and multiple sequences, cells contains several cells with the same position, so this assignment repeatedly overwrites each (block, position) member while filled[b] counts cells from every sequence. The pooled indexer key for all queries is therefore assembled from whichever sequence wrote last, even though the later bias filters visibility per sequence, producing incorrect sparse top-k selections for multi-sequence -kvu workloads. Build block membership per sequence or disable this sparse path for that cache layout.
Useful? React with 👍 / 👎.
* rpc : implement event and async backend APIs * cache responses from RPC_CMD_GET_ALLOC_SIZE
The QSA graph needed a build_attn that attends only to the cells named by a
top_k tensor, and the first version got it by adding a llm_graph_input_attn_kv
overload to llm_graph_context and factoring the mask construction out of the
existing MLA sparse path into a shared build_attn_mask_top_k.
That put a new arch on the shared attention path and made the deepseek32 and
glm-dsa attention build depend on a helper introduced for qwen4exp. Build the
mask in src/models/qwen4exp.cpp instead and leave llama-graph.{h,cpp} exactly as
they were: the MLA path keeps its own copy of the same node sequence.
The nodes emitted are unchanged, so this is bit-identical.
The indexer key cache was added by extending llama_memory_hybrid with an
optional third cache, and the host-side cell/block mapping that drives QSA was
added as set_input_qsa on llama_kv_cache. Both are shared classes that every
hybrid and every attention model goes through.
Move both into a new memory type, llama_memory_hybrid_idx, following
llama_kv_cache_msa: the indexer cache and the pos<->cell translation live with
the sparse-attention memory rather than in the classes that serve every other
architecture. llama-kv-cache.{h,cpp} and llama-memory-hybrid.{h,cpp} are
restored to their unmodified state.
init_batch is repeated from llama_memory_hybrid because the indexer cache has to
be handed the attention cache's slot infos, and those are not reachable through
the context the base returns. Allocating them separately lets the two caches
drift, which is what pointed QSA's top-k at the wrong cells before.
The context derives from llama_memory_hybrid_context so build_inp_mem_hybrid
keeps working unchanged, and get_n_stream is computed from the slot infos
exactly as llama_kv_cache_context did.
Behaviour is unchanged: logits over an 8192-token sequence are bit-identical to
the previous implementation, sparse and dense alike.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 035e22731a
ℹ️ 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".
| blk_of[j] = (int32_t) b; | ||
| cur_blk_cells[b*r + (p%r)] = (int32_t) j; | ||
| filled[b]++; |
There was a problem hiding this comment.
For Qwen multimodal inputs, mtmd_image_tokens_get_decoder_pos() assigns every image token the same temporal position (tools/mtmd/mtmd.cpp:2405), so this position-based slot repeatedly overwrites one cur_blk_cells entry while filled[b] increases for every patch. The block is then considered complete even though its gather contains mostly default cell 0 entries, and all image tokens collapse onto one score, producing incorrect sparse selection during image processing and later text decoding. Fresh evidence beyond the existing unified-cache report is that this occurs with a single sequence because Qwen M-RoPE itself repeats the temporal position; block membership needs to follow sequence-local token order rather than cells.pos_get(j) alone.
Useful? React with 👍 / 👎.
| add_kv(LLM_KV_PLE_CONV_KERNEL, hparams.ple_conv_kernel); | ||
| add_kv(LLM_KV_PLE_EOS_TOKEN_ID, hparams.ple_eos_token_id); | ||
| add_kv(LLM_KV_EMBEDDING_LENGTH_PER_LAYER, hparams.ple_head_dim); |
There was a problem hiding this comment.
When a multimodal Qwen4Exp model is round-tripped through llama_model_save_to_file(), this reconstructed PLE metadata omits LLM_KV_PLE_IMAGE_TOKEN_ID. Reloading the saved file therefore leaves ple_image_token_id at zero, and embedding-only image batches hash EOS instead of the checkpoint's image placeholder in llm_graph_input_ple::set_input(), changing PLE activations and model output. Persist the image token ID with the other PLE keys when it is present.
Useful? React with 👍 / 👎.
| return new llama_kv_cache( | ||
| model, hparams_idx, type_k, type_v, v_trans, offload, unified, | ||
| kv_size, n_seq_max, n_pad, n_swa, swa_type, | ||
| nullptr, filter_idx, nullptr, nullptr); |
There was a problem hiding this comment.
Every Qwen4Exp indexer layer allocates a full value-cache tensor here even though the indexer path only calls cpy_k() and get_k(). llama_kv_cache sets has_v = !is_mla, and the copied Qwen4Exp hparams are not MLA, so this reserves n_embd_v_gqa * kv_size elements per full-attention layer and stream with no reader or writer. At long contexts this adds hundreds of MiB of avoidable KV memory; use a key-only cache representation or otherwise suppress the V allocation for mem_idx.
Useful? React with 👍 / 👎.
…ound larger warps (ggml-org#27726)
|
Prompt processing has become noticeably slower and less stable. |
* feat(ui): make base dialog responsive and support sticky headers * ui: move dialog close button to the sticky header Assisted-by: pi * chore: Formatting & linting
…org#27744) * ui : open MCP servers in a dialog from the chat form Replace the MCP servers submenu with a single "MCP Servers" item that opens a new DialogMcpServers dialog instead of navigating to the /mcp-servers route. Assisted-by: pi * ui : browse MCP resources from the server card Make the Resources capability badge clickable so it opens the MCP resources browser dialog, and drop the page-only chrome from SettingsMcpServers. Assisted-by: pi * ui : remove mcp-servers route and sidebar entry MCP servers are now managed in a dialog, so drop the dedicated route and the sidebar icon that navigated to it. Assisted-by: pi * ui : remove unused MCP servers submenu component The submenu was replaced by the MCP servers dialog, so delete the component and its export. Assisted-by: pi * feat(ui): add DialogSettingsChat dialog * refactor(ui): switch SettingsChat to in-app section navigation * feat(ui): open settings as dialog from sidebar * refactor(ui): remove settings route and URL-based settings navigation * fix(ui): adjust MCP dialogs for new base sizing * chore: Formatting & linting
* kv: track token id * rm get_prev_tokens, move it to the main pr * nits * add get_prev_tokens
llama_memory_hybrid_idx forwarded clear, seq_rm, seq_cp, seq_keep, seq_add and seq_div to the indexer cache but not state_write / state_read, so a saved session dropped the indexer keys and a restored one selected QSA top-k against an empty cache. The effect is invisible until the context passes indexer_top_k + compress_ratio - 1 cells, because QSA is exactly dense below that and the indexer contents cannot change the result. The indexer section is written last rather than next to the attention cache it mirrors. As a suffix, a reader that does not expect it stops early and the trailing bytes are caught by the size check in state_load_file; placed between the attention and recurrent sections it would instead be parsed as recurrent state, which can succeed and restore silent garbage. It follows the same LLAMA_STATE_SEQ_FLAGS_PARTIAL_ONLY gate as the attention cache, since a partial checkpoint deliberately skips the token-level attention caches. The indexer restores its own cells instead of taking the attention cache's restored slots. The two caches share size, padding and every sequence operation, and init_batch hands the indexer the attention cache's slot infos, so both state_read_meta calls run find_slot over identical occupancy and land on identical cells. The overrides live on llama_memory_hybrid_idx, the only memory type that owns an indexer cache, so llama_memory_hybrid and every architecture that uses it write and read exactly the bytes they did before. The session and sequence state versions are bumped because the qwen4exp state layout changed. The session path already rejects a short read via its size check, but llama_state_seq_load_file accepts one silently, so only the version check stops a pre-fix blob from being half-restored by a fixed build. (cherry picked from commit 2721542354f8e158c3217625f4e2e7b83e51e3fe)
The QSA bias is a graph input, so it is pinned on the host and uploaded every decode, and at -c 32768 -np 4 its twelve copies were 768 of the 815 MiB of reserved host compute buffer. Only one half of it needs a cell: whether the cell sits in the always-visible tail, and whether its block was pooled. Both are properties of the block. The other half - empty, other sequence, or in the future - is the plain visible/not test the attention mask already carries over the same cells, so add that mask instead of repeating it. The bias then holds one value per block. A block sits wholly inside or wholly outside the tail because the tail starts on a block boundary, so one value per block is exact. Cells no block covers keep their -inf from the mask. The mask is F16 and the bias F32, and a mixed ggml_add reinterprets the F16 buffer as float rather than converting it, so the cast is required. reserved host compute buffer at -c 32768 -np 4: --kv-unified 814.86 -> 238.86 MiB, CUDA0 721.07 -> 421.07 MiB --no-kv-unified 214.86 -> 70.86 MiB, CUDA0 317.07 -> 265.07 MiB Selection is unchanged: over 8192 tokens, four times the budget, every QSA layer returns identical top-k indices and the logprobs are bitwise equal. Two things a reviewer should know. A cell whose position divides past the last block is guarded by an assert rather than handled, because no run reached it. And the mask's same-position M-RoPE rule cannot fire for text and was never exercised for images, so the 2D case is unverified.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef6876693f
ℹ️ 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".
| #define RPC_PROTO_MAJOR_VERSION 6 | ||
| #define RPC_PROTO_MINOR_VERSION 0 |
There was a problem hiding this comment.
The reviewed Qwen4Exp change also bumps the RPC protocol and replaces its dispatch model, rewrites substantial Hexagon code, adds Vulkan cross-entropy kernels, replaces Snapdragon tooling, and changes UI settings routing. These changes are independent of Qwen4Exp and couple backend, protocol, and product regressions to the model port, making the change impractical to review, test, or revert as one unit; move them into separate focused changes.
AGENTS.md reference: AGENTS.md:L15-L19
Useful? React with 👍 / 👎.
| sock = socket_t::connect(host.c_str(), port); | ||
| if (sock == nullptr) { | ||
| return nullptr; | ||
| GGML_ABORT("Failed to connect to %s\n", endpoint.c_str()); |
There was a problem hiding this comment.
When an RPC endpoint is temporarily unavailable, socket_t::connect() now reaches GGML_ABORT, terminating the entire host process during ggml_backend_rpc_add_server() device discovery. The parent implementation returned nullptr, allowing device discovery to report zero devices and the application to handle the unavailable server; preserve that recoverable behavior rather than making a remote connection failure fatal.
Useful? React with 👍 / 👎.
* convert: fix Nemotron-H LoRA GGUF conversion * Removed redundant JSON import.
… into qwen4exp/qwen3.8-flash-next
…l-org#27746) * ui : strip trailing container-format segments from parsed model names * ui : show reasoning and modality icons on model options and search by modality * ui : keep reasoning submenu visible regardless of model state * ui : add show-org-name-in-trigger display setting * ui : move model list into a submenu within the model selector * ui : make model option hover and focus highlight override the active state * ui : add raw model id tooltip to model selector options * feat: Enable microphone input as default for audio models * ui : fix eslint issues in chat form and model selector * ui: show modality icons instead of file submenu in chat add menu Assisted-by: pi * chore: Format * chore: Format * ui: add ModelCapability enum and shared modality/capability icon constants Assisted by: pi:GLM-5.3-Flash * ui: derive modality badge icons and labels from shared constants Assisted by: pi:GLM-5.3-Flash * ui: split model option icons into capabilities and modalities Replace the supportsThinking flag on ModelId with a capabilities object keyed like ModelModalities, so future capabilities (tool calls, etc.) slot in alongside reasoning. Icons and labels now come from the shared CAPABILITY_ICONS/MODALITY_ICONS constants. Assisted by: pi:GLM-5.3-Flash
* llama: model_loader: add TENSOR_GET_ROW_LAZY * add --tensor-read-lazy * rename to TENSOR_READ_LAZY * gen docs * address comments
|
@ngxson That went a bit weird here. Edit: Doesn't seem to matter in PR though. |
…n Backend (ggml-org#27453) * vulkan: add LIGHTNING_INDEXER op * vulkan: updated lightning_indexer.comp and ggml-vulkan.cpp with 128-lane dot-product reduction moved from a shared-memory tree to subgroupAdd. * vulkan: cleanup; Skip bounds checks * vulkan: cleanup FA_K_ONLY * Revert "vulkan: cleanup FA_K_ONLY" This reverts commit fdcbdd9. * vulkan: restore interleaved K/V buffer ordering * vulkan: Remove FA_K_ONLY * vulkan: Revert flash_attn_dequant * vulkan: Revert tests in backend-ops.cpp
16 participants
Adds support for
Qwen3.8-Flash-Next(HFmodel_type: qwen4_exp,Qwen4ExpForConditionalGeneration) end to end: converter, text graph, sparse attention, vision, and the quantizer changes the model needs. This is the whole port in one PR; it is the same work as a private staging stack, combined for a single review pass.What the architecture needs
build_moe_ffnunchangedggml_rope_multiwithLLAMA_ROPE_TYPE_IMROPE, unchangedhc_count = 4)qwen4exp.cpp;deepseek4.cppis deliberately untouchedset_input, then a plainggml_get_rowsllama_memory_hybridNo new ggml op was required:
git diff master --stat -- ggml/is empty.Correctness
Measured against the vLLM reference implementation on the real 360 GB checkpoint.
test-llama-archs -a qwen4expSeven-turn chat and an image test both pass on real weights.
Not breaking other models
Six shared files contain deletions rather than pure additions, and each is backed rather than argued:
llama-quant.cpp: byte-identical output across six A/B cases against the parent commit, over 256-block and 32-block base types, with and without a PLE table present.llama-graph.cpp:build_attn_mask_top_kwas lifted verbatim out of the DSA overload. The node construction sequence is identical after splicing the helper back into its call site, anddeepseek32andglm-dsaboth pass at 0.00e+00 on CPU.llama-memory-hybrid.cpp: the newfilter_idxdefaults tonullptrand every use is guarded, so a model that does not ask for an indexer cache cannot observe it.llama-model-loader.cpp: adds aGGUF_TYPE_UINT64case to a type check that previously threw; no existing model carries a uint64 array.llama-hparams.cpp,llama-model.cpp: new fields and new switch cases only.deepseek4.cppis unchanged. Extracting a shared hyper-connection base was considered and rejected: the two formulations share the residual layout and nothing else, so it would have touched DSV4's hot path to share about ten lines.Quantizer changes
Three commits here fix problems that are not specific to this model:
tensor_type_fallbackhad no case for 32-block types, so any tensor with an ncols not divisible by 32 aborted the run rather than falling back. This is whatblk.N.ple_conv1d.weight(ncols 4) hit.--tensor-typecould not nameper_layer_token_embd, because--token-embedding-typereturned before patterns were consulted.workbuffer was sizednelements * 4as an upper bound on a value already known exactly, which cost about 150 GB per process on this model. Sizing it exactly is byte-exact: six cases, six matching md5s.Commits
18 commits, grouped as they were reviewed:
test-llama-archscoverage