Skip to content

[CI][ROCm] Fix deterministic AMD L3 blockers - #6884

Merged
hsliuustc0106 merged 2 commits into
vllm-project:mainfrom
andyluo7:codex/rocm-ci-execution
Sep 1, 2026
Merged

hsliuustc0106 merged 2 commits into
vllm-project:mainfrom
andyluo7:codex/rocm-ci-execution

Conversation

@andyluo7

Copy link
Copy Markdown
Collaborator

Summary

  • preserve the Qwen3-TTS Base advanced-model marker and run level by removing the nested shell quoting that truncated the pytest command
  • give the Qwen3-Omni colocate sleep/abort regression a ROCm-only 2 GiB KV-cache budget instead of percentage-based over-allocation
  • keep the CUDA test configuration unchanged and add focused CI/config regressions

Why

Recent AMD L3 main runs repeatedly fail these two blocking jobs. The TTS job falls back to core-model dummy weights because its nested single quotes terminate the outer bash command before --run-level reaches pytest. The colocate test loads about 64.5 GiB of weights, then percentage sizing attempts about 125 GiB of KV cache for an 8K-token control-plane test and fails during allocation.

Validation

  • Buildkite YAML regression: 1 passed locally
  • rendered Bash argument-flow check confirms marker advanced_model and cuda and run level advanced_model
  • ROCm overlay materializes kv_cache_memory_bytes=2147483648 and removes percentage sizing only on ROCm
  • Python compileall and Ruff 0.14.10 pass

Full project pytest is unavailable on the local macOS checkout because its Torch/vLLM environment is not installed. This draft therefore requires a real AMD L3 merge-test run before either blocker is considered resolved.

CI request

Please trigger the AMD merge-test/L3 suite on this exact head. The acceptance gate is that both Qwen3-TTS Base E2E Test and Qwen3-Omni colocate async complete successfully with real weights on MI300.

Signed-off-by: andyluo7 <andy.luo@amd.com>
@andyluo7
andyluo7 force-pushed the codex/rocm-ci-execution branch from 278b7e7 to 8ad4cd7 Compare August 31, 2026 18:41
@andyluo7

Copy link
Copy Markdown
Collaborator Author

Self-review completed:

  • Traced the AMD Jinja template: the YAML command list is joined into one TEST_COMMANDS shell, so separate export lines persist while avoiding the nested single-quote truncation. A Bash argument-flow check confirms pytest receives both the full advanced_model and cuda marker and --run-level advanced_model.
  • Traced kv_cache_memory_bytes from the deploy overlay through stage engine_extras into the vLLM cache config. The 2 GiB cap is under platforms.rocm only; the focused config regression asserts CUDA retains gpu_memory_utilization=0.9 and receives no explicit KV cap.
  • Ran the focused Buildkite YAML test, Python compileall, Ruff 0.14.10, YAML materialization, and shell argument-flow validation locally.

The remaining acceptance test must be the real AMD L3 suite on this exact head. @yenuo26 @hsliuustc0106, could one of you please add the merge-test and amd-test labels? I attempted it, but the fork author account does not have label permission.

@andyluo7
andyluo7 marked this pull request as ready for review August 31, 2026 18:44
@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.

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR was classified as CI work.

CI owner: @yenuo26

@andyluo7, 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.

@andyluo7

Copy link
Copy Markdown
Collaborator Author

Author self-review (after CI-owner assignment): I verified the full diff against current main, confirmed the AMD Jinja renderer joins the separate export and pytest lines into one shell, reproduced the final pytest argv, and checked the ROCm-only cache override through the deploy projection with an explicit CUDA non-regression assertion. Static checks are green. The change still requires the requested real AMD L3 run before merge.

@vllm-omni-review-bot

vllm-omni-review-bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Omni ReviewBot triage note

Automated triage of commit 0cbd91734887 produced:

  • Priority: high. Prompt maintainer attention is suggested.

These are automated triage suggestions only — the final decision belongs to the maintainers.

@andyluo7

Copy link
Copy Markdown
Collaborator Author

@tjtanaa, this follows the current AMD L3 layout from #6830 and targets the two deterministic failures seen after that merge. I cannot request you as a reviewer or apply labels from the fork account. Could you help add merge-test and amd-test so the exact head receives the full MI300 L3 run?

@hsliuustc0106 hsliuustc0106 added CI/CD codes related to changes to CI/CD ROCm PR related to AMD hardware labels Sep 1, 2026
@hsliuustc0106

Copy link
Copy Markdown
Collaborator

@hsliuustc0106 hsliuustc0106 added the high priority high priority issue, needs to be done asap label Sep 1, 2026
@andyluo7

andyluo7 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks. I checked build #11263: it ran on PR #6829 at d122a9b, not this PR head. Its only blocking job was Simple · Diffusion Test, where test_cross_attn_key_padding_vs_sdpa[40] and [256] hit BF16 max diff 0.015625 against the old strict <0.01 limit.

This PR head already contains #6830’s ROCm-specific comparison, and all four Simple Diffusion shards passed on MI300 in AMD build #11230. That build then failed on the Qwen3-Omni and Qwen3-TTS Base L3 jobs addressed by this PR.

Could you trigger a fresh L2 run and the AMD L3 merge-test on head 8ad4cd7a? The branch is currently three commits behind main; I can rebase first if you prefer.

@congw729 congw729 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.

The code changes look reasonable to me, and I did not find a blocking issue in the diff. The focused regressions cover both the Qwen3-TTS argument preservation and the ROCm-only KV-cache override while keeping CUDA unchanged.

Before approval, I would still like to see the AMD L3 merge test run on the current head 8ad4cd7, especially the Qwen3-TTS Base and Qwen3-Omni colocate-async jobs, since those are the behaviors this PR is intended to fix.

@hsliuustc0106 hsliuustc0106 added ready label to trigger buildkite CI amd-test Used to trigger AMD CI separately. labels Sep 1, 2026
@andyluo7

andyluo7 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @congw729. The branch is now at 0cbd9173 after merging the latest main. AMD L2 on this exact head passed in build #11281.

I checked the bootstrap log: that run rendered test-amd-ready.yml, so it did not run Qwen3-TTS Base E2E Test or Omni Model Test Qwen3-Omni.

@hsliuustc0106 @yenuo26, could you share the Buildkite build number if the AMD L3 run is already registered, or trigger vllm-omni-amd-ci on 0cbd9173 with DEBUG_TEST_YAML=merge? I will verify those two target jobs and report the results here.

@hsliuustc0106
hsliuustc0106 merged commit 477083a into vllm-project:main Sep 1, 2026
6 checks passed
JoseCarlosGarcia95 pushed a commit to valendra-tech/vllm-omni that referenced this pull request Sep 5, 2026
Signed-off-by: andyluo7 <andy.luo@amd.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
ZhengWG pushed a commit to ZhengWG/vllm-omni that referenced this pull request Sep 8, 2026
Signed-off-by: andyluo7 <andy.luo@amd.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Sep 25, 2026
Signed-off-by: andyluo7 <andy.luo@amd.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

amd-test Used to trigger AMD CI separately. CI/CD codes related to changes to CI/CD high priority high priority issue, needs to be done asap ready label to trigger buildkite CI ROCm PR related to AMD hardware

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants