Repository navigation
[CI] Fix Structured Diffusion Config Wan Test - #7707
Conversation
Signed-off-by: Alex Brooks <albrooks@redhat.com>
|
This PR appears to belong to: docs/design/module/diffusion/index.md. Module owners: @david6666666 @Isotr0py @princepride Routing: @david6666666 via module named in the PR description; @Isotr0py via module named in the PR description; @princepride via module named in the PR description @alex-jw-brooks, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
linyueqian
left a comment
There was a problem hiding this comment.
Approving at the current head. This fixes the failure I filed in #7704: the test I let through in #6849 called get_diffusion_od_config() on the Omni object, which has no such method, so Diffusion · Wan22 Test raised AttributeError on the first main build that ran it. The fix reads the real OmniDiffusionConfig off the single diffusion stage client and asserts its type, and the remaining assertions (stage_id, model, model_class_name == "WanPipeline", output_type == "pil") now run against that object, whose dataclass carries all four fields. That is also more honest than the engine helper the test probably meant, which returns a metadata namespace rather than the config. Test-only change, one file, no runtime code touched. Validation: static read of the diff and of OmniDiffusionConfig and the stage client; alex reports the offline and online test_struct* cases pass locally with the merge-pipeline command. Since the ready lane does not run this advanced_model test, the confirming run is this PR's own post-merge main build, which will include the Wan22 step because the test file changed; I will watch it.
…t#7707) Signed-off-by: Alex Brooks <albrooks@redhat.com> Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
…t#7707) Signed-off-by: Alex Brooks <albrooks@redhat.com>
Purpose
Fixes #7704
get_diffusion_od_configisn't a valid method on theOmniclass. There's one on the engine class here, which is probably what was originally intended, but this is a misnomer and gives a simple namespace with some of the model's metadata rather than the actualOmniDiffusionConfig😞The minimal fix is to pull the real
OmniDiffusionConfigoff of the stage clients. You can verify the test now passes with the command below:CC @linyueqian @yenuo26 PTAL. Thanks!
Passing build link: https://buildkite.com/vllm/vllm-omni/builds/15454/canvas