Skip to content

Conversation

@fake0fan
Copy link
Contributor

To align with our initial vision and for future overall optimization, we gradually began to provide Stage abstractions for Diffusion.

image

[Core] Add Stage Abstraction Support for Diffusion Models

Overview

This PR adds stage abstraction support for the diffusion model component of vLLM-Omni, achieving a consistent architectural design with LLM models. It also includes code refactoring to unify the sampling parameter interface, improving code maintainability and extensibility.

Major Changes

1. Code Refactoring

  • Refactored entry point code structure: Refactored and integrated LLM-related code from omni_llm.py into omni.py, unifying entry point management
  • Enhanced Stage abstraction: Extended omni_stage.py to support stage configuration and management for diffusion models

2. Diffusion Stage Abstraction Support

  • New unified sampling parameter class (omni_sampling_params.py):
    • Created OmniSamplingParams class to uniformly manage sampling parameters for both LLM and diffusion models
    • Supports LLM parameters (temperature, top_p, top_k, etc.) and diffusion parameters (num_inference_steps, guidance_scale, etc.)
    • Provides conversion methods with vLLM SamplingParams
  • Extended Diffusion Engine:
    • Updated diffusion_engine.py to support stage abstraction
    • Enhanced stage support in gpu_worker.py
  • Updated configuration system:
    • Added configuration file: QwenImagePipeline.yaml
    • Updated stage configuration files for multiple models

3. Example Updates

  • Updated text_to_image.py example to demonstrate how to use the new stage abstraction interface

4. Outputs

image

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

assert model != "", "Null model id detected, please specify a model id."
model = omni_snapshot_download(model)
if args:
args[0] = model

Choose a reason for hiding this comment

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

P1 Badge Avoid assigning to immutable args tuple in Omni init

When Omni is instantiated with the model passed positionally (e.g. Omni("Qwen/Qwen-Image")), the constructor assigns to args[0], but args is a tuple, so the assignment raises TypeError: 'tuple' object does not support item assignment before any initialization occurs. This makes the new entrypoint unusable for positional calls that previously worked with OmniLLM; callers must now pass model as a keyword or hit a hard crash.

Useful? React with 👍 / 👎.

@ZJY0516
Copy link
Collaborator

ZJY0516 commented Dec 20, 2025

looking forward to it

@hsliuustc0106
Copy link
Collaborator

let's get the initial version done before 1230 release

@ZJY0516
Copy link
Collaborator

ZJY0516 commented Dec 22, 2025

Does this pr support reuse vllm as text encoding stage for diffusion models?

@fake0fan
Copy link
Contributor Author

Does this pr support reuse vllm as text encoding stage for diffusion models?

Not yet.

This PR only encapsulates the entire diffusion model into a single stage first.

@@ -0,0 +1,36 @@
# stage config for running Qwen-Image with diffusion stage type.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have concerns about adding this for diffusion models, as I believe it introduces a significant UX drawback.

Copy link
Collaborator

Choose a reason for hiding this comment

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

agree

@princepride
Copy link
Contributor

Does this PR mean that all models under diffusion folder can be deployed using YAML?

@fake0fan
Copy link
Contributor Author

Does this PR mean that all models under diffusion folder can be deployed using YAML?

Through some offline discussions, we decided that this version will not require providing a yaml file for the Diffusion model. Instead, the system will automatically generate a YAML file for the current Diffusion model.

@princepride
Copy link
Contributor

In which group you are discussing? Can you add me?

@erfgss
Copy link

erfgss commented Dec 23, 2025

Fixes #340

Signed-off-by: Chenguang ZHENG <[email protected]>
Signed-off-by: Chenguang ZHENG <[email protected]>
@fake0fan fake0fan changed the title [WIP][Core] Supports stage abstraction in the diffusion model [Core] Supports stage abstraction in the diffusion model Dec 23, 2025
Signed-off-by: Chenguang ZHENG <[email protected]>
@david6666666 david6666666 added the ready label to trigger buildkite CI label Dec 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants