Skip to content

[CI] Remove small resolution test in Qwen-Image Perf test when vae patch parallel is enabled#2872

Merged
Gaohan123 merged 1 commit into
vllm-project:mainfrom
wtomin:ci-perf-fix
Apr 20, 2026
Merged

[CI] Remove small resolution test in Qwen-Image Perf test when vae patch parallel is enabled#2872
Gaohan123 merged 1 commit into
vllm-project:mainfrom
wtomin:ci-perf-fix

Conversation

@wtomin

@wtomin wtomin commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS (AT THE BOTTOM) HAVE BEEN CONSIDERED.

Purpose

Solving #2863.

Vae patch parallelism is enabled to reduce the peak memory of long sequence decoding. It may increase the latency when the resolution is too small, because the communication overhead is larger than the parallel computation gains.

Therefore, this PR removes the "512x512_steps20" test in the Ulysses SP=2 + CFG-parallel=2 + VAE Patch Parallel=4 test case. This actually helps to stablize the nightly CI performance, preventing it from failure.

Previous CI results

resolution num_steps buildkite url latency_mean test case CI success
512x512 20 6979 3.1746
512x512 20 6662 1.92344
1536x1536 35 6979 8.44113
1536x1536 35 6662 8.34948

The table above indicates that larger resolution yields more stable performance results.

Test Plan

pytest -s -v tests/dfx/perf/scripts/run_diffusion_benchmark.py --test-config-file tests/dfx/perf/tests/test_qwen_image_vllm_omni.json

Test Result

Wait for Nightly-CI results.

cc @yenuo26 @Gaohan123 @hsliuustc0106

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. Please provide the test scripts & test commands. Please state the reasons if your codes don't require additional test scripts. For test file guidelines, please check the test style doc
  • The test results. Please paste the results comparison before and after, or the e2e results.
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model. Please run mkdocs serve to sync the documentation editions to ./docs.
  • (Optional) Release notes update. If your change is user-facing, please update the release notes draft.

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)

Signed-off-by: Didan Deng <33117903+wtomin@users.noreply.github.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@yenuo26 yenuo26 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@wtomin wtomin added the nightly-test label to trigger buildkite nightly test CI label Apr 17, 2026

@gcanlin gcanlin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@hsliuustc0106

Copy link
Copy Markdown
Collaborator

The rationale is sound: small resolutions with VAE patch parallel are indeed unstable.

Alternative approaches considered:

  • Mark as flaky and skip in CI: Still removes it from automated runs, same effect
  • Adjust baseline thresholds: Hard to define good thresholds for unstable tests
  • Conditionally enable VAE patch parallel: Adds complexity

Removing the test is the pragmatic solution here.

@hsliuustc0106

Copy link
Copy Markdown
Collaborator

Reasoning makes sense - small resolution + high parallelism = communication overhead dominates.

One suggestion: add a comment in the test config explaining why 512x512 is skipped for this parallelization config. Future reviewers may wonder why this resolution is missing otherwise.

Example:
"// 512x512 skipped with VAE patch parallel=4: communication overhead exceeds parallel gains at small resolutions, causing unstable CI latency"

@Gaohan123 Gaohan123 added ready label to trigger buildkite CI and removed nightly-test label to trigger buildkite nightly test CI labels Apr 20, 2026
@Gaohan123 Gaohan123 added this to the v0.20.0 milestone Apr 20, 2026
@Gaohan123 Gaohan123 enabled auto-merge (squash) April 20, 2026 15:54
@Gaohan123 Gaohan123 merged commit 400690f into vllm-project:main Apr 20, 2026
8 checks passed
lvliang-intel pushed a commit to lvliang-intel/vllm-omni that referenced this pull request Apr 21, 2026
…tch parallel is enabled (vllm-project#2872)

Signed-off-by: Didan Deng <33117903+wtomin@users.noreply.github.com>
nainiu258 pushed a commit to nainiu258/vllm-omni that referenced this pull request Apr 21, 2026
…tch parallel is enabled (vllm-project#2872)

Signed-off-by: Didan Deng <33117903+wtomin@users.noreply.github.com>
Signed-off-by: nainiu258 <cperfect02@163.com>
qinganrice pushed a commit to qinganrice/vllm-omni that referenced this pull request Apr 23, 2026
…tch parallel is enabled (vllm-project#2872)

Signed-off-by: Didan Deng <33117903+wtomin@users.noreply.github.com>
lengrongfu pushed a commit to lengrongfu/vllm-omni that referenced this pull request May 1, 2026
…tch parallel is enabled (vllm-project#2872)

Signed-off-by: Didan Deng <33117903+wtomin@users.noreply.github.com>
clodaghwalsh17 pushed a commit to clodaghwalsh17/nm-vllm-omni-ent that referenced this pull request May 12, 2026
…tch parallel is enabled (vllm-project#2872)

Signed-off-by: Didan Deng <33117903+wtomin@users.noreply.github.com>
daixinning pushed a commit to daixinning/vllm-omni that referenced this pull request May 28, 2026
…tch parallel is enabled (vllm-project#2872)

Signed-off-by: Didan Deng <33117903+wtomin@users.noreply.github.com>
quyifei23 pushed a commit to quyifei23/vllm-omni that referenced this pull request Jun 6, 2026
…tch parallel is enabled (vllm-project#2872)

Signed-off-by: Didan Deng <33117903+wtomin@users.noreply.github.com>
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

5 participants