Repository navigation
Conversation
|
This PR appears to belong to: docs/design/module/model_integration.md, docs/design/module/ar_runtime.md. Module owners: @tzhouam @fake0fan @Gaohan123 Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Gaohan123 via module of the changed files @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. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
Omni ReviewBot: supersededThe CI failure noted on |
Omni ReviewBot routing recordAssigned Direct under experiment |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
The Breeze depth decoder used to compile BreezeDepthLayer.forward with Inductor max_autotune always on. That multi-candidate Triton GEMM path is what AMD R2-01 hits before the GCNRegPressure.cpp abort. This change keeps epilogue_fusion off, turns max_autotune off only when current_platform.is_rocm() is true, and leaves the CUDA/other compile recipe unchanged. Users on ROCm should get a slower but stable depth-graph compile instead of a process abort; other platforms keep the previous autotune.
Change flow
flowchart TD
caller["[EXISTING] BreezeDepthDecoder.generate_frames first-graph compile"]:::existing
plat["[EXISTING] current_platform.is_rocm"]:::existing
helper["[NEW] _depth_layer_compile_options"]:::new
compile["[CHANGED] torch.compile BreezeDepthLayer.forward options"]:::changed
test["[NEW] test_depth_layer_compile_options CPU policy pin"]:::new
result["[CHANGED] ROCm max_autotune off; non-ROCm stays on"]:::changed
caller --> helper
plat --> helper
helper --> compile
helper --> test
compile --> result
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
CI at
fbc8604112db(2026-09-27T20:26:57.519884+00:00): required check(s) blocking:pre-commit(failed),buildkite/vllm-omni(failed), and -4 more. Observed Buildkite:buildkite/vllm-omni-amd-ci(failed),buildkite/vllm-omni(failed),buildkite/vllm-omni-npu-ci(passed).
See inline comments below.
Omni ReviewBot: finding feedback[p1] Declared validation never re-runs the R2-01 depth compile abort — The new test only asserts The PR Validation section reports The CPU helper guard that does match the new test is CUDA/AMD This finding appears in the bot's COMMENT review, but GitHub could not place it as an inline diff comment. If you are the PR author and disagree, react 👎 to this comment. The disagreement will be shown to the maintainer; it does not approve or merge the PR. |
fbc8604 to
cda14ac
Compare
1cd4887 to
f6999e0
Compare
Signed-off-by: andyluo7 <andy.luo@amd.com>
Signed-off-by: andyluo7 <andy.luo@amd.com>
Signed-off-by: andyluo7 <andy.luo@amd.com>
f6999e0 to
1500fd2
Compare
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
On ROCm, Breeze depth-layer torch.compile no longer enables Inductor max_autotune, which had aborted AMD R2-01 during Triton multi-candidate GEMM tuning (GCNRegPressure). Non-ROCm keeps max_autotune=True. A CPU policy test pins both branches, and the included graph/RNG test updates match the existing NVIDIA-only register_generator_state capture gate so ROCm’s intentional eager RNG path does not fail assertions.
Change flow
flowchart TD
A["[EXISTING] BreezeDepthDecoder.generate_frames"]:::existing
B["[NEW] _depth_layer_compile_options()"]:::new
C["[CHANGED] torch.compile options:<br/>max_autotune off on ROCm"]:::changed
D["[NEW] test_depth_layer_compile_options"]:::new
E["[CHANGED] ROCm graph/RNG test expectations"]:::changed
F["[EXISTING] Stable compile + eager RNG on ROCm"]:::existing
A --> B --> C --> F
B --> D
C --> E --> F
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
CI at
1500fd2a319a(2026-09-30T22:09:11.934703+00:00): all required and operator-watched checks reported green. Observed Buildkite:buildkite/vllm-omni-amd-ci(failed),buildkite/vllm-omni(passed),buildkite/vllm-omni-intel-ci(passed), and 1 more.
No actionable findings.
🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!
|
No code blockers found at |
Current combined AMD evidence - September 30, 2026, 22:53 UTC
The integration at
c30e63d04dc9a54c5c4b43235eb2ce535fe68df3now has two complete AMD passes at the identical commit. Each build has 65/65 jobs finished, every exit 0 and non-soft, with no retries. Both include all five NIGHTLY leaves.Phase time runs from the earliest phase job start to the latest finish; queue and image-build time are excluded. Both READY and MERGE are below 40 minutes in both observations.
Repeat Accuracy evaluates all 1,088 Seed-TTS samples, mean WER 0.3088705089, with zero request, PCM or ASR failures. Daily-Omni evaluates all 1,197 samples, accuracy 0.7218045113, with zero HTTP or parse failures. Both result JSON files are downloaded and SHA-256 verified against Buildkite metadata. Full CosyVoice passes all 3 cases in 2,317.40s, including both reference voice-clone cases.
These results exercise the combined c30 tree. They establish combined implementation and interaction coverage; they do not establish a second standalone run at a different source PR SHA. Earlier 024 results remain separate evidence.
CUDA #16446 at c30 still has a Documentation Whisper-small transcript failure against the unchanged 0.8 gate; WAV/JSON retention is qualified, and the exact-model replay remains pending. Intel #9379 failed during a dependency download before runtime testing. Source PR checks and maintainer review remain separate merge gates.
Source head:
1500fd2a319a09e942238ea16cfa127ffd172542. 3 changed file(s) are identical between this source head and c30. The standalone source checks are classified in the retained validation history.Retained validation history
Summary
max_autotuneenabled on non-ROCm platformsWhy
AMD R2-01 repeatedly aborts while compiling the Breeze depth graph with:
The crash occurs after Inductor benchmarks multiple Triton GEMM candidates. Disabling
max_autotuneonly on ROCm avoids that unstable compiler path while preserving the existing optimization elsewhere.Buildkite evidence: https://buildkite.com/vllm/vllm-omni-amd-ci/builds/12855#01a0e0f7-658e-429d-a216-c7b09e24fb41
Validation
python3 -m py_compile vllm_omni/model_executor/models/breeze_tts_2/depth_decoder.py tests/model_executor/models/test_breeze_tts_2_graphs.pypre-commit run --files vllm_omni/model_executor/models/breeze_tts_2/depth_decoder.py tests/model_executor/models/test_breeze_tts_2_graphs.pyf6999e052b40816a23267baa3abf3e56d939f718: 604 passed, 272 skipped, 4547 deselected, 671.34 seconds; Breeze graph and first-code cases passed. This is READY GPU compiler coverage, not a merge-only CPU policy result.4af28f33b.1500fd2a319a09e942238ea16cfa127ffd172542has fresh passing READY/R2-01 compiler and CPU policy results detailed below. Bothreadyandmerge-testwere selected. Aggregate AMD red from other targets does not establish a Breeze regression.Fresh READY compiler result - September 30
Current head
1500fd2a319a09e942238ea16cfa127ffd172542: AMD #13041 R2-01 exited 0 with 650 passed, 316 skipped, 4828 deselected in 772.08s. Breeze graph/compiler cases ran and passed; artifacts were uploaded. This directly contrasts with theGCNRegPressurecrash on #13043 without the combined repair and the 5 Breeze RNG assertions on #13044 without the test-contract patch. CPU policy target also passed in #13041 Simple model-executor shard 3: exit 0, non-soft, 1126 passed, 5 skipped, 340 deselected in 1223.52s. Bothtest_depth_layer_compile_optionsvariants explicitly ran and passed. This closes the requested GPU compiler and CPU policy validation gap; two same-head observations and human approval remain separate promotion gates.NPU check classification - September 30
Exact source head
1500fd2a319a09e942238ea16cfa127ffd172542: NPU #8134 has 20 jobs finished with exit 0, non-soft. The sole failing A5 layer job never started:started_at, agent assignment and agent finish are null. Buildkite reports that its Kubernetes pod remained Pending for three hours, withPodScheduled=False, insufficient NPU/CPU capacity, unreachable-node taints and unschedulable nodes. This explains the aggregate red check as a runner scheduling failure. A5 runtime remains unqualified; no passing test is inferred for that job.