Conversation
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [BugFix] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in the speculative decoding process where zero-draft requests were unnecessarily consuming RNG state. By conditionally skipping exponential sampling for requests without draft tokens, the implementation ensures consistent RNG behavior and improves reproducibility for fixed-seed speculative decoding. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Ops][BugFix] Avoid advancing RNG state for requests with zero draft tokensSuggested PR Summary:
### What this PR does / why we need it?
This PR optimizes the rejection sampler by avoiding random number generation for requests that have zero draft tokens. Previously, random numbers were generated for all generators regardless of whether they had draft tokens, which unnecessarily advanced the RNG state and impacted reproducibility. Now, the RNG state is only advanced for active draft requests.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added unit tests in `tests/ut/sample/a2/test_rejection_sampler_rng.py` to verify generator state preservation and independence.I have no further feedback as the changes are well-tested and correctly address the reproducibility issue.
e011d4f to
05f0855
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: yaleyoou <yaleyoou@gmail.com>
05f0855 to
0ad5bc8
Compare
|
@realliujiaxu @wangxiyuan @Yikun Could one of you please add the ready-precise label? Test selection has completed and cigate is waiting for it. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
What this PR does / why we need it?
sample_recovered_tokens()consumed per-request RNG for zero-draftrequests and then discarded the generated exponential samples with
torch.where. This made later rejection sampling depend on schedulingand batch composition.
Use the existing Python
num_draft_tokensdata to skip generator-awaredraws for zero-draft rows. Active-draft and unseeded sampling behavior
remain unchanged. Add NPU regression coverage for zero, active, mixed,
independent, partial-generator, and skipped-round cases.
Fixes #13608
Does this PR introduce any user-facing change?
No API change. Fixed-seed speculative decoding now preserves request RNG
state during zero-draft rounds.
How was this patch tested?
7 new focused NPU regression tests passed
31 existing NPU rejection-kernel tests passed
19 rejection-sampler unit tests passed
85 spec-decode unit tests passed
No scalar synchronization, new H2D copy, or persistent HBM growth was observed
git diff --checkand relevant pinned formatting/lint hooks passedFull
format.sh cidid not complete because actionlint's Go dependencydownload timed out at the proxy
Ascend 910B3, CANN 9.0.0, torch 2.10.0,
torch-npu 2.10.0.post2, triton-ascend 3.2.1
vllm-project/vllm@0351e9a
vLLM version: v0.27.1
vLLM main: vllm-project/vllm@cdc4824