Skip to content

Conversation

@bangshengtang
Copy link
Collaborator

Summary: internally we use a custom plugin to handle our customized config format, without this change, draft model config will be default to "auto", this has caused mismatches

Test Plan: CI

Differential Revision: D88501036

Summary: internally we use a custom plugin to handle our customized config format, without this change, draft model config will be default to "auto", this has caused mismatches

Test Plan: CI

Differential Revision: D88501036
Copy link
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 addresses an issue in speculative decoding where the draft model did not inherit the config_format from the target model. This could lead to mismatches, especially when using custom configuration formats handled by plugins. The proposed change correctly propagates the config_format from the target model's configuration to the draft model's configuration during its initialization. The fix is concise, targeted, and appears correct. I have no further comments.

@zhuohan123 zhuohan123 enabled auto-merge (squash) December 5, 2025 18:42
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 5, 2025
Copy link
Member

@yewentao256 yewentao256 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work!

@bangshengtang
Copy link
Collaborator Author

hmm, @zhuohan123 @yewentao256 the CI failure seems irrelevant

@zhuohan123 zhuohan123 disabled auto-merge December 5, 2025 21:33
@zhuohan123 zhuohan123 merged commit 77e4472 into vllm-project:main Dec 5, 2025
47 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fb-exported meta-exported ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants