Skip to content

[Bugfix] Zero new KV blocks for quantized + sliding-window hybrid caches - #47574

Merged
tlrmchlsmth merged 4 commits into
vllm-project:mainfrom
EdalatiAli:fix-skipswa-kvquant
Jul 20, 2026
Merged

tlrmchlsmth merged 4 commits into
vllm-project:mainfrom
EdalatiAli:fix-skipswa-kvquant

Conversation

@EdalatiAli

@EdalatiAli EdalatiAli commented Jul 3, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

A hybrid KV cache that quantizes full-attention layers but keeps sliding-window layers in higher precision (--kv-cache-dtype fp8 --kv-cache-dtype-skip-layers sliding_window) produces NaN / all-zero output once the input exceeds the sliding window. Uniform fp8 and bf16 are fine; only the hybrid combo fails (seen on Blackwell + FlashInfer).

Root cause

Attention groups share one block pool. The sliding-window group frees blocks mid-request; those pages get reallocated to the fp8 group and read back including slots not written this step. As bf16 that leftover is harmless
garbage, but as fp8 it decodes to NaN/Inf and corrupts attention. The existing zeroing guard (needs_kv_cache_zeroing) was gated only on Mamba (#35219), so this config was left exposed.

Fix

Extend needs_kv_cache_zeroing to also fire for block-dropping attention (sliding-window / chunked-local) + quantized (FP8/NVFP4) KV cache:

needs_kv_cache_zeroing = has_mamba_layers or (has_block_dropping_layers and has_quantized_kv_cache)

Uniform / unquantized caches stay False → zero overhead.

Test Plan

  • Run RULER (niah_single_1) evals on CohereLabs/command-a-plus-05-2026-w4a4 with various kv cache quantization options, before and after the fix.
  • Here are the repro steps on B200, FlashInfer backend, lm-eval-harness 0.4.x.

Serve the hybrid config:

vllm serve CohereLabs/command-a-plus-05-2026-w4a4 \
  --tool-call-parser cohere_command4 --reasoning-parser cohere_command4 \
  --enable-auto-tool-choice \
  --kv-cache-dtype fp8 --kv-cache-dtype-skip-layers sliding_window
  1. Run the needle eval (repeat with --kv-cache-dtype fp8 alone, and with no
    --kv-cache-dtype flag for the bf16 reference):
lm_eval --model local-completions \
  --model_args "base_url=http://127.0.0.1:8000/v1/completions,model=CohereLabs/command-a-plus-05-2026-w4a4,num_concurrent=32,tokenized_requests=False" \
  --tasks niah_single_1 \
  --metadata '{"max_seq_lengths":[4096,32768,131072],"tokenizer":"CohereLabs/command-a-plus-05-2026-w4a4"}' \
  --gen_kwargs "temperature=0.9,top_p=0.95,repetition_penalty=1.04" \
  --trust_remote_code

Test Result

KV cache config 4K 32K 132K
bf16 (reference) 0.994 0.388 0.556
fp8 uniform 0.994 0.376 0.462
fp8 + skip-SWA w/o fix 0.002 0.000 0.002
fp8 + skip-SWA with fix 0.990 0.376 0.520

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

@mergify mergify Bot added v1 bug Something isn't working labels Jul 3, 2026
Hybrid configs mixing a block-dropping attention type (sliding-window /
chunked-local) with a quantized (FP8/NVFP4) KV cache could read stale
bytes from reused blocks, decoding to NaN. Extend needs_kv_cache_zeroing
to cover them.

Signed-off-by: EdalatiAli <aliedalati@cohere.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@EdalatiAli
EdalatiAli force-pushed the fix-skipswa-kvquant branch from 14e0b47 to a7f997a Compare July 9, 2026 15:39
@EdalatiAli
EdalatiAli marked this pull request as ready for review July 9, 2026 17:25

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@EdalatiAli EdalatiAli changed the title [Bugfix] Zero freshly allocated KV blocks for quantized + sliding-window hybrid caches [Bugfix] Zero new KV blocks for quantized + sliding-window hybrid caches Jul 9, 2026
Comment thread vllm/v1/kv_cache_interface.py Outdated
Comment on lines +942 to +958
@property
def has_block_dropping_layers(self) -> bool:
"""Any group uses an attention type that frees KV blocks mid-request
(sliding-window or chunked-local)."""
return any(
isinstance(g.kv_cache_spec, (SlidingWindowSpec, ChunkedLocalAttentionSpec))
for g in self.kv_cache_groups
)

@property
def has_quantized_kv_cache(self) -> bool:
"""Any group stores its KV cache in a quantized dtype (FP8/NVFP4)."""
return any(
isinstance(g.kv_cache_spec, AttentionSpec)
and g.kv_cache_spec.kv_quant_mode != KVQuantMode.NONE
for g in self.kv_cache_groups
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel the condition is a little bit off. The fundamental issue is that we have multiple KV cache groups stored at different precisions, right? Any way to capture that more directly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Switched to has_mixed_precision_kv_cache to directly capture the case. Thank you for the comment!

@NickLucche NickLucche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EdalatiAli I might be reading it wrong, but aren't we also including models that are not using different dtypes per sw/fa layers (in fact any model that uses SW and kv fp8 which is quite common)?
I believe we should only target those with hybrid accuracy.

Signed-off-by: EdalatiAli <aliedalati@cohere.com>

@NickLucche NickLucche left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is addressing a more generic issue when mixing dtypes in kv cache, which is one may argue is also what happens with mamba.
Anyways I am happy with the revised check, although we might consider merging mamba/fa condition, I think the current check form might be clearer.
Thanks @EdalatiAli !

@NickLucche NickLucche added the ready ONLY add when PR is ready to merge/full CI is needed label Jul 20, 2026
@tlrmchlsmth
tlrmchlsmth enabled auto-merge (squash) July 20, 2026 13:02
@tlrmchlsmth
tlrmchlsmth merged commit 8ce53a6 into vllm-project:main Jul 20, 2026
81 checks passed
@EdalatiAli

Copy link
Copy Markdown
Contributor Author

@NickLucche Thank you for the insightful comments!

ArjunPakhan pushed a commit to ArjunPakhan/vllm that referenced this pull request Jul 21, 2026
…hes (vllm-project#47574)

Signed-off-by: EdalatiAli <aliedalati@cohere.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Nicolò Lucchesi <nlucches@redhat.com>
@ZJY0516

ZJY0516 commented Jul 25, 2026

Copy link
Copy Markdown
Member

Has any test been done for PD? For Mamba, we found that GPU zero ops and PD transfer may have a data race. @NickLucche @tlrmchlsmth

@NickLucche

Copy link
Copy Markdown
Member

Syncd offline with @ZJY0516 , I believe #48481 is addressing known data races in a generic way that would include the addition here.
Happy to help out and monitor accuracy regressions regardless

michael-gpu pushed a commit to practical-inference/vllm-ampere-extended that referenced this pull request Oct 20, 2026
…hes (vllm-project#47574)

Signed-off-by: EdalatiAli <aliedalati@cohere.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Nicolò Lucchesi <nlucches@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants