Skip to content

[CI/Build][ROCm] Move full Omni processor matrix to nightly - #8571

Merged
andyluo7 merged 1 commit into
vllm-project:mainfrom
haic0:ci/amd-omni-processor-timing-20261006
Oct 9, 2026
Merged

andyluo7 merged 1 commit into
vllm-project:mainfrom
haic0:ci/amd-omni-processor-timing-20261006

Conversation

@haic0

@haic0 haic0 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Reduce AMD READY/MERGE latency by replacing the full Omni processor correctness matrix with a deterministic ROCm integration smoke. The complete matrix remains in AMD nightly, and CUDA READY/MERGE retain their existing mandatory full coverage.

This shape also accounts for #8646: AMD now uses a ROCm-only local metadata snapshot path. READY/MERGE therefore keep real AMD coverage of that path instead of removing Omni processor coverage entirely.

Evidence and expected impact

After #8646, AMD build #13522 ran Simple · Omni Processor Test from 03:34:04.490Z to 04:11:46.015Z, about 37m42s, with exit status 0.

The prior exact PR head also passed the full nightly lane in AMD build #13444 from 20:18:39.925Z to 20:47:59.876Z, about 29m20s. The build aggregate failed in unrelated jobs.

The blocking smoke is capped at eight minutes and is designed for a sub-five-minute runtime. If it meets that target, READY/MERGE should save roughly 30 minutes of MI300 occupancy per run while preserving the AMD-specific integration contract.

Changes

  • Keep a blocking Simple · Omni Processor Snapshot Smoke in AMD READY and MERGE.
    • Select one representative architecture for each backing repository: Qwen2.5-Omni and Qwen3-Omni.
    • Use the real ROCm metadata snapshot and tokenizer path.
    • Verify a resolver cache miss followed by reuse and a cache hit.
    • Cap pytest at eight minutes with a 10-minute Buildkite job timeout.
  • Move the complete four-architecture × three-hit-rate × 32-batch correctness matrix to one nonblocking AMD nightly lane.
  • Add exception-safe, opt-in phase timing with flushed JSON start/end records and aggregate summaries.
  • Upload nightly JUnit, pytest output, phase timing, and CPU/cgroup context artifacts while preserving pytest failures.
  • Keep AMD model-executor shards excluding omni, preventing duplicate full coverage.
  • Keep CUDA READY/MERGE unchanged; their Model Executor lane still selects all core_model and cpu tests, including the full Omni matrix.
  • Add Buildkite contract tests for all routing, timeout, artifact, runner, and CUDA-ownership guarantees.

Risks and mitigations

  • The AMD per-PR check is intentionally narrower than the full cache-distribution sweep. Mandatory CUDA READY/MERGE and the complete AMD nightly matrix retain broad correctness coverage.
  • The full AMD lane is nonblocking, so nightly failures require follow-up rather than blocking a PR automatically. Timing and failure artifacts make those failures inspectable.
  • The smoke still depends on Hugging Face metadata availability. It downloads only JSON/TXT processor metadata and uses the same service dependency as the previous blocking lane.
  • The sub-five-minute target needs exact-head AMD measurement. The command fails closed at eight minutes instead of silently expanding the critical path.

Validation

Current head: 941f6f07622bd0c1d885d5ba7d2828e52c975ed5, rebased onto current main at de3813bdc778bb4938b11c0ab9b3659ee4764283.

Passed locally:

  • PYTHONPATH=$PWD pytest -o addopts= --confcutdir=tests/buildkite -q tests/buildkite/test_amd_pipeline.py: 18 passed.
  • Changed-file pre-commit: all hooks passed, including Ruff, mypy 3.10, ShellCheck, test marks, SPDX, YAML, Buildkite pipeline validation, forbidden imports, and torch.cuda checks.
  • python3 -m py_compile for both changed Python test files.
  • git diff --check.
  • DCO on the pushed exact head.

Exact-head CI on 941f6f07622bd0c1d885d5ba7d2828e52c975ed5:

  • AMD build #13529: 35/35 steps passed with no retries, soft failures, cancellations, or ignored steps.
    • Blocking snapshot smoke: both repository cases passed; 38.96s pytest time and 1m26s total job time.
    • Full nightly matrix and focused regressions: 24 passed in 46m07s pytest time and 46m57s total job time.
    • Uploaded JUnit, pytest log, and CPU/cgroup context artifacts.
  • CUDA build #16939: passed.
  • GitHub pre-commit, Python 3.11 and 3.12 wheels, DCO, and documentation checks: passed.

The processor runtime tests are not runnable on the local Mac because its Python environment lacks PyTorch; the exact-head AMD build above provides the required ROCm runtime evidence.

Merge gates

  • Exact-head AMD READY smoke passes for both repositories within the target.
  • Exact-head CUDA, pre-commit, build, DCO, and documentation checks are terminal and acceptable.
  • The PR has the required human approval and no unresolved review threads.

AI assistance and accountability

AI assistance was used to integrate the PR with #8643/#8646, implement the focused smoke and pipeline contracts, run local validation, and update this description. The human submitter remains responsible for reviewing and defending the change and for inspecting the exact-head hardware evidence.

@haic0

haic0 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Author self-review

Reviewed the complete six-file diff at exact head ceda26af4267f96363fe9d06c9bf61a5ff597d98 on base d5a3380103df2fc827f095c60dbfb6e1c5655fd3.

  • Verified every manual timer boundary is now an exception-safe with timing.phase(...); start/end JSON is flushed immediately, end records carry success/error plus elapsed time, error output excludes exception values, and the original exception is re-raised. Aggregate summaries remain.
  • Verified disabled instrumentation does not read the clock or emit output. Processing inputs, RNG, cache construction/lifetime, call order, assertions, four architectures × three hit rates, and 32 batches remain unchanged.
  • Verified AMD READY/MERGE contain no dedicated Omni lane and retain core_model and cpu and not omni; AMD NIGHTLY contains exactly one full lane, renders soft_fail: true, keeps the inherited 60-minute contract, and preserves one fail-closed pytest invocation, --durations=0, JUnit, CPU diagnostics, streamed output, and artifacts.
  • Verified CUDA pipeline source diff is empty. At evidence commit 9482bf1e8c068e72cc778616edff5b6a0e7c0245, CUDA READY/MERGE select the full core_model and cpu Model Executor suite without an Omni exclusion or source gate; source defines exactly 12 × 32 cases. No AMD-specific per-PR behavior justified a smoke lane.
  • Native MiniJinja READY/MERGE/NIGHTLY lane counts are 0 / 0 / 1; after removing the moved lane/group, old and new rendered pipelines are semantically identical.
  • Focused AMD contracts passed (56 passed); broader Buildkite contracts reached 155 passed, with three unrelated package-discovery failures caused by absent local vllm. Changed-file pre-commit, mypy 3.12, Buildkite schema, ShellCheck, diff/Unicode/DCO, and full precheck passed.
  • Focused timer tests and Omni collection remain unexecuted locally because .venv lacks vllm; no fabricated 12-item runtime result is claimed.
  • Duplicate check found no equivalent open PR. [CI/Build][ROCm] Deduplicate combined AMD test suites #8417 remains orthogonal; merged [CI/Build][ROCm] Move Pi0.5 CPU coverage to AMD nightly #8559 is the nightly-routing precedent.

The PR remains draft. No AMD CI was triggered. A human must review every changed line and obtain exact-head nightly evidence for 12-case collection/runtime, failure artifacts, streamed forced-termination records, and process cleanup before marking ready.

andyluo7 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

I reviewed this at exact head 6c117bf6c92a9a37a2e510a758fc77a37cbb9703. The instrumentation is useful groundwork, but I do not think the PR is ready yet, and it does not currently reduce test time.

Two instrumentation issues should be fixed first:

  1. _PhaseTimer accumulates data in memory and emits it only from the test-level finally. A Buildkite timeout/SIGTERM can terminate pytest without executing that block, so the timeout case can produce no phase records—the main failure mode we need to diagnose. Please stream and flush progress at phase boundaries (ideally start/end records), while retaining the final aggregate summary if useful.
  2. Each phase uses manual start() / stop(). If processor construction, processing, or an assertion raises, stop() is skipped and the failing phase is absent from the report. Please use an exception-safe context manager and add a test that raises inside a timed phase.

For the actual runtime reduction, the current evidence points to removing duplicated AMD CPU coverage rather than only instrumenting it:

  • AMD build #13409, commit 9482bf1e8c068e72cc778616edff5b6a0e7c0245: the 12 Omni processor cases took 33m07s; recent AMD jobs range from about 28 to 52 minutes.
  • CUDA build #16809, the same commit and same 12 cases: they ran from 02:36:31.710Z through 02:38:29.859Z, about 1m58s, inside the mandatory Model Executor CPU job.
  • The test is marked core_model and cpu and omni. CUDA READY and MERGE already run pytest -sv tests/model_executor -m 'core_model and cpu'. AMD excludes Omni from its sharded CPU suite and then runs the full test separately on an MI300 worker.

Recommended plan for a significant reduction:

  1. Keep the full 12 × 32 correctness matrix in mandatory CUDA READY/MERGE.
  2. Move the full AMD Omni processor lane, with the repaired instrumentation, to AMD nightly. Removing it from AMD READY/MERGE saves roughly 28–52 minutes and one MI300 pod per run without losing mandatory CPU correctness coverage.
  3. If AMD-specific import/environment coverage is still required on every PR, replace the full lane with a small deterministic smoke test: one case per architecture, explicitly covering a cache miss, mixed reuse, and a cache hit. Avoid the current probabilistic 32-batch sweep in the blocking path.
  4. Keep the 32-batch × three-hit-rate stress sweep in nightly. That preserves broader cache-distribution coverage while making READY/MERGE fast.
  5. Qualify the new AMD smoke against the full test once, then require a target such as <5 minutes for the blocking lane.

This should be substantially more effective than adding timers or xdist to the current 384-batch matrix.

@haic0
haic0 force-pushed the ci/amd-omni-processor-timing-20261006 branch from 6c117bf to ceda26a Compare October 7, 2026 05:15
@haic0 haic0 changed the title [CI/Build][ROCm] Instrument Omni processor runtime [CI/Build][ROCm] Move Omni processor coverage to nightly Oct 7, 2026
@haic0

haic0 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

@andyluo7 Addressed at exact head ceda26af4267f96363fe9d06c9bf61a5ff597d98 (true-rebased onto d5a3380103df2fc827f095c60dbfb6e1c5655fd3):

  • replaced every manual timer pair with with timing.phase(...);
  • stream/flush start and end JSON immediately; end records include elapsed time plus success/error, omit exception values, and preserve/re-raise the original exception; added focused success/flush, error propagation, aggregate, and disabled-gate tests;
  • removed the full lane from AMD READY/MERGE, retained and not omni in both shards, and added exactly one NonBlocking 60-minute AMD nightly lane with the full matrix, JUnit, durations, CPU diagnostics, and failure-preserving artifacts;
  • kept CUDA and the 12 × 32 matrix unchanged; no AMD smoke was added because no AMD-specific per-PR behavior was found.

Local source/render evidence: CUDA diff is empty; native MiniJinja lane counts are READY/MERGE/NIGHTLY 0 / 0 / 1, with nightly soft_fail: true; 56 focused AMD contracts passed. The local environment lacks vllm, so timer pytest and 12-item collection remain exact-head nightly qualification items. The PR stays draft and no AMD CI was triggered.

@andyluo7
andyluo7 force-pushed the ci/amd-omni-processor-timing-20261006 branch from ceda26a to c1df77a Compare October 7, 2026 15:22
@andyluo7 andyluo7 added amd-test Used to trigger AMD CI separately. nightly-test label to trigger buildkite nightly test CI cuda-test Used to trigger vllm-omni cuda CI separately. ready label to trigger buildkite CI and removed nightly-test label to trigger buildkite nightly test CI cuda-test Used to trigger vllm-omni cuda CI separately. ready label to trigger buildkite CI labels Oct 7, 2026
@hsliuustc0106 hsliuustc0106 added CI/CD codes related to changes to CI/CD ROCm PR related to AMD hardware labels Oct 8, 2026
Keep a blocking two-repository ROCm snapshot smoke in READY/MERGE, retain the complete correctness matrix with timing artifacts in nightly, and preserve full CUDA coverage.

Co-authored-by: haic0 <149741444+haic0@users.noreply.github.com>
Signed-off-by: andyluo7 <andy.luo@amd.com>
@andyluo7
andyluo7 force-pushed the ci/amd-omni-processor-timing-20261006 branch from c1df77a to 941f6f0 Compare October 9, 2026 04:48
@andyluo7 andyluo7 changed the title [CI/Build][ROCm] Move Omni processor coverage to nightly [CI/Build][ROCm] Move full Omni processor matrix to nightly Oct 9, 2026
@andyluo7 andyluo7 added ready label to trigger buildkite CI and removed ready label to trigger buildkite CI labels Oct 9, 2026
@andyluo7 andyluo7 added amd-test Used to trigger AMD CI separately. cuda-test Used to trigger vllm-omni cuda CI separately. and removed amd-test Used to trigger AMD CI separately. cuda-test Used to trigger vllm-omni cuda CI separately. labels Oct 9, 2026
@andyluo7
andyluo7 marked this pull request as ready for review October 9, 2026 06:06
@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.

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

Approved. Exact-head AMD and CUDA CI passed, including the two-repository ROCm snapshot smoke and the full AMD nightly matrix. Both wheel builds, pre-commit, DCO, and docs are green, and there are no unresolved review threads.

@andyluo7
andyluo7 merged commit 61cae20 into vllm-project:main Oct 9, 2026
7 checks passed
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Oct 14, 2026
…ject#8571)

Signed-off-by: andyluo7 <andy.luo@amd.com>
Co-authored-by: andyluo7 <andy.luo@amd.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 cuda-test Used to trigger vllm-omni cuda CI separately. nightly-test label to trigger buildkite nightly test CI 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.

3 participants