Repository navigation
[Feat] Support Pipeline Resolution Based on HF Config & Make Pipeline Registry Eager - #3760
Conversation
hsliuustc0106
left a comment
There was a problem hiding this comment.
BLOCKING:
- Test Coverage — This PR adds new pipeline resolution logic but doesn't include any tests:
- Test for the new function (both paths: enable_audio_output=True/False)
- Test for the method with both callable and PipelineConfig objects
- Test for the behavior on single-stage pipelines
- Test that Qwen3-Omni-30B-A3B-Captioner loads successfully as thinker-only
Please add tests to verify the new resolver logic works correctly.
hsliuustc0106
left a comment
There was a problem hiding this comment.
BLOCKING: Missing tests for new pipeline resolution logic. See inline comments.
8b2bff3 to
d112200
Compare
|
@alex-jw-brooks Hello, any updates? |
|
Hey @Gaohan123, I have been a bit focused on other threads, sorry! This PR is now pretty large since it's also cleaning up the pipeline registry and removing a lot of dead code. I will rebase this PR, update the description to be more accurate, and move it out of draft tomorrow |
2da150e to
c71ede8
Compare
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Hi @hsliuustc0106 @Gaohan123! This PR ended up being larger than I originally expected since it took a bit of refactoring to make the registry eager, but I think it is finally ready for a review when you have time 🙏 I've updated the body with the main changes |
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
952abad to
3dc7f06
Compare
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
… Registry Eager (vllm-project#3760) Signed-off-by: Alex Brooks <albrooks@redhat.com>
Purpose
FIX #3757
While most models are consistent about their usage of stages within a pipeline, some models may only use a subset of the stages, and will run into issues if we try to initialize all of the stages. Such an example is
Qwen/Qwen3-Omni-30B-A3B-Instruct, which should bethinkeronly, but breaks when trying to initialize the talker/code2wav.To this end, we need a mechanism for resolving the correct pipeline configuration given the HF config, e.g., for the case of Qwen3Omni, selecting the thinker only configuration if
enable_audio_outputisFalse, or all stages ifenable_audio_outputisTrue.Changes in this PR:
More general / architectural changes:
.__init__s across diffusion pipelines to avoid unintentional heavy imports, since things declared here don't seem to be used anywaySince resolvers should be 1:1 with HF Transformer config types, they should be super simple and wrapped in a decorator that declares the type they are attached to; the decorator just ensures that
Noneis returned (i.e., no pipeline config) if the type is not the same. Example:Minor / model specific changes:
async_chunkfor single stage models; currently it defaults toTrueand breaks the single stage model, but this doesn't make sense for single stage scenarios