Skip to content

Conversation

@nandan2003
Copy link

@nandan2003 nandan2003 commented Nov 21, 2025

Purpose

Fixes a circular import error where vllm.model_executor.model_loader.utils could not be imported because it attempted to import from models.adapters at the top level, while models.adapters (indirectly) depends on utils.

Fixes #29110

Test Plan

I verified this locally by attempting to import the module in an isolated script to ensure the circular dependency chain is broken.

Verification Script:

import torch

# Mock compile to bypass python 3.14+ restriction if needed for testing import logic only
def no_op_compile(*args, **kwargs):
    def decorator(func):
        return func
    return decorator
torch.compile = no_op_compile

try:
    from vllm.model_executor.model_loader.utils import process_weights_after_loading
    print("Success! Circular dependency is fixed.")
except ImportError as e:
    print(f"FAILURE: {e}")

Test Result

  • Before Fix: ImportError: cannot import name 'process_weights_after_loading' from 'vllm.model_executor.model_loader.utils'
  • After Fix: Success! Circular dependency is fixed.

Additional Context

The import of as_embedding_model, as_reward_model, as_seq_cls_model, and try_create_mm_pooling_model_cls was moved from the global scope of utils.py into get_model_architecture() where they are actually used. This defers the import execution and resolves the cycle.

@nandan2003 nandan2003 requested a review from 22quinn as a code owner November 21, 2025 17:31
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 correctly resolves a circular import issue by moving imports from the global scope into the function where they are used. This is a good and standard approach. However, I've found a critical issue where the type hint for the _get_model_architecture function was changed, making it inconsistent with its implementation and usage, which will likely lead to errors. Please see the detailed comment for the required fix.

nandan2003 and others added 2 commits November 21, 2025 23:44
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Nandan Vallamdasu  <[email protected]>
Signed-off-by: nandan2003 <[email protected]>
@nandan2003
Copy link
Author

Hi

The pre-commit/mypy failures in vllm/v1/worker/... appear unrelated to my changes.

My modification is strictly isolated to vllm/model_executor/model_loader/utils.py to resolve the circular dependency. Verified the fix locally by successfully importing the module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ImportError: process_weights_after_loading missing in vLLM = 0.8.4 breaks VERL rollout initialization

1 participant