[BigFIx][CI] Fix a resample bug and enable mrv2 in Mimimax M2.5 - #16920
AuroraEmiya wants to merge 1 commit into
Conversation
Signed-off-by: AuroraEmiya <Sakura.iostream@gmail.com>
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 sampling logic issue in the vLLM Ascend backend by adding a noise salt to the resampling process. Additionally, it updates the E2E test configuration for the MiniMax-M2.5 model to utilize the V2 model runner, ensuring better compatibility and performance testing. 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
|
|
👋 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. |
There was a problem hiding this comment.
Code Review
This pull request enables the V2 model runner in the E2E test configuration for MiniMax M2.5 and fixes a bug in the speculative decoding categorical resampling kernel by ensuring residual sampling uses a separate RNG domain via a noise salt. The review feedback suggests updating the PR title and summary to match the repository style guide, and replacing the use of tl.where with a standard Python ternary operator to avoid potential compilation or performance issues on Triton backends.
|
/nightly MiniMax-M2.5-w8a8-QuaRot-A2
|
Port vllm-project#16920: salt the RNG position when resampling a random residual so residual draws do not reuse the rejection-conditioned noise. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: yjyang62 <yjyang62@users.noreply.github.com>
Port vllm-project#16920: salt the RNG position when resampling a random residual so residual draws do not reuse the rejection-conditioned noise. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: yjyang62 <yjyang62@users.noreply.github.com>
Port vllm-project#16920: salt the RNG position when resampling a random residual so residual draws do not reuse the rejection-conditioned noise. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: yjyang62 <yjyang62@users.noreply.github.com>
Port vllm-project#16920: salt the RNG position when resampling a random residual so residual draws do not reuse the rejection-conditioned noise. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: yjyang62 <yjyang62@users.noreply.github.com>
Port vllm-project#16920: salt the RNG position when resampling a random residual so residual draws do not reuse the rejection-conditioned noise. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: yjyang62 <yjyang62@users.noreply.github.com>
Port vllm-project#16920: salt the RNG position when resampling a random residual so residual draws do not reuse the rejection-conditioned noise. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: yjyang62 <yjyang62@users.noreply.github.com>
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
What this PR does / why we need it?
This PR contains two related fixes for the MiniMax-M2.5 + Eagle3 MRV2 nightly path:
Fix residual resampling RNG coupling in MRV2 speculative decoding.
In the stochastic rejection path, the rejection decision and the residual categorical resample could use the same RNG domain:
For the first rejected speculative token, both paths can therefore consume the same uniform random value. However, once rejection has happened, that random value is already conditioned by the rejection event and is no longer an independent
Uniform(0, 1)draw for residual sampling.This PR keeps the rejection decision unchanged and moves only random non-bonus residual resampling into a separate RNG domain:
The salt is applied only when
is_random_residualis true. Greedy paths, bonus-token sampling, residual probability mass computation, ordinary categorical sampling, and draft sampling are unchanged.Explicitly enable MRV2 for the MiniMax-M2.5 A2 nightly case.
PR Revert "[Feature][MRV2] expand default MRv2 architecture whitelist and add dspark" #16832 reverted the default MRV2 architecture whitelist introduced by [Feature][MRV2] expand default MRv2 architecture whitelist and add dspark #16626. After that revert, Ascend enables Model Runner V2 only when
VLLM_USE_V2_MODEL_RUNNERis explicitly set.The MiniMax-M2.5 A2 nightly configuration did not set this environment variable, so the test would otherwise fall back to MRV1 and would no longer exercise the MRV2 speculative-decoding path fixed above.
This PR adds:
to the MiniMax-M2.5 A2 nightly configuration.
Related:
Does this PR introduce any user-facing change?
Yes, for the affected MRV2 stochastic speculative-decoding path.
Residual resampling after a random rejection now uses an independent RNG domain, so fixed-seed token trajectories may differ from the previous implementation. The sampling API, probability definition, model inputs/outputs, and configuration schema are unchanged.
The additional
VLLM_USE_V2_MODEL_RUNNER=1change only affects the MiniMax-M2.5 A2 nightly test configuration and ensures that CI continues to exercise MRV2 after #16832.How was this patch tested?
1. Full MiniMax-M2.5 GPQA validation
The R-FIX implementation was validated with three independent full GPQA Diamond runs using the production-like MiniMax-M2.5 MRV2 + Eagle3 configuration.
Common configuration:
R-FIX results:
All three runs completed with 198 valid results and zero request failures.
For reference, the current single-FP32 categorical baseline was observed at:
These runs do not show an accuracy-mean improvement from this patch, and this PR does not claim one. They do show that the fixed MRV2 path executes successfully end-to-end and that the three observed R-FIX runs had substantially lower run-to-run spread. More repetitions would be needed to make a statistical stability claim.
2. Source isolation
The validated R-FIX
resample.pyused for the three GPQA runs had SHA256:The only intended semantic change in that file is the RNG-domain separation for
is_random_residual.3. CI configuration
The MiniMax-M2.5 A2 nightly config explicitly enables:
so that after #16832 the nightly continues to test the MRV2 + Eagle3 path rather than silently falling back to MRV1.