Skip to content

Conversation

hmellor
Copy link
Member

@hmellor hmellor commented Sep 15, 2025

Part of #18953

…tive.py`

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
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 refactors the codebase by moving SpeculativeConfig into its own file, vllm/config/speculative.py, from the large vllm/config/__init__.py. This is a good step towards better code organization.

The changes in existing files are mostly updating the import paths for SpeculativeConfig, which are all correct.

However, the new file vllm/config/speculative.py introduces a circular dependency by importing ModelConfig from vllm.config, while vllm.config also imports from vllm.config.speculative. I've left a comment with a suggestion on how to resolve this using forward references and local imports, which is a common pattern to break such cycles.

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 16, 2025
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@hmellor hmellor merged commit 0faf3cc into vllm-project:main Sep 16, 2025
41 checks passed
@hmellor hmellor deleted the extract-speculative-config branch September 16, 2025 11:51
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
…tive.py` (vllm-project#24904)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance-related issues qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed speculative-decoding v1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants