Repository navigation
[Bugfix][Core] Keep KV cache zeroing for mixed-precision UniformTypeKVCacheSpecs groups - #64472
Conversation
…VCacheSpecs groups generate_scheduler_kv_cache_config reduces each UniformTypeKVCacheSpecs group to one layer's spec, so the scheduler's KVCacheConfig no longer sees the other layers' precisions and has_mixed_precision_kv_cache turns False. The scheduler then skips the KV cache zeroing that mixed-precision caches need. Resolve the property before the reduction and keep the result on the config. Co-authored-by: qizixi <22851944+zixi-qi@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Yongye Zhu <zyy1102000@gmail.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: Yifan Qiao <yifanqiao@inferact.ai>
|
Documentation preview: https://vllm--64472.org.readthedocs.build/en/64472/ |
|
/ci run |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #98505. |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #98505. |
|
/ci run |
|
✅ Triggered Buildkite CI #98807 for commit |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #98807. |
|
/ci run |
|
✅ Triggered Buildkite CI #98993 for commit |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #98993. |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #98993. |
|
/ci run |
|
✅ Triggered Buildkite CI #99089 for commit |
|
Hi @zyongye, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
|
/ci run |
|
✅ Triggered Buildkite CI #99284 for commit |
|
/ci retry |
|
✅ No failed, timed-out, or expired jobs need retrying: https://buildkite.com/vllm/ci/builds/99284 |
|
Kevin's review agent: approve for CI changes only |
khluu
left a comment
There was a problem hiding this comment.
Kevin's review agent: approve for CI changes only
Overview
Preserve KV cache zeroing when mixed-precision
UniformTypeKVCacheSpecsgroups are reduced for the scheduler. Resolve the zeroing decision across workers so pipeline stages agree even when only one stage has mixed precision. Split out of #63232 (MiniMax-M3.1-Flash); this change remains independent of the other split PRs.Claims
Validation
The same regression tests were run on original PR head
998f165a2ab0ce47f63ff280b96a600901c7d914and this revision.False is Trueon a uniform worker)kv_block_zeroeris not None; first completion returned HTTP 500CPU commands (with
HF_HUB_OFFLINE=1 PYTHONPATH=$PWD CUDA_VISIBLE_DEVICES=''):.venv/bin/python -m pytest tests/v1/core/test_kv_cache_utils.py \ -k test_generate_scheduler_kv_cache_config_keeps_zeroing -q .venv/bin/python -m pytest tests/v1/core/test_kv_cache_utils.py \ tests/v1/core/test_kv_cache_planning.py \ -k 'test_generate_scheduler_kv_cache_config or test_wrapped_mamba_group_requires_block_zeroing or test_get_kv_cache_configs_pp_sharding' -qGPU command, run through Slurm with two visible GB200 GPUs and cached real weights (before: job 40854, 1 failed; after: job 40859, 1 passed in 171.17 s):
HF_HUB_OFFLINE=1 PYTHONPATH=$PWD CUDA_VISIBLE_DEVICES=0,1 \ .venv/bin/python -m pytest tests/distributed/test_pipeline_parallel.py \ -k test_pp_mixed_precision_kv_cache -v -s --tb=shortThe regression compares PP=2 against PP=1 using the existing
generate_closehelper: greedy first-token top-5 logprobs rounded to two decimal places. Both revisions use the same existing native build. This validates the Python configuration/zeroing path with Qwen, not MiniMax native operators or model accuracy on a benchmark. No performance claim is made.The E2E test is included in the existing pipeline/context parallelism CI steps, with dependencies added for the changed core files. NVIDIA coverage is in the existing optional nightly step; AMD coverage uses its existing PR step. AMD was not run locally.
Details
generate_scheduler_kv_cache_configreduces eachUniformTypeKVCacheSpecsto one layer spec. Recomputing mixed precision after that reduction loses information. Saving only worker 0's mixed-precision result also leaves PP workers inconsistent: a uniform stage skips zeroer initialization although it receives the scheduler'snew_block_ids_to_zero.has_mixed_precision_kv_cacheremains a local fact.needs_kv_cache_zeroingbecomes a stored execution decision, initialized from each worker's full configuration. The planner ORs that decision across workers after final replanning, and scheduler projection preserves it through deepcopy. The tradeoff is that uniform stages also zero new attention blocks when another stage requires zeroing.This extends the metadata-preservation fix in #49623 to intra-group precision and PP worker agreement. It is not a duplicate of the open fixes for transfer destinations/tails (#62313, #55085), backend-specific block reuse (#58716), or initial allocation zero fill (#59432).
AI assistance was used: Claude for the original split, and OpenAI Codex for this follow-up implementation, regression tests, and validation.
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.