-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
[Model] vllm v1 support mlp_speculator #20655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: skylee-01 <497627264@qq.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @skylee-01, 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 introduces comprehensive support for mlp_speculator
as a new speculative decoding method in vLLM v1. It involves updating configuration utilities to recognize the new method, adapting the MLPSpeculator
model for its role as a draft model, and integrating a dedicated proposer class into the GPU model runner to manage the generation of speculative tokens.
Highlights
- New Speculative Decoding Method: Added support for "mlp_speculator" as a new speculative decoding method within vLLM v1, allowing its configuration and use.
- MLPSpeculator Model Adaptation: Modified the
MLPSpeculator
model's initialization to correctly handle configurations when used as a draft model in speculative decoding, and adjusted itsgenerate_proposals
method for conditionalsampling_metadata
usage. - New Proposer Class: Introduced
MLPSpeculatorProposer
(vllm/v1/spec_decode/mlp_speculator.py
) to encapsulate the logic for generating draft tokens specifically for theMLPSpeculator
model. - Integration into GPU Model Runner: Integrated
MLPSpeculatorProposer
intoGPUModelRunner
to enable themlp_speculator
method, including the necessary data preparation and call flow for proposing draft tokens.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize 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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this 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 adds support for mlp_speculator
in vLLM v1. The changes are in the right direction, but I've found several critical issues related to the integration with the v1 engine, particularly in vllm/model_executor/models/mlp_speculator.py
and vllm/v1/worker/gpu_model_runner.py
. These issues could lead to runtime errors or incorrect behavior. I've provided detailed suggestions to fix them. Addressing these points will be important for the stability of this new feature.
if hasattr(vllm_config, 'speculative_config'): | ||
config = vllm_config.speculative_config.draft_model_config.hf_config | ||
self.sampling_metadata_is_required = False | ||
else: | ||
config = vllm_config.model_config.hf_config | ||
self.sampling_metadata_is_required = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic to differentiate between v0 and v1 paths is incomplete. For the v1 path (hasattr(vllm_config, 'speculative_config')
), self.sampler
and self.logits_processor
are still initialized with v0 components at the end of __init__
(lines 147-149), which will cause a crash due to type mismatches with SamplingMetadata
.
To fix this, the initialization of sampler
and logits_processor
should be moved inside this if/else
block. For the v1 path, self.sampler
should be an instance of vllm.v1.sample.sampler.Sampler
and self.logits_processor
should be None
.
👋 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 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 🚀 |
Signed-off-by: skylee-01 <497627264@qq.com>
Signed-off-by: skylee-01 <497627264@qq.com>
Signed-off-by: skylee-01 <497627264@qq.com>
Signed-off-by: skylee-01 <497627264@qq.com>
Hi Curys, please help me review this PR. Thank you very much. @DarkLight1337 |
@WoosukKwon can you review this? I'm not that familiar with spec decode code |
Teacher, could you please help me take a look at this pull request? @WoosukKwon |
Teacher Lily, please help me review it. @LiuXiaoxuanPKU |
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.Purpose
vllm v1 support mlp_speculator
Test Plan
Test Result
(Optional) Documentation Update