Repository navigation
Conversation
|
This PR was classified as CI work. CI owner: @yenuo26 @congw729 @NickCao Routing: @yenuo26 via semantic router, CI owner, CODEOWNERS; @congw729 via CODEOWNERS; @NickCao via CODEOWNERS @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. |
b7fd120 to
136b0a2
Compare
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
This PR moves the slow Pi0.5 CPU unit module out of the four-way AMD generic diffusion shard and onto its own blocking MI300 job in both the ready and merge pipelines. The shared shard now --ignores tests/diffusion/models/pi05/test_pi05_units.py, and the new Simple · Pi0.5 CPU Test step runs that same file with the existing core_model and cpu filter, a 45-minute timeout, and no pytest-shard split so the module-scoped fixture stays in one lane. Pipeline-contract tests now pin the exclusion, dedicated label, blocking grade, timeout, and unsharded command on both YAML files. The user-visible effect is that AMD CPU diffusion shards should stop burning their 45-minute budget on Pi0.5 while that coverage remains a blocking merge/ready gate.
Change flow
flowchart TD
A["[EXISTING] AMD Simple Diffusion 4-way shard"]:::existing --> B["[CHANGED] Shard pytest ignores test_pi05_units.py"]:::changed
B --> C["[NEW] Simple Pi0.5 CPU Test blocking MI300 lane"]:::new
C --> D["[EXISTING] AMD template renders 45m Blocking job"]:::existing
B --> E["[CHANGED] test_amd_pipeline isolation contract"]:::changed
C --> E
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
136b0a2a935e(2026-09-27T20:26:57.071053+00:00): required check(s) blocking:buildkite/vllm-omni(failed), and -5 more. Observed Buildkite:buildkite/vllm-omni-amd-ci(failed),buildkite/vllm-omni(failed),buildkite/vllm-omni-npu-ci(passed).
No actionable findings.
Signed-off-by: andyluo7 <andy.luo@amd.com>
136b0a2 to
54d6e36
Compare
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
This PR fixes AMD READY/MERGE timeouts by pulling the slow Pi0.5 CPU unit module out of the four-way diffusion shard and into its own blocking MI300 lane. Both test-amd-ready.yml and test-amd-merge.yml ignore tests/diffusion/models/pi05/test_pi05_units.py in the shared shard and run that path alone with the same core_model and cpu marker and a 45-minute budget. Pipeline-contract tests now cover ready and merge for the ignore flag, dedicated blocking step, and non-sharded command.
Change flow
flowchart LR
A["[EXISTING] AMD Simple Diffusion CPU shards"]:::existing
B["[CHANGED] Shard pytest ignore pi05 units"]:::changed
C["[NEW] Simple · Pi0.5 CPU Test lane"]:::new
D["[CHANGED] test_amd_pipeline contract"]:::changed
E["[EXISTING] Blocking READY/MERGE MI300 coverage"]:::existing
A --> B
B --> C
C --> E
B --> D
C --> D
D --> E
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
54d6e36b9f0c(2026-09-30T22:35:06.858282+00:00): all required and operator-watched checks reported green. Observed Buildkite:buildkite/vllm-omni-npu-ci(failed),buildkite/vllm-omni-amd-ci(failed),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!
|
Self-review at source head Checked that both generic diffusion shards exclude only the Pi0.5 module and both READY/MERGE run the whole module in a dedicated blocking 45-minute lane. Retained exact-source AMD #13043 READY/MERGE each pass all 103 cases in 1858.23s/2030.17s. Seven focused pipeline tests and applicable hooks passed. c30 #13061/#13064 separately establish two complete AMD interaction observations with blocking phases below 40 minutes. Source aggregate AuK/Breeze failures require their companion fixes. Human approval remains. |
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:
54d6e36b9f0c70943bf05c7ad96da082825fc058. 3 shared changed file(s) also contain other repairs in c30. The standalone source checks are classified in the retained validation history.Retained validation history
Fresh current-head AMD result
AMD #13043 READY Pi0.5 CPU on current head
54d6e36b9f0c70943bf05c7ad96da082825fc058: exit 0, non-soft, 103 passed in 1858.23s (30m58s), below the 45m lane budget. The same-build MERGE copy also exited 0, non-soft: 103 passed in 2030.17s (33m50s). Complete combined suite remains pending. The build also has separate unpatched AuK and Breeze failures; they do not invalidate this named Pi0.5 result. Two same-head observations and human approval remain required.Summary
Why
The generic diffusion shard repeatedly exhausts its 45-minute budget while it is still executing Pi0.5 CPU tests. In AMD build 12855, shard 4 reached
test_bfloat16_runs_and_tracks_float32and was terminated at the job timeout; the other Pi0.5 tests had consumed more than 20 minutes after the preceding suite work. Isolating the module preserves blocking coverage while removing it from the shared shard critical path.Failure evidence: https://buildkite.com/vllm/vllm-omni-amd-ci/builds/12855#01a0e0f7-6585-4596-99f6-b36f9d829dec
Validation
python3 -m pytest -q -o addopts='' tests/buildkite/test_amd_pipeline.py(7 passed)pre-commit run --files .buildkite/amd/test-amd-merge.yml .buildkite/amd/test-amd-ready.yml tests/buildkite/test_amd_pipeline.pyNPU check classification - September 30
Exact source head
54d6e36b9f0c70943bf05c7ad96da082825fc058: NPU #8136 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.