-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[feat] Support EAGLE for Qwen2 #23158
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
👋 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 🚀 |
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 EAGLE for Qwen2 models. The implementation in vllm/model_executor/models/qwen2_eagle.py
has a couple of critical issues. The __init__
method of EagleQwen2ForCausalLMEagle
is incomplete, missing initialization for lm_head
and lora_config
, which will lead to runtime errors. Additionally, the load_weights
method does not return the set of loaded weights, which breaks the contract of the weight loading interface in vLLM. I've provided suggestions to fix these issues.
def __init__(self, *, vllm_config: VllmConfig, prefix: str = ""): | ||
nn.Module.__init__(self) | ||
self.config = vllm_config. \ | ||
speculative_config.draft_model_config.hf_config | ||
target_layer_num = vllm_config.model_config.get_num_layers( | ||
vllm_config.parallel_config) | ||
self.model = Qwen2Model(vllm_config=vllm_config, | ||
prefix="model", | ||
start_layer_id=target_layer_num) | ||
|
||
logit_scale = getattr(self.config, "logit_scale", 1.0) | ||
self.logits_processor = LogitsProcessor(self.config.vocab_size, | ||
scale=logit_scale) |
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 __init__
method is incomplete. It inherits from Qwen2ForCausalLM
but doesn't initialize all necessary attributes. Specifically, self.lm_head
is not initialized, which is used by the inherited compute_logits
method, and self.lora_config
is not set, which is required by the SupportsLoRA
interface. This will lead to runtime errors.
def __init__(self, *, vllm_config: VllmConfig, prefix: str = ""):
nn.Module.__init__(self)
self.config = vllm_config.speculative_config.draft_model_config.hf_config
self.lora_config = vllm_config.lora_config
self.quant_config = vllm_config.quant_config
target_layer_num = vllm_config.model_config.get_num_layers(
vllm_config.parallel_config)
self.model = Qwen2Model(vllm_config=vllm_config,
prefix="model",
start_layer_id=target_layer_num)
from vllm.distributed import get_pp_group
from vllm.model_executor.layers.vocab_parallel_embedding import (
ParallelLMHead)
from vllm.model_executor.models.utils import (PPMissingLayer,
maybe_prefix)
if get_pp_group().is_last_rank:
self.lm_head = ParallelLMHead(
self.config.vocab_size,
self.config.hidden_size,
quant_config=self.quant_config,
prefix=maybe_prefix(prefix, "lm_head"))
else:
self.lm_head = PPMissingLayer()
logit_scale = getattr(self.config, "logit_scale", 1.0)
self.logits_processor = LogitsProcessor(self.config.vocab_size,
scale=logit_scale)
Signed-off-by: 纬杭 <[email protected]>
5765592
to
46e8a5b
Compare
Signed-off-by: 纬杭 <[email protected]>
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.
Thanks for updating the PR, can you make use of the newly-introduced SupportsEagle3
interface?
Sure |
Signed-off-by: 纬杭 <[email protected]>
Signed-off-by: 纬杭 <[email protected]>
…/vllm into qwq_eagle2_support
Signed-off-by: 纬杭 <[email protected]>
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.
LGTM, thanks
PTAL at the failing model test. By the way, Eagle3 support is going to be added soon (#23337), so not sure whether this PR is still needed |
Purpose
new PR for #21363
Test Plan
#21363
Test Result
#21363
(Optional) Documentation Update
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.