Skip to content

Conversation

@zhuohan123
Copy link
Member

@zhuohan123 zhuohan123 commented Feb 12, 2026

Purpose

Corrected the description of num_logprobs parameter. It should be maximum number of logprobs across the requests instead of minimum.

Test Plan

Test Result


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.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Corrected the description of num_logprobs parameter.

Signed-off-by: Zhuohan Li <[email protected]>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request corrects the description of the num_logprobs parameter in the gather_logprobs function's docstring. The change from 'minimum' to 'maximum' is accurate, as this parameter represents the highest number of logprobs requested across all sequences in a batch, and torch.topk is used to retrieve that number of top logprobs. The change improves the clarity and correctness of the documentation. The change is correct and I have no further suggestions.

@yeqcharlotte yeqcharlotte enabled auto-merge (squash) February 12, 2026 21:33
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Feb 12, 2026
@vllm-bot vllm-bot merged commit d707678 into main Feb 13, 2026
45 of 50 checks passed
@vllm-bot vllm-bot deleted the zhuohan/sampler-comment-fix branch February 13, 2026 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants