Skip to content

Commit c2d31d8

Browse files
TheTomclaude
authored andcommitted
CRITICAL: found TWO root causes for PPL=165 TheTom#30
1. V cache returns rotated-space values (cosine=0.02 vs correct 0.987) 2. dynamic_cast to llama_kv_cache_context fails for MoE models (uses llama_memory_hybrid_context, not kv_cache_context) → Q rotation and V inverse rotation NEVER executed Fix: store rotation tensors in llm_graph_context, not KV cache. Or access through hybrid memory interface. Co-Authored-By: tturney@psyguard.ai Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 843a1a5 commit c2d31d8

4 files changed

Lines changed: 2144 additions & 14 deletions

File tree

docs/quality-benchmarks.md

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,72 @@ The block size 32 change + MSE-only + pre-rotate-queries may have introduced a b
138138
4. The non-vec flash attention instantiation might use wrong nl parameter
139139

140140
MUST FIX before claiming any quality results.
141+
142+
## Quality Bisection Log
143+
144+
Goal: find which change broke perplexity (165.6 vs 6.1 baseline).
145+
146+
Changes applied (in order):
147+
1. Pre-rotate-queries: rotation moved from dequant to Q in attention graph
148+
2. MSE-only: dropped QJL, 3-bit centroids (2-bit qs + 1-bit signs)
149+
3. Block size 32: storage blocks shrunk from 128 to 32 elements
150+
151+
Bisection plan:
152+
- [ ] Test A: revert block 32 → 128, keep MSE-only + pre-rotate
153+
- [ ] Test B: revert MSE-only → 2bit+QJL, keep block 128 + pre-rotate
154+
- [ ] Test C: revert pre-rotate → rotation in dequant, keep block 128 + 2bit+QJL
155+
- [ ] If A/B/C all still bad: check the rotation matrix consistency
156+
157+
## Bisection Results
158+
159+
### Test A: Disable Q rotation → PPL 165.6 (STILL BAD)
160+
Pre-rotate-queries is NOT the problem. Q rotation has no effect on perplexity.
161+
162+
### Root Cause Found: V CACHE IN ROTATED SPACE
163+
164+
The bug: both K and V caches go through the same turbo3 quantize (which rotates)
165+
and dequant (which returns rotated values without inverse rotation).
166+
167+
For K cache: this is correct because Q is also rotated → <R*q, R*k> = <q, k> ✓
168+
For V cache: this is WRONG because the attention output = attn_weights @ V
169+
needs to be in ORIGINAL space, not rotated space.
170+
171+
The fix: V cache must NOT be rotated during quantize, or the attention output
172+
must be inverse-rotated after the V multiplication.
173+
174+
Options:
175+
1. Skip rotation for V in quantize — only rotate K
176+
2. Add inverse rotation after attention output: out = R^T @ (attn @ R*V) = attn @ V ✓
177+
3. Use f16 for V cache (only compress K) — mixed types need support
178+
179+
Option 1 is simplest: in the SET_ROWS kernel, check if this is K or V and
180+
skip rotation for V. The dequant already skips rotation.
181+
Problem: the SET_ROWS kernel doesn't know if it's writing K or V.
182+
183+
Option 2 is cleanest: add R^T multiplication to attention output in the graph.
184+
This is one ggml_mul_mat per layer (same as Q rotation). Cost is negligible.
185+
186+
### Root Cause #1: V cache dequant returns rotated-space values
187+
Python verified: cosine(input, dequant_output) = 0.02 (garbage)
188+
After inverse rotation: cosine = 0.987 (correct)
189+
190+
### Root Cause #2: dynamic_cast to llama_kv_cache_context FAILS
191+
The Qwen 3.5 MoE uses llama_memory_hybrid_context (not llama_kv_cache_context).
192+
The dynamic_cast returns null, so the V inverse rotation never executes.
193+
This also means the Q pre-rotation never executed — explaining why the
194+
chat server seemed to work (no rotation applied = raw quantize/dequant, which
195+
happens to produce plausible-looking text at low context but garbage perplexity).
196+
197+
### Fix needed
198+
1. Store turbo rotation tensors in llm_graph_context (not KV cache)
199+
OR access them through the hybrid memory interface
200+
2. Apply V inverse rotation through a non-cast path
201+
3. Verify Q rotation also executes (it was also using the same cast)
202+
203+
### The "coherent text" mystery explained
204+
The model produced grammatical text because:
205+
- Without Q rotation, attention scores are computed in wrong space but still produce
206+
some attention pattern (just not the right one)
207+
- Without V inverse rotation, the output is in rotated space which is orthogonal to
208+
the correct space — but each layer compounds the error
209+
- Short conversations look plausible but perplexity reveals the content is wrong

src/llama-kv-cache.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,13 @@ llama_kv_cache::llama_kv_cache(
224224

225225
layers.push_back({ il, k, v, k_stream, v_stream, });
226226

227-
// TurboQuant: create rotation matrix tensor (once, shared across layers)
227+
// TurboQuant: create rotation matrix tensors (once, shared across layers)
228228
if (turbo_rotation == nullptr &&
229229
(type_k == GGML_TYPE_TURBO3_0 || type_k == GGML_TYPE_TURBO4_0)) {
230230
turbo_rotation = ggml_new_tensor_2d(ctx, GGML_TYPE_F32, 128, 128);
231-
ggml_format_name(turbo_rotation, "turbo_rotation");
231+
ggml_format_name(turbo_rotation, "turbo_rotation"); // R^T
232+
turbo_rotation_inv = ggml_new_tensor_2d(ctx, GGML_TYPE_F32, 128, 128);
233+
ggml_format_name(turbo_rotation_inv, "turbo_rotation_inv"); // R
232234
}
233235
}
234236

@@ -275,11 +277,12 @@ llama_kv_cache::llama_kv_cache(
275277

276278
ggml_backend_buffer_clear(buf, 0);
277279

278-
// Fill turbo rotation matrix AFTER buffer clear (clear zeroes everything)
280+
// Fill turbo rotation matrices AFTER buffer clear (clear zeroes everything)
279281
if (turbo_rotation != nullptr && turbo_rotation->buffer != nullptr && !model.hparams.no_alloc) {
280282
#include "turbo-rotation-data.h"
281-
ggml_backend_tensor_set(turbo_rotation, TURBO_ROTATION_MATRIX, 0, 128 * 128 * sizeof(float));
282-
LLAMA_LOG_INFO("%s: TurboQuant rotation matrix initialized (128x128)\n", __func__);
283+
ggml_backend_tensor_set(turbo_rotation, TURBO_ROTATION_RT, 0, 128 * 128 * sizeof(float));
284+
ggml_backend_tensor_set(turbo_rotation_inv, TURBO_ROTATION_R, 0, 128 * 128 * sizeof(float));
285+
LLAMA_LOG_INFO("%s: TurboQuant rotation matrices initialized (128x128)\n", __func__);
283286
}
284287
ctxs_bufs.emplace_back(std::move(ctx), buf);
285288
}
@@ -2469,6 +2472,10 @@ ggml_tensor * llama_kv_cache_context::get_turbo_rotation() const {
24692472
return kv->get_turbo_rotation();
24702473
}
24712474

2475+
ggml_tensor * llama_kv_cache_context::get_turbo_rotation_inv() const {
2476+
return kv->get_turbo_rotation_inv();
2477+
}
2478+
24722479
ggml_tensor * llama_kv_cache_context::cpy_k(ggml_context * ctx, ggml_tensor * k_cur, ggml_tensor * k_idxs, int32_t il) const {
24732480
return kv->cpy_k(ctx, k_cur, k_idxs, il, sinfos[i_cur]);
24742481
}

src/llama-kv-cache.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,11 @@ class llama_kv_cache : public llama_memory_i {
165165
ggml_tensor * get_k(ggml_context * ctx, int32_t il, uint32_t n_kv, const slot_info & sinfo) const;
166166
ggml_tensor * get_v(ggml_context * ctx, int32_t il, uint32_t n_kv, const slot_info & sinfo) const;
167167

168-
// TurboQuant: get rotation matrix for pre-rotate-queries
168+
// TurboQuant: get rotation matrices
169+
// turbo_rotation = R^T (for Q forward rotation via ggml_mul_mat)
170+
// turbo_rotation_inv = R (for output inverse rotation via ggml_mul_mat)
169171
ggml_tensor * get_turbo_rotation() const { return turbo_rotation; }
172+
ggml_tensor * get_turbo_rotation_inv() const { return turbo_rotation_inv; }
170173

171174
// store k_cur and v_cur in the cache based on the provided head location
172175
ggml_tensor * cpy_k(ggml_context * ctx, ggml_tensor * k_cur, ggml_tensor * k_idxs, int32_t il, const slot_info & sinfo) const;
@@ -273,10 +276,9 @@ class llama_kv_cache : public llama_memory_i {
273276

274277
std::vector<kv_layer> layers;
275278

276-
// TurboQuant pre-rotate-queries: rotation matrix tensor (128×128)
277-
// Created when type_k is TURBO3_0 or TURBO4_0
278-
// Used in build_attn_mha to rotate Q before flash attention
279-
ggml_tensor * turbo_rotation = nullptr;
279+
// TurboQuant rotation matrices (128×128)
280+
ggml_tensor * turbo_rotation = nullptr; // R^T (for Q forward rotation)
281+
ggml_tensor * turbo_rotation_inv = nullptr; // R (for output inverse rotation)
280282

281283
// model layer id -> KV cache layer id
282284
std::unordered_map<int32_t, int32_t> map_layer_ids;
@@ -365,8 +367,9 @@ class llama_kv_cache_context : public llama_memory_context_i {
365367
ggml_tensor * get_k(ggml_context * ctx, int32_t il) const;
366368
ggml_tensor * get_v(ggml_context * ctx, int32_t il) const;
367369

368-
// TurboQuant: get rotation matrix for pre-rotate-queries optimization
370+
// TurboQuant rotation accessors
369371
ggml_tensor * get_turbo_rotation() const;
372+
ggml_tensor * get_turbo_rotation_inv() const;
370373

371374
// store k_cur and v_cur in the cache based on the provided head location
372375
// note: the heads in k_cur and v_cur should be laid out contiguously in memory

0 commit comments

Comments
 (0)