Conversation
Replace top_k_per_row_prefill on the DeepSeek sparse indexer prefill path with a stable value-desc / index-asc selector so equal finite logits keep the same compressed-context indices. Co-authored-by: Grok 4.6 <grok@x.ai> Signed-off-by: Michael Meding <264272563+mimeding@users.noreply.github.com>
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
Purpose
The same long prompt should not change which compressed-context tokens sparse attention uses, or the generated answer, just because several indexer scores were exactly equal. Today the DeepSeek prefill path calls
ops.top_k_per_row_prefill. On equal finite logits that kernel can emit different index sets for the same row when the row batch changes, so later attention sees a different KV subset.This PR replaces that prefill call with a stable value-desc / index-asc selector: columns in
[ks, ke)only, equal scores keep the smaller column index, unused slots stay-1.Changed behavior
vllm/model_executor/layers/sparse_attn_indexer.py(code owner @zyongye).indexer_topk.py(persistent_topk/cooperative_topk/top_k_per_row_decode/ …).torch.argsort(..., descending=True, stable=True)plus a validity mask, matching a production wrap that overwrote the CUDA result.Related work (not duplicates)
Checked open PRs/issues for
top_k_per_row_prefill,prefill topk tie, and #53691.persistent_topkdeterministic. Different registered operator; itstopk.cu/persistent_topk.cuhchange does not cover this prefill path.torch.topkset match, not index-stable ties.top_k_per_row_prefillselection sets differ under row batching. Their live H100 sample found no boundary ties. This PR does not implement TP sharding.enable_thinkingprecedence. Not stacked with this change.Validation
CPU, macOS arm64, Python 3.12, PyTorch 2.14.0, no CUDA. Source checkout with
PYTHONPATH, not a native vLLM GPU build (vllm._versionwarning):PYTHONPATH=. .venv/bin/python -m pytest -q --confcutdir=tests/kernels \ tests/kernels/test_sparse_prefill_topk.py # 10 passed in 0.75sCases: equal finite scores keep the smaller index;
[ks, ke)clipping; empty range; k wider than the valid range; valid-infvs invalid columns; two-row ties; 244 vs 640 boundary tie; 1-row vs batched-row identity under ties; 20-repeat identity; CPU inverted-bounds error.Changed-file pre-commit (
--hook-stage manual, three paths): ruff check/format, typos, mypy 3.10–3.13, SPDX, root lazy imports, forbidden imports, torch.cuda API, config validation, boolean-with.compileallandgit diff --checkpassed.Not run on this branch: CUDA
tests/kernels/test_top_k_per_row.py, current-main GPU serving, or model evals. Existing kernel tests still allow set-equality under ties for the CUDA op; that op is no longer used on this DeepSeek prefill path.Supporting deployment evidence (different tree, NVIDIA vLLM 0.27.1 overlay, wrap image
b15aa6ff): the same selector contract was 24/24 byte-stable across 10 runs / two serving starts and 48/48 long retrieval including 120k. That is not a general fidelity claim and is not a current-main GPU result.Limits
top_k_per_row_prefill.torch.argsortover the logit row is a correctness-first replacement; this PR does not claim a prefill latency win.AI assistance and review status
Grok 4.6 assisted with duplicate-work comparison against current
origin/main(52dd0d7562adb3c2d556ce6fe7ca7c3226e1976b), implementation, CPU tests, and this description. PerAGENTS.md, a human submitter must review every changed line and run relevant tests; that human line-by-line review and human-run GPU/model tests are not claimed here. This PR is draft until that happens.