From 40e90413fd47ccff5519d3400c1ad4aa7ca39242 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 25 Aug 2026 19:17:58 +0000 Subject: [PATCH] tests: cover qwen4exp in test-llama-archs 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. --- src/llama-model-saver.cpp | 37 +++++++++++++++++++++++++++++++++++++ src/llama-model-saver.h | 1 + src/models/qwen4exp.cpp | 10 ++++++---- tests/test-llama-archs.cpp | 12 +++++------- 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/llama-model-saver.cpp b/src/llama-model-saver.cpp index 9adaa93f62..8860bd3f43 100644 --- a/src/llama-model-saver.cpp +++ b/src/llama-model-saver.cpp @@ -60,6 +60,10 @@ void llama_model_saver::add_kv(const enum llm_kv key, const int32_t value) { gguf_set_val_i32(gguf_ctx, llm_kv(key).c_str(), value); } +void llama_model_saver::add_kv(const enum llm_kv key, const uint64_t value) { + gguf_set_val_u64(gguf_ctx, llm_kv(key).c_str(), value); +} + void llama_model_saver::add_kv(const enum llm_kv key, const float value) { gguf_set_val_f32(gguf_ctx, llm_kv(key).c_str(), value); } @@ -113,6 +117,8 @@ void llama_model_saver::add_kv(const enum llm_kv key, const Container & value, c gguf_set_arr_data(gguf_ctx, llm_kv(key).c_str(), GGUF_TYPE_BOOL, value.data(), n_values); } else if (std::is_same::value) { gguf_set_arr_data(gguf_ctx, llm_kv(key).c_str(), GGUF_TYPE_INT32, value.data(), n_values); + } else if (std::is_same::value) { + gguf_set_arr_data(gguf_ctx, llm_kv(key).c_str(), GGUF_TYPE_UINT64, value.data(), n_values); } else if (std::is_same::value) { gguf_set_arr_data(gguf_ctx, llm_kv(key).c_str(), GGUF_TYPE_FLOAT32, value.data(), n_values); } else if (std::is_same::value) { @@ -124,6 +130,7 @@ void llama_model_saver::add_kv(const enum llm_kv key, const Container & value, c // instantiate for external usage: template void llama_model_saver::add_kv>(const enum llm_kv, const std::vector &, const bool); template void llama_model_saver::add_kv>(const enum llm_kv, const std::vector &, const bool); +template void llama_model_saver::add_kv>(const enum llm_kv, const std::vector &, const bool); void llama_model_saver::add_kv(const enum llm_kv key, const std::vector & value) { std::vector tmp(value.size()); @@ -308,6 +315,32 @@ void llama_model_saver::add_kv_from_model() { add_kv(LLM_KV_HYPER_CONNECTION_SINKHORN_ITERATIONS, hparams.dsv4_hc_sinkhorn_iters); add_kv(LLM_KV_HYPER_CONNECTION_EPSILON, hparams.dsv4_hc_eps); add_kv(LLM_KV_HASH_LAYER_COUNT, hparams.dsv4_hash_layer_count); + add_kv(LLM_KV_HYPER_CONNECTION_LOW_RANK, hparams.hc_low_rank); + + // the PLE group only means anything whole: write all of it or none + if (hparams.ple_n_heads > 0) { + std::vector ple_layers; + for (uint32_t il = 0; il < hparams.n_layer_all; ++il) { + if (hparams.is_ple_impl[il]) { + ple_layers.push_back(il); + } + } + add_kv(LLM_KV_PLE_LAYERS, ple_layers); + add_kv(LLM_KV_PLE_NGRAM_SIZE, hparams.ple_ngram_size); + add_kv(LLM_KV_PLE_HEADS_PER_NGRAM, hparams.ple_heads_per_ngram); + 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); + add_kv(LLM_KV_PLE_LAYER_MULTIPLIERS, std::vector( + hparams.ple_layer_multipliers.begin(), + hparams.ple_layer_multipliers.begin() + hparams.ple_ngram_size)); + add_kv(LLM_KV_PLE_HEAD_OFFSETS, std::vector( + hparams.ple_head_offsets.begin(), + hparams.ple_head_offsets.begin() + hparams.ple_n_heads)); + add_kv(LLM_KV_PLE_HEAD_VOCAB_SIZES, std::vector( + hparams.ple_head_vocab_sizes.begin(), + hparams.ple_head_vocab_sizes.begin() + hparams.ple_n_heads)); + } const float rope_scaling_factor = hparams.rope_freq_scale_train == 1.0f ? 0.0f : 1.0f/hparams.rope_freq_scale_train; @@ -442,6 +475,10 @@ void llama_model_saver::add_tensors_from_model() { add_tensor(model->hc_head_fn); add_tensor(model->hc_head_base); add_tensor(model->hc_head_scale); + add_tensor(model->per_layer_tok_embd); + add_tensor(model->hc_head_norm); + add_tensor(model->hc_head_down); + add_tensor(model->hc_head_up); for (const struct llama_layer & layer : model->layers) { for (size_t i = 0; i < sizeof(layer)/sizeof(struct ggml_tensor *); ++i) { diff --git a/src/llama-model-saver.h b/src/llama-model-saver.h index 36a715e2b6..95e19e666e 100644 --- a/src/llama-model-saver.h +++ b/src/llama-model-saver.h @@ -21,6 +21,7 @@ struct llama_model_saver { void add_kv(enum llm_kv key, uint32_t value); void add_kv(enum llm_kv key, int32_t value); + void add_kv(enum llm_kv key, uint64_t value); void add_kv(enum llm_kv key, float value); void add_kv(enum llm_kv key, bool value); void add_kv(enum llm_kv key, const char * value); diff --git a/src/models/qwen4exp.cpp b/src/models/qwen4exp.cpp index 49028a674c..0a009329dc 100644 --- a/src/models/qwen4exp.cpp +++ b/src/models/qwen4exp.cpp @@ -409,10 +409,11 @@ ggml_tensor * llama_model_qwen4exp::graph::build_qsa_top_k( const int64_t n_idx_h = hparams.indexer_n_head; const int64_t r = hparams.dsv4_compress_ratios[il]; const int64_t n_kv = mctx_idx->get_n_kv(); - const int64_t n_blocks = (n_kv + r - 1)/r; GGML_ASSERT(r > 0); + const int64_t n_blocks = (n_kv + r - 1)/r; + auto qsa = std::make_unique(mctx_idx, (uint32_t) r); qsa->k_idxs = mctx_idx->build_input_k_idxs(ctx0, ubatch); @@ -512,9 +513,10 @@ ggml_tensor * llama_model_qwen4exp::graph::build_layer_attn( const int64_t n_embd_head = hparams.n_embd_head_v(); GGML_ASSERT(n_embd_head == hparams.n_embd_head_k()); - // The indexer reads the same block input as q/k/v; with no indexer cache this - // falls back to dense, which is what the model computes below the budget anyway. - ggml_tensor * top_k = mctx_idx ? build_qsa_top_k(mctx_idx, cur, inp_pos, sections, il) : nullptr; + // indexer reads the same block input as q/k/v; no cache or no ratio means dense + const bool qsa = mctx_idx != nullptr && hparams.dsv4_compress_ratios[il] > 0; + + ggml_tensor * top_k = qsa ? build_qsa_top_k(mctx_idx, cur, inp_pos, sections, il) : nullptr; // Order: joint QG projection, QG split, Q norm, KV projection, K norm, RoPE, attention diff --git a/tests/test-llama-archs.cpp b/tests/test-llama-archs.cpp index 0d455a45ca..8263c79581 100644 --- a/tests/test-llama-archs.cpp +++ b/tests/test-llama-archs.cpp @@ -252,10 +252,14 @@ static gguf_context_ptr get_gguf_ctx(const llm_arch arch, const bool moe) { if (arch == LLM_ARCH_QWEN4EXP) { ms.add_kv(LLM_KV_HYPER_CONNECTION_COUNT, uint32_t(4)); ms.add_kv(LLM_KV_HYPER_CONNECTION_LOW_RANK, uint32_t(8)); + // without this the QSA layers fall back to dense and go uncovered + ms.add_kv(LLM_KV_ATTENTION_COMPRESS_RATIOS, std::vector(n_layer, 4)); } ms.add_kv(LLM_KV_ATTENTION_INDEXER_HEAD_COUNT, arch == LLM_ARCH_MINIMAX_M3 || arch == LLM_ARCH_DEEPSEEK4 ? n_head : uint32_t(1)); - ms.add_kv(LLM_KV_ATTENTION_INDEXER_KEY_LENGTH, uint32_t(64)); + // qwen4exp ropes indexer keys with the main rotary width, so its head can't be < n_rot + ms.add_kv(LLM_KV_ATTENTION_INDEXER_KEY_LENGTH, + arch == LLM_ARCH_QWEN4EXP ? n_embd_head : uint32_t(64)); ms.add_kv(LLM_KV_ATTENTION_INDEXER_TOP_K, uint32_t(8)); ms.add_kv(LLM_KV_ATTENTION_INDEXER_BLOCK_SIZE, uint32_t(4)); ms.add_kv(LLM_KV_ATTENTION_INDEXER_LOCAL_BLOCKS, uint32_t(1)); @@ -492,12 +496,6 @@ static bool arch_supported(const llm_arch arch) { if (arch == LLM_ARCH_GEMMA4 || arch == LLM_ARCH_GEMMA4_ASSISTANT) { return false; // FIXME @ngxson } - if (arch == LLM_ARCH_QWEN4EXP) { - // FIXME: loading reports "key not found: qwen4exp.hyper_connection.count" even - // though the gguf_context passed in carries both HC keys among its 67 KVs, so - // the mismatch is in the loader's view of it. Graph is covered by vLLM parity. - return false; - } if (arch == LLM_ARCH_GRANITE_SWITCH) { return false; // FIXME adapter fixture }