Skip to content

Conversation

@DarkLight1337
Copy link
Member

@DarkLight1337 DarkLight1337 commented Dec 5, 2025

Purpose

Move out renderer-specific fields into a new config.

  • ModelConfig.tokenizer -> RendererConfig.tokenizer
  • ModelConfig.tokenizer_mode -> RendererConfig.tokenizer_mode
  • ModelConfig.tokenizer_revision -> RendererConfig.tokenizer_revision
  • ModelConfig.skip_tokenizer_init -> RendererConfig.skip_tokenizer_init
  • ModelConfig.io_processor_plugin -> RendererConfig.io_processor_plugin
  • MultiModalConfig.media_io_kwargs -> RendererConfig.media_io_kwargs
  • ModelConfig.allowed_local_media_path -> RendererConfig.allowed_local_media_path
  • ModelConfig.allowed_media_domains -> RendererConfig.allowed_media_domains

Since renderer may still need to access model config fields (like hf_config), RendererConfig contains ModelConfig. This also means a bunch of tests need to be updated to pass RendererConfig(model_config=model_config) explicitly to VllmConfig.

Related changes:

  • Separate ModelConfig.maybe_pull_model_tokenizer_for_runai into ModelConfig.maybe_pull_model_for_runai and RendererConfig.maybe_pull_tokenizer_for_runai. Also fix the latter using model instead of tokenizer.
  • Refactor ModelConfig.get_and_verify_max_len into ModelConfig.recalculate_max_model_len and call it twice during initialization: inside ModelConfig.__post_init__ without tokenizer, and after RendererConfig is constructed with the tokenizer.
  • Update tokenizer/processor factories and chat utils to accept renderer_config instead of model_config.
  • Update multimodal registry and budget to accept renderer_config instead of model_config.
  • Update SupportsTranscription interface methods to accept renderer_config instead of model_config.
  • Add _HfExamplesInfo.build_model_config and _HfExamplesInfo.build_renderer_config convenience methods for testing.

Prepare for #22880

Test Plan

Test Result


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: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 5, 2025
@DarkLight1337 DarkLight1337 changed the title [Refactor] Separate out RendererConfig from ModelConfig` [Refactor] Separate out RendererConfig from ModelConfig Dec 5, 2025
@mergify
Copy link

mergify bot commented Dec 5, 2025

Documentation preview: https://vllm--30145.org.readthedocs.build/en/30145/

@mergify mergify bot added documentation Improvements or additions to documentation deepseek Related to DeepSeek models frontend llama Related to Llama models labels Dec 5, 2025
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337
Copy link
Member Author

/gemini review

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 is a significant refactoring that separates renderer-specific configurations into a new RendererConfig class, moving them out of ModelConfig. The changes are extensive and touch many files, but they appear to be consistent with the goal of this refactoring. I've identified two critical issues in the new vllm/config/renderer.py file that could affect loading models and tokenizers from S3 URIs (a Run:ai feature). My review comments provide details and suggestions for fixing these issues.

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337
Copy link
Member Author

/gemini review

@DarkLight1337 DarkLight1337 added the ready-run-all-tests Trigger CI with all tests for wide-ranging PRs label Dec 5, 2025
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 is a significant refactoring that separates renderer-specific configurations from ModelConfig into a new RendererConfig class. The changes are extensive and applied consistently across the codebase, improving separation of concerns. However, I've identified a critical issue in the recalculate_max_model_len method's signature which could lead to a TypeError during engine initialization if max_model_len is not explicitly provided by the user. The proposed fixes address this issue.

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337
Copy link
Member Author

/gemini review

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 is a large but well-executed refactoring that separates renderer-specific configurations from the core model configuration into a new RendererConfig class. This improves code structure and separation of concerns. The changes are applied consistently across the codebase. I have found one potential issue that needs attention.

@mergify
Copy link

mergify bot commented Dec 5, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @DarkLight1337.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Dec 5, 2025
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337 DarkLight1337 changed the title [Refactor] Separate out RendererConfig from ModelConfig [Renderer] Separate out RendererConfig from ModelConfig Dec 6, 2025
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) December 6, 2025 04:40
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deepseek Related to DeepSeek models documentation Improvements or additions to documentation frontend kv-connector llama Related to Llama models multi-modality Related to multi-modality (#4194) qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed ready-run-all-tests Trigger CI with all tests for wide-ranging PRs speculative-decoding structured-output tpu Related to Google TPUs v1

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants