Repository navigation
ggml-openvino: Fix SWA layer detection and KV write indices (Gemma-4-E2B past 512 tokens) - #336
Conversation
|
|
||
| // equal extents: tell the two masks apart by the layers that read them, the larger group is swa (gemma-4: 4 of 5 layers) | ||
| // groups of equal size can not be told apart, so leave the layers unclassified | ||
| if (model_params.swa_layers.empty()) { |
There was a problem hiding this comment.
For a model with multiple layer classes but no SWA, wouldn’t this misclassify the larger group as SWA?
There was a problem hiding this comment.
Good catch, thanks!
You're right that two masks don't imply SWA - I found out that llama.cpp's DeepSeek sparse attention builds two full-attention masks with the same name and the fallback would have marked the larger group as SWA.
I pushed a fix. The fallback now classifies the larger group as SWA only once its mask actually keeps fewer keys per row than the other mask, i.e. once the window is visible. Before that the two masks have identical contents, so leaving the layers unclassified gives the same result. The classification is remembered per KV cache buffer, so it doesn't flip back when a new sequence starts, and swa_layers is now part of the graph reuse check. The switch costs one recompile, and only in the equal-extent case (e.g. gemma-4 at -c 1024 -ub 512).
308ccd8 to
836d571
Compare
When the SWA cache size matches the full cache size, identify SWA layers by the number of layers using each mask instead of cache size; the larger group is SWA, and a tie leaves the layers unclassified.
Give SWA KV cache index tensors unique input names (a _swa suffix, like the masks) to prevent them from being merged with the full-attention cache indices.
Two attention masks do not imply SWA: DeepSeek sparse attention uses two full-attention masks with the same name. When cache extents match, classify the larger mask group as SWA only once its mask has fewer keys per row than the other mask. Keep the classification per KV cache buffer so new sequences reuse the same graph, and include swa_layers when checking graph reuse.
836784d to
706a4d9
Compare
Waiting for the window to appear in the mask caused a mid-sequence graph recompile (~9 s for Gemma-4 at -c 1024 after position 512). SWA classification already gives each mask group its own mask, KV write indices, and attention size, which is correct whether or not the mask is a sliding window. Stateful mask rebuild recovers the window from the mask, or the causal mask for full attention. Classify the larger of two mask groups from the first graph and drop the per-KV-buffer bookkeeping.
|
Follow-up on the SWA classification: I pushed a commit that classifies the layers from the first graph instead of waiting for the window to show in the mask. Waiting for the window had a cost I missed earlier. The graph inputs change once the sequence passes the window, so the model recompiles partway through the sequence: about 9 s for gemma-4 at -c 1024, when it first passes position 512. Two masks still don't imply SWA (DeepSeek sparse attention), but the classification doesn't need them to:
So the larger of the two mask groups now gets the role from the first graph, and the per-KV-buffer bookkeeping is gone. Groups of equal size are still left unclassified. |
cavusmustafa
left a comment
There was a problem hiding this comment.
I think we can merge this for now as there seems to be no way to detect swa layers without some model level assumptions. Can you add a comment in the code so we can revisit this layer? Also, probably we can figure out a way to gate this detection to only stateful mode. For stateless, I believe we can ignore this and let llama.cpp handle swa buffer management anyways. All these can be a future work.
Add a TODO above the equal-extent fallback to document that the larger mask group is currently assumed to be the SWA group. Since stateless graphs only require the two groups to remain distinct, this heuristic may only be necessary for stateful execution and could potentially be restricted to that path.
|
Thanks! Added a TODO above the fallback. On stateless: llama.cpp manages the SWA buffers, but the graph still names both masks attn_inp_kq_mask and both caches' write indices attn_inp_k_idxs / attn_inp_v_idxs, and we key OV parameters by name. What stateless really needs is the two groups kept apart, not the SWA role, so we could split them by tensor identity there and keep the "larger group is SWA" guess for stateful only. Happy to do that as a follow-up! |
Overview
On
dev_backend_openvino(308ccd8), Gemma-4 produces incorrect output once the context exceeds 512 tokens. KLD against the CPU backend is 0.83 at 1024 tokens and 5.4 at 2048 tokens, against 0.36 with this fix, and the CPU device is affected too. Two separate issues in the sliding-window attention (SWA) cache handling cause this. This PR fixes both issues in separate commits.1. SWA layers are not detected when both caches have the same size (0d3ea01)
The backend identifies SWA layers by comparing the sizes of the full-attention and SWA KV caches. The SWA cache is sized
n_swa + n_ubatch, which for Gemma-4 with-c 1024 -ub 512is equal to the full cache size. As a result, no layers are identified as SWA, causing the SWA layers to slice the KV cache using the full-attention length.When the cache sizes are equal, the two masks are now distinguished based on the number of layers that use each mask: the larger group is treated as SWA (4 of 5 layers in Gemma-4). If both groups have the same number of layers, there is no way to distinguish them, so the layers remain unclassified as before.
2. SWA and full-attention caches share the same KV write index input (b61f73f)
Since 9655061 (ggml-org#26625), the KV write index tensors for both caches are named
attn_inp_k_idxs/attn_inp_v_idxs. The backend merges tensors with the same name into a single OpenVINO input. Once decoding passes the SWA window, this causes the SWA layers to write their keys and values to rows belonging to the full-attention cache.The SWA index tensors now use a
_swainput name, matching the naming already used for the SWA mask inputs.One file changed:
ggml/src/ggml-openvino/ggml-decoder.cpp(+31 / -4).Additional information
Setup: Panther Lake (Core Ultra X7 358H, Arc B390), Windows 11, OpenVINO 2026.4.0, base 308ccd8. Accuracy is measured as KL divergence against llama.cpp's CPU backend on WikiText-2 (
llama-perplexity --kl-divergence); lower is better.Gemma-4 E2B Q4_K_M:
-b 1), ctx 1024-b 1), ctx 2048The fixed results are consistent with the results from an earlier version of these fixes on the previous base (ab07546), based on a sweep of 7 models from 512 to 4096 tokens.
No regressions:
llama-simplewith Llama-3.2 and Gemma-4: correct text.llama-bench -fa 1, decode (tg128) before -> after:Prefill (pp512) differences are within llama-bench's reported spread.
Requirements