qwen4exp: backport upstream correctness fixes (#27941) - #255
Merged
gianni-cor merged 3 commits intoSep 14, 2026
Merged
gianni-cor merged 3 commits into
gianni-cor merged 3 commits into
Conversation
amangupta-tether
force-pushed
the
fix/qsa-multimodal-dense-fallback
branch
from
September 10, 2026 08:16
e832255 to
31a5c7e
Compare
amangupta-tether
changed the title
amangupta-tether
marked this pull request as ready for review
September 10, 2026 08:17
gianni-cor
reviewed
Sep 11, 2026
gianni-cor
reviewed
Sep 11, 2026
…add tests (ggml-org#27941) * qwen4exp: follow up fixes * -kvu NaN collapse fix Assisted-by: Claude * indexer cache ext.x/ext.y restore fix Assisted-by: Claude * kv-cells: rename seq_set to seq_get_all seq_get is already taken by the single-id getter, so the suggested name cannot be overloaded on return type alone. Assisted-by: Claude * memory-hybrid-idx: implement set_input_qsa on the memory class The context held the whole implementation, where the pattern elsewhere is a thin context forwarding to the memory class, as llama_kv_cache_context does for set_input_kq_mask. The body reads no context state, so it moves unchanged and the context keeps a forwarder. Also shortens the seq_get_all comment as suggested. * tests: check that a sequence state survives a save/restore round-trip Saves seq 0, erases it, restores the blob and saves again, requiring the two blobs to match. Compares blobs rather than generated text, which cannot see a field dropped on the way back in. Note this passes on master for qwen4exp, so it does not demonstrate the ext.x/ext.y drop this PR fixes; reaching that needs 2D mrope content. * tests: give the synthetic qwen4exp a PLE so the state test bites has_cell_ext() is n_pos_per_embd() > 1 || ple_n_heads > 0, and the indexer cache sets rope_type = NONE, so without a PLE it serializes no cell ext at all and the round-trip test cannot see a dropped ext.x/ext.y. With one, removing the ext_set restore in state_read_meta fails the test: 198 of 335692 bytes differ, first at offset 282092. Loading such a model needed two fixes: - the row count of per_layer_token_embd came from require_weight(), which a model synthesised from metadata alone has no file to answer. Derive it from the head ranges and prefer the file's padded count where there is one. - the PLE conv history is a row of the recurrent cache, so a PLE on a full attention layer dereferenced a null p_l. Reject it at load time instead. The meta mirror is skipped for qwen4exp. It returned NaN logits before this fixture carried a PLE, which the nmse check passes since a NaN comparison is false, and aborts with one. -sm tensor on real devices works. Assisted-by: Claude * llama: disable -sm tensor for qwen4exp test-llama-archs skipped the tensor split for this arch from inside the test, so the arch still advertised support it does not have. Declare it in llm_arch_supports_sm_tensor instead and drop the test-side exception; the existing llm_arch_supports_sm_tensor branch then does the skipping. Assisted-by: Claude (cherry picked from commit 36b1015) Assisted-by: Codex
amangupta-tether
force-pushed
the
fix/qsa-multimodal-dense-fallback
branch
from
September 14, 2026 04:56
3caf2d5 to
333fe7e
Compare
gagallo7
pushed a commit
to gagallo7/qvac-fabric-llm.cpp
that referenced
this pull request
Sep 15, 2026
…rto#255) Reduced pick: the ggml-org#27941 half (36b1015) is already in the b10796 base, so only the two fork follow-ups are carried: * qwen4exp: disable QSA for unified multi-sequence KV * qwen4exp: enable tensor parallelism (cherry picked from commit a2c50b3)
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
3 participants
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
Backport ggml-org/llama.cpp PR #27941, commit
36b10154383b60eb15baac2c7a40d2a5f784faa7, ontotemp-10549.The upstream fix groups image cells by rank when M-RoPE repeats temporal positions, uses actual block-start spatial coordinates, and separates QSA blocks by sequence set. It also restores indexer keys during sequence copies, preserves cached spatial metadata on state restore, validates malformed model metadata, and avoids the pooled-normalization CUDA grid limit at long context.
Addresses the multimodal QSA issue reported in PR #218.
Additional information
Two cherry-pick conflicts were adapted to this branch:
src/llama-arch.cpp: retain the existing Qwen3Next tensor-split restriction and add upstream's Qwen4Exp restriction. Qwen4Exp-sm tensoris disabled by this backport.tests/test-save-load-state.cpp: add upstream's byte-for-byte state round-trip test to the existing harness as test 6, without importing unrelated harness changes.Validation on Linux arm64 CPU:
test-kv-cacheandtest-kv-cells: passed.test-llama-archs --arch qwen4exp --seed 1: passed inference and model round-trip checks.-np 1) and separate-stream (-np 2) configurations.GPU validation, full CI, and real-model performance/perplexity checks have not been run. The upstream image rank path remains limited to a single sequence in the cache stream; this backport does not expand that coverage.
Requirements