[Sampler] Support returning final logprobs by 22quinn · Pull Request #22387 · vllm-project/vllm

22quinn · 2025-08-06T19:54:45Z

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.

Purpose

RL workflows may need to get the final logprobs, after applying all logit processors including top_k/top_p, temperature. This PR updates the logprobs_mode's processed_logprobs, processed_logits to return the final values.

Test Plan

pytest tests/v1/sample/test_logprobs.py -k 'test_logprobs_mode'

Test Result

Unit tests passed

(Optional) Documentation Update

Added detailed sampling flow illustration in v1 Sampler class.
Updated v1 guide.
https://vllm--22387.org.readthedocs.build/en/22387/api/vllm/v1/sample/sampler.html#vllm.v1.sample.sampler.Sampler
https://vllm--22387.org.readthedocs.build/en/22387/usage/v1_guide.html#logprobs-calculation

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

github-actions · 2025-08-06T19:54:54Z

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

gemini-code-assist

Code Review

This PR introduces a new sampled_logprobs mode. I've identified a critical issue in vllm/v1/sample/sampler.py where the sample() method returns inconsistent types, and a high-severity issue in vllm/v1/sample/ops/topk_topp_sampler.py where the CUDA implementation's behavior is inconsistent with the native and TPU implementations. These issues could lead to incorrect log-probability values and subtle bugs.

vllm/v1/sample/sampler.py

vllm/v1/sample/ops/topk_topp_sampler.py

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

22quinn · 2025-08-06T22:47:22Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces support for a new logprobs_mode, final_logprobs, which allows retrieving log probabilities after all sampling processors (including temperature, top-k, and top-p) have been applied. The overall implementation is good, but I've identified a critical issue that could lead to incorrect sampling results due to an in-place modification of the logits tensor. Additionally, there's a minor error in the documentation for the new mode. My review provides specific suggestions to address these points.

vllm/v1/sample/sampler.py

vllm/config.py

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

njhill

Thanks @22quinn!

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: Nick Hill <nhill@redhat.com> Co-authored-by: Woosuk Kwon <woosuk.kwon@berkeley.edu> Signed-off-by: Duncan Moss <djm.moss@gmail.com>

### What this PR does / why we need it? 1. use action/checkout@v5 instead of v4 2. remove dbo test case because there is issue with it and will be refactored later 3. make vllm-ascend compatible with vllm v0.10.1.1 and add CI for it 4. fix sampler api changes introduced by vllm-project/vllm#22387 6. fix qwen3 moe config changes intruoduced by vllm-project/vllm#20562 7. fix kvcache block changes introduced by vllm-project/vllm#23262 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with existing test. - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@0c6e40b --------- Signed-off-by: MengqingCao <cmq0113@163.com>

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: Nick Hill <nhill@redhat.com> Co-authored-by: Woosuk Kwon <woosuk.kwon@berkeley.edu>

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: Nick Hill <nhill@redhat.com> Co-authored-by: Woosuk Kwon <woosuk.kwon@berkeley.edu> Signed-off-by: Xiao Yu <xiao.yu@amd.com>

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: Nick Hill <nhill@redhat.com> Co-authored-by: Woosuk Kwon <woosuk.kwon@berkeley.edu>

### What this PR does / why we need it? 1. use action/checkout@v5 instead of v4 2. remove dbo test case because there is issue with it and will be refactored later 3. make vllm-ascend compatible with vllm v0.10.1.1 and add CI for it 4. fix sampler api changes introduced by vllm-project/vllm#22387 6. fix qwen3 moe config changes intruoduced by vllm-project/vllm#20562 7. fix kvcache block changes introduced by vllm-project/vllm#23262 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with existing test. - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@0c6e40b --------- Signed-off-by: MengqingCao <cmq0113@163.com>

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: Nick Hill <nhill@redhat.com> Co-authored-by: Woosuk Kwon <woosuk.kwon@berkeley.edu>

### What this PR does / why we need it? 1. use action/checkout@v5 instead of v4 2. remove dbo test case because there is issue with it and will be refactored later 3. make vllm-ascend compatible with vllm v0.10.1.1 and add CI for it 4. fix sampler api changes introduced by vllm-project/vllm#22387 6. fix qwen3 moe config changes intruoduced by vllm-project/vllm#20562 7. fix kvcache block changes introduced by vllm-project/vllm#23262 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with existing test. - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@0c6e40b --------- Signed-off-by: MengqingCao <cmq0113@163.com>

logprobs after sampling

ce9af20

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

mergify bot added v1 tpu Related to Google TPUs labels Aug 6, 2025

gemini-code-assist bot reviewed Aug 6, 2025

View reviewed changes

vllm/v1/sample/sampler.py Outdated Show resolved Hide resolved

vllm/v1/sample/ops/topk_topp_sampler.py Outdated Show resolved Hide resolved

fix

52575b0

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

mergify bot removed the tpu Related to Google TPUs label Aug 6, 2025

22quinn added 2 commits August 6, 2025 15:29

naming: final_logprobs

036cb2d

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

doc

8324d3a

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

gemini-code-assist bot reviewed Aug 6, 2025

View reviewed changes

vllm/v1/sample/sampler.py Outdated Show resolved Hide resolved

vllm/config.py Outdated Show resolved Hide resolved

fix inplace, doc

cfa03f4

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

22quinn changed the title ~~[Sampler] Support returning logprobs after sampling~~ [Sampler] Support returning final logprobs Aug 6, 2025

22quinn added 2 commits August 6, 2025 15:59

supports | None annotation

42d641b

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

simplify logic

a7914df

Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>

22quinn marked this pull request as ready for review August 6, 2025 23:21

22quinn requested review from WoosukKwon, alexm-redhat, comaniac, hmellor, houseroad, mgoin, njhill, robertgshaw2-redhat, simon-mo, tlrmchlsmth, youkaichao and ywang96 as code owners August 6, 2025 23:21

wuxibin89 mentioned this pull request Aug 7, 2025

[BREAKING][vllm, fsdp] feat: add Rollout-Training Mismatch Fix -- Truncated importance sampling verl-project/verl#2953

Merged

7 tasks

zhandaz mentioned this pull request Aug 7, 2025

feat: [1/2] Top-k and Top-p support for dtensor worker with vLLM V0 when TP==1 NVIDIA-NeMo/RL#773

Open

mergify bot removed the needs-rebase label Aug 20, 2025

njhill approved these changes Aug 21, 2025

View reviewed changes

njhill merged commit f571ff8 into vllm-project:main Aug 21, 2025
38 of 39 checks passed

MengqingCao mentioned this pull request Aug 21, 2025

[CI] fix ci vllm-project/vllm-ascend#2464

Merged

LeonEricsson mentioned this pull request Aug 27, 2025

[GRPO] Truncated Importance Sampling to address rollout-training mismatch huggingface/trl#3867

Merged

5 tasks

yuki-97 mentioned this pull request Sep 15, 2025

Upgrade to vllm 0.10.2, ray, and torch 2.8.0 NVIDIA-NeMo/RL#1122

Closed

22quinn deleted the topk-topp-logprobs branch November 16, 2025 22:28

RobotGF mentioned this pull request Dec 31, 2025

[rollout] feat: Add vllm logprob mode and default processed_logprob verl-project/verl#4755

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Sampler] Support returning final logprobs#22387

[Sampler] Support returning final logprobs#22387
njhill merged 34 commits intovllm-project:mainfrom
22quinn:topk-topp-logprobs

22quinn commented Aug 6, 2025 •

edited by github-actions bot

Loading

Uh oh!

github-actions bot commented Aug 6, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

22quinn commented Aug 6, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

njhill left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

22quinn commented Aug 6, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Essential Elements of an Effective PR Description Checklist

Purpose

Test Plan

Test Result

(Optional) Documentation Update

Uh oh!

github-actions bot commented Aug 6, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

22quinn commented Aug 6, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

njhill left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

22quinn commented Aug 6, 2025 •

edited by github-actions bot

Loading