[Test] Add OmniGen2 online serving expansion L4 tests for Ulysses-SP and CFG-Parallel#2326
[Test] Add OmniGen2 online serving expansion L4 tests for Ulysses-SP and CFG-Parallel#2326yangjianjuan wants to merge 1 commit into
Conversation
test feature: SP and CFP-Parallel Signed-off-by: yangjianjuan <510818155@qq.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be6ce66aeb
ℹ️ 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".
| "--cfg-parallel-size", | ||
| "2", |
There was a problem hiding this comment.
Drop CFG-parallel case for OmniGen2 expansion test
This case claims CFG-parallel coverage for OmniGen2, but OmniGen2Pipeline does not use CFGParallelMixin (or any rank-aware CFG split path), so --cfg-parallel-size 2 does not actually exercise CFG-parallel behavior and can pass while that feature is effectively untested for this model. In practice this creates false confidence: regressions in real CFG-parallel execution would not be caught by this test.
Useful? React with 👍 / 👎.
|
Could you double check it? Cause I think OmniGen2 does not support SP or CFG-Parallel for now. Besides, there is RFC related to L4 tests #1832 . Please request for task assignment first. |
OK :-) |
|
I double checked, and realized that if usp and cfg-parallel were enabled, but those models didn't support the two features, the inference will not fail, but disable the two features automatically. Sorry I will postpone the merge of this PR, until OmniGen2 indeed support the two features. Would you like to raise a PR to support SP for OmniGen2? I will sync it to #1217 . CFG-Parallel of OmniGen2 is in #2074. |
OK, I accept to support SP for OmniGen2. |
|
@yangjianjuan Hi, may I ask about the progress of the SP support for OmniGen2? |
Sorry, I'm currently focused on another priority. I’ll be able to get started on this next week. |
Purpose
This PR adds L4 online serving expansion tests for OmniGen2 models
test features:
Test Plan
pytest test_omni_gen2_expansion.py -m 'diffusion' -s -vTest Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model. Please runmkdocs serveto sync the documentation editions to./docs.BEFORE SUBMITTING, PLEASE READ https://github.com/vllm-project/vllm-omni/blob/main/CONTRIBUTING.md (anything written below this line will be removed by GitHub Actions)