Skip to content

Fix EagleMistralLarge3Model initialization#37232

Merged
ywang96 merged 1 commit intovllm-project:mainfrom
juliendenize:fix_mistral_eagle
Mar 16, 2026
Merged

Fix EagleMistralLarge3Model initialization#37232
ywang96 merged 1 commit intovllm-project:mainfrom
juliendenize:fix_mistral_eagle

Conversation

@juliendenize
Copy link
Copy Markdown
Contributor

@juliendenize juliendenize commented Mar 16, 2026

Purpose

This PR fixes initialization of EagleMistralLarge3Model due to #36361 that added aux_hidden_state_layers init requirement.

Test Plan

ran an inference

Test Result

on main it raises error, now it works :)


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.

Signed-off-by: juliendenize <julien.denize@mistral.ai>
Copy link
Copy Markdown
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 correctly fixes an initialization error in EagleMistralLarge3Model by adding the missing aux_hidden_state_layers attribute. The error arose because the model's __init__ method does not call super().__init__(), making it brittle to changes in its base class, DeepseekV2Model. My review includes a suggestion to add a TODO comment to track this structural issue and encourage a future refactoring to improve maintainability.

prefix=maybe_prefix(prefix, "fc"),
)
self.norm = RMSNorm(config.hidden_size, eps=config.rms_norm_eps)
self.aux_hidden_state_layers: tuple[int, ...] = ()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

While this change correctly initializes aux_hidden_state_layers, it highlights a broader maintainability issue. The __init__ method of EagleMistralLarge3Model does not call super().__init__() from its base class DeepseekV2Model, but instead re-implements much of its logic. This is why aux_hidden_state_layers had to be added here manually after it was added to the base class.

To prevent similar issues in the future and to track this technical debt, I suggest adding a TODO comment. This will make it clear that a refactoring is needed to make the class more robust against changes in its parent.

        # TODO: Refactor to call super().__init__ from DeepseekV2Model
        # to avoid missing future attributes.
        self.aux_hidden_state_layers: tuple[int, ...] = ()

@ywang96 ywang96 added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 16, 2026
@ywang96 ywang96 merged commit 7961486 into vllm-project:main Mar 16, 2026
6 of 8 checks passed
Lucaskabela pushed a commit to Lucaskabela/vllm that referenced this pull request Mar 17, 2026
Signed-off-by: juliendenize <julien.denize@mistral.ai>
wendyliu235 pushed a commit to wendyliu235/vllm-public that referenced this pull request Mar 18, 2026
Signed-off-by: juliendenize <julien.denize@mistral.ai>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: juliendenize <julien.denize@mistral.ai>
JiantaoXu pushed a commit to JiantaoXu/vllm that referenced this pull request Mar 28, 2026
Signed-off-by: juliendenize <julien.denize@mistral.ai>
mtparet pushed a commit to blackfuel-ai/vllm that referenced this pull request Apr 9, 2026
Signed-off-by: juliendenize <julien.denize@mistral.ai>
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 speculative-decoding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants