Skip to content

[Bugfix][Kernel] Break native top-k cutoff ties by token index - #56613

Open
LucasWilkinson wants to merge 2 commits into
vllm-project:mainfrom
LucasWilkinson:codex/topk-cutoff-token-id
Open

LucasWilkinson wants to merge 2 commits into
vllm-project:mainfrom
LucasWilkinson:codex/topk-cutoff-token-id

Conversation

@LucasWilkinson

@LucasWilkinson LucasWilkinson commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Break native top_k_per_row insertion-sort cutoff ties by token index. Previously, tied candidates were ranked by their temporary shared-memory slot, so atomic arrival order could change which token survived the cutoff. Prefer the lower token index.

This is a comparator fix with no new kernel or launch. It does not stabilize the complete output order or change the radix-sort final pass. Separate from #55122 (persistent selector), #53287/#55314 (candidate overflow), and #55872 (opt-in FlashInfer backend); none changes this comparator.

Prior validation on B300, using the identical sampler.cu on the source-built investigation branch (codex/nixl-pcp-dcp-offload-test, commit 279e9406):

.venv/bin/python -m pytest tests/kernels/test_top_k_per_row.py -k cutoff_tie -q

All four cases failed before the fix and passed afterward (prefill/decode, k=512/2048). Those diagnostic tests remain on the investigation branch; this draft contains only the comparator change. Pre-commit, including clang-format, ruff and mypy, passed. A fresh build/test of the extracted branch is pending.

Model evidence from the combined PCP/offload investigation: GLM-5.2-NVFP4, PCP4+TP1+EP4+DCP4 → TP4 scored 28/32 GSM8K with either GPU caching or CPU offload, versus 26/32 direct TP4. All 32 GPU/CPU pairs matched tokens and log probabilities. That run also included separate ordering/layout/offload fixes; it does not isolate this patch's model-quality effect.

AI assistance: OpenAI Codex. Reviewed by submitter

Break insertion-sort cutoff ties by token index instead of atomic arrival order. Cover prefill and decode selection at top-k 512 and 2048.

Co-authored-by: OpenAI Codex

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
@mergify mergify Bot added the bug Something isn't working label Sep 12, 2026
@LucasWilkinson
LucasWilkinson marked this pull request as ready for review September 12, 2026 14:50

@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.

@LucasWilkinson LucasWilkinson added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 12, 2026
@LucasWilkinson

Copy link
Copy Markdown
Collaborator Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #88522 for commit 9598843057f8.

Co-authored-by: OpenAI Codex

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
@LucasWilkinson

Copy link
Copy Markdown
Collaborator Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #88526 for commit ffa9ad390a86.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant