Repository navigation
[CI/Build] Rebalance CUDA ready jobs and move slow cases to weekly - #8888
Conversation
…_marks helper - Modified test files to replace direct CUDA and NPU markers with the hardware_marks helper for consistency and clarity. - Updated the pre-commit configuration to reflect changes in test file requirements. - Enhanced documentation in the CI test writing guide regarding the use of hardware markers. Signed-off-by: wangyu <410167048@qq.com>
Accept the deletion of MOSS-TTS tests removed in vllm-project#8640. Route platform marks that landed on main through hardware_test / hardware_marks so the stricter check-mark hook stays clean. Co-authored-by: Cursor <cursoragent@cursor.com>
- Introduced new disallowed markers for direct SKU and bare platform. - Added detailed problem formatting and failure summary for missing or disallowed marks. - Improved error messaging for test files failing the mark check. This update aims to provide clearer feedback on test file compliance with CI requirements. Signed-off-by: wangyu <410167048@qq.com>
- Added support for RTX 5090 GPU in `pyproject.toml` and updated relevant test files to use the new hardware marker. - Enhanced validation in `check_test_marks.py` to ignore mentions of platform and SKU in comments and strings. This update ensures compatibility with the latest hardware while maintaining compliance with testing standards. Signed-off-by: wangyu <410167048@qq.com>
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # tests/model_executor/models/minicpmo_4_5/test_talker_mrv2.py # tests/model_executor/models/personaplex/test_ring_per_row_offsets.py
- Replaced `hardware_marks` with `hardware_test` in multiple test files to standardize GPU marker usage. - Updated tests for `trtllm_attn`, `minimax_h3`, and `seedvr2` to ensure compatibility with specified hardware configurations. This change enhances clarity and consistency in test annotations across the codebase. Signed-off-by: wangyu <410167048@qq.com>
Added a skip marker to the bf16 test case in the omni_wan_vae_decode test to address a known issue. This change ensures that the test suite accurately reflects the current state of the codebase and avoids false negatives. Signed-off-by: [Your Name] <[Your Email]> Signed-off-by: wangyu <410167048@qq.com>
…operations Added a comment regarding H3 VAE operations' dependency on NVIDIA CUDA and ROCm. Updated the pytest command to ignore the minimax_h3/test_minimax_h3_vae_ops.py file, ensuring the test suite runs more efficiently by excluding unnecessary tests. Signed-off-by: [Your Name] <[Your Email]> Signed-off-by: wangyu <410167048@qq.com>
- Removed deprecated joy_image references from source file dependencies. - Added new worker and engine model executor functions to source file dependencies. - Updated test commands in CI configurations for clarity and consistency. - Adjusted pytest markers for accuracy tests to better reflect their execution context. - Deleted obsolete test file for Pi0.5 CPU routing. Signed-off-by: wangyu <410167048@qq.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR matches CODEOWNERS paths: /.buildkite/, /tests/. Code owners: @congw729 @NickCao Routing: @congw729 via CODEOWNERS; @NickCao via CODEOWNERS @yenuo26, 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. |
- Renamed worker function to engine_model_executor_worker_function in source file dependencies. - Consolidated engine and model executor tests into a single test group. - Reduced timeout for several test steps from 30 to 20 minutes for efficiency. - Updated test commands to include new source file dependencies for engine and model executor tests. Signed-off-by: wangyu <410167048@qq.com>
|
@amy-why-3459 PTAL |
amy-why-3459
left a comment
There was a problem hiding this comment.
Thanks for the rebalance. I checked where each removed ready/merge step lands now. The CosyVoice3 packed-flow and MRV2 FA3 graph-bounds tests are still selected by the new Engine&Model Executor&Work Test · H100 step (they are core_model + hardware_test(cuda: H100)), and pi05 units still run under Simple · Diffusion Models Test. Two gaps and one question inline:
test_full_response_v1_preserves_audio(Higgs v3) no longer runs anywhere.- Deleting
tests/buildkite/test_pi05_ci_routing.pyalso drops the AMD-side guards (pi05 excluded from AMD ready/merge, NonBlocking on mi300 nightly). Only the CUDA test depends on the renamed label, so I'd update that one test toSimple · Diffusion Models Test/tests/diffusion/modelsrather than delete the file. - The Kandinsky6 weekly step keeps
source_file_dependencieswhile SANA-Video's was removed in the same file.
| @pytest.mark.advanced_model | ||
| @pytest.mark.slow | ||
| @pytest.mark.tts | ||
| @hardware_test(res={"cuda": "H100"}, num_cards=1) |
There was a problem hiding this comment.
With core_model/advanced_model replaced by slow, this test drops out of ready. The only weekly step that runs this file is TTS · Higgs · L4, which filters -m "slow and L4 and tts", so an H100-only test is deselected there too. Net effect: the v1 full-response audio regression test stops running. Either add an H100 weekly entry for this node ID or keep it in ready.
There was a problem hiding this comment.
TTS · H100 can cover it, so there is no need to add a new one.
| mirror_hardwares: h100_1 | ||
|
|
||
| - label: "Diffusion · Kandinsky6 · H100 · Single-GPU" | ||
| source_file_dependencies: diffusion_kandinsky6_function |
There was a problem hiding this comment.
SANA-Video's weekly step drops its source_file_dependencies in this PR, but the newly moved Kandinsky6 step adds one. If weekly jobs are meant to run unconditionally, this one will be skipped on weeks with no Kandinsky6 changes, which defeats moving it out of merge. Is that intended?
There was a problem hiding this comment.
deleted Kandinsky6 source_file_dependencies
- Deleted references to Kandinsky6 in source file dependencies. - Updated test-weekly.yml to remove the associated diffusion_kandinsky6_function step. Signed-off-by: [Your Name] <[Your Email]> Signed-off-by: wangyu <410167048@qq.com>
|
@amy-why-3459 PTAL |
Omni ReviewBot routing recordAssigned Strict on cursor (cursor-grok-4.6-high) under experiment |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
3 actionable finding(s).
CI at
4fac16578b8a(2026-10-14T02:59:35.686129+00:00): all required and operator-watched checks reported green. Observed Buildkite:buildkite/vllm-omni-amd-ci(passed),buildkite/vllm-omni(passed),buildkite/vllm-omni-npu-ci(pending), and 1 more.
Note: The assigned review arm
strict/cursor/cursor-grok-4.6-highcould not complete this review, so it was produced by the fallback armstrict/zcode/GLM-5.3-Flash. It is excluded from the routing experiment.
Full review analysis
Scan:
| Category | Result |
|---|---|
| Tests / verification | 3 finding(s) below |
| Security | no finding reported |
| Docs / comments | no finding reported |
| Behavior / compatibility | no finding reported |
| Correctness | no finding reported |
Validated:
- [resolved] .buildkite/cuda/test-merge.yml:136 — folding the inline shm-leak list into omni_qwen3_omni_shm_leak_function does NOT narrow triggers: *qwen3_omni (yml:104-120) covers serving_chat.py:107, models/qwen3_omni/:108, qwen3_code_predictor.py:109, snake_activation.py:113, qwen2_5_omni_thinker.py:114, cuda_graph_decoder_wrapper.py:115, stage_input_processors/qwen3_omni.py:116, qwen3_omni_moe.yaml:117. Residual checked: the 40-min h100_2 job now also fires on async_omni_engine.py:105, omni_base.py:106, fused_sampling.py:110, short_kv_attention.py:111, talker_first_audio.py:112 and the mrv2/mrv2_h200/thinking deploy yamls (:118-120) — a deliberate widening consistent with the PR body's 'reusing *qwen3_omni' and sibling omni_qwen3_omni_function.
- [claim-verified] PR body 'Weekly TTS · H100 already selects slow and H100 and tts' — .buildkite/cuda/test-weekly.yml:284 runs
tests/e2e/ -m "slow and H100 and tts", and test_full_response_v1_preserves_audio (tests/e2e/online_serving/test_higgs_audio_v3.py:320-322) now carries slow+tts+H100; the reviewer's 'no longer runs anywhere' gap is closed at this head. - [claim-refuted] PR body names registry keys
engine_model_executor_functionandworker_function; the diff adds one combinedengine_model_executor_worker_function(ci_source_file_dependencies.yml:226). Cosmetic description drift only. - [validated] No stale consumers of removed keys: repo grep for joy_image/kandinsky6/sana_video_function matches only pytest-path usages (.buildkite/amd/test-amd-nightly.yml:111, .buildkite/amd/test-amd-ready.yml:290) and the two new keys at ci_source_file_dependencies.yml:226,247 consumed by test-merge.yml:139 and test-ready.yml:112; nothing references
diffusion_joy_image_accuracy,diffusion_kandinsky6_function,diffusion_sana_video_function,x-joy-image, orx-kandinsky6. - [validated] tests/helpers/mark.py:295-337 + pyproject.toml:300-321 —
hardware_test/hardware_marksattach SKU+cuda+cards_n marks, so the weekly selectors match the moved tests: online Kandinsky6 gets H100+cards_1 viahardware_marks(res={"cuda": "H100"})param marks (tests/e2e/online_serving/test_kandinsky6.py:53, slow+diffusion at :71-72), offline viahardware_test(:57, slow+diffusion at :55-56), JoyAI via pytestmark slow+diffusion +hardware_test(res={"cuda": "H100"}, num_cards=1). - [validated] Ready lane census of tests/engine, tests/worker, tests/worker_v2: every cuda-marked core_model test carries an L4 or H100 SKU mark (test_async_omni_engine_abort.py:65 L4; tests/worker L4 pytestmarks at :10/:14 + hardware_test :20; worker_v2 H100 :22/:31, L4 :9/:14/:189/:14-shared_stage_cuda) — no coverage loss in these three trees; only the two moss_tts files in tests/model_executor are stragglers (filed as comment).
3 actionable finding(s).
Verdict: REQUEST CHANGES
Findings
- [P1] The same lane split —
-m 'core_model and cuda and L4'(.buildkite/cuda/test-r… —tests/model_executor/models/moss_tts/test_local_attention_heads.py
Evidence for The same lane split — `-m 'core_model and cuda and L4'` (.buildkite/cuda/test-r…
The same lane split — -m 'core_model and cuda and L4' (.buildkite/cuda/test-ready.yml:107) / and H100 (:114) — also deselects tests/model_executor/models/moss_tts/test_local_attention_heads.py: its pytestmark (:9-13) is core_model plus a bare pytest.mark.cuda plus a CUDA-availability skipif, with no SKU mark. The removed single ready lane ran this tree with -m 'core_model and cuda'; the merge gate is CPU-only over tests/model_executor (test-merge.yml:17), no nightly/weekly step collects tests/model_executor (weekly 'TTS · Moss · L4' runs only tests/e2e moss files), so this head-group lookup-attention vs SDPA parity coverage drops from every NVIDIA lane — it now survives only in the pre-existing AMD NonBlocking ROCm burn-in (test-amd-ready.yml:328-355, grade: NonBlocking), which is not equivalent coverage. Sibling test_local_state_kernels.py:9 ([core_model, cuda]) drops out the same way. Add the hardware mark while keeping the runtime skipif: pytestmark = [pytest.mark.core_model, pytest.mark.skipif(not torch.cuda.is_available(), reason="CUDA required"), *hardware_marks(res={"cuda": "L4"}, num_cards=1)] (L4 registered at pyproject.toml:303; helper at tests/helpers/mark.py:295).
Evidence: Trigger: at this PR head the only NVIDIA GPU lanes over tests/model_executor require a SKU mark the test lacks. Current trigger and adverse effect: an item marked exactly {core_model, cuda} matches neither new selector, so the lookup-attention vs SDPA parity test is collected by zero NVIDIA lanes (merge CPU-only; nightly/weekly never reference tests/model_executor), where before this diff it ran in the deleted lane with -m 'core_model and cuda'. Decisive lines (test file unchanged by this diff, present in the PR-time tree): tests/model_executor/models/moss_tts/test_local_attention_heads.py:9-13 pytestmark = [ / pytest.mark.core_model, / pytest.mark.cuda, / pytest.mark.skipif(not torch.cuda.is_available(), reason="CUDA required"), / ]; added by this diff: .buildkite/cuda/test-ready.yml:107 - "pytest -sv tests/engine/ tests/model_executor/ tests/worker_v2/ tests/worker/ -m 'core_model and cuda and L4' --run-level 'core_model'" and :114 same with -m 'core_model and cuda and H100'; unchanged, outside the diff: .buildkite/cuda/test-merge.yml:17 - "pytest -sv tests/model_executor -m 'core_model and cpu'" (CUDA marks never run on the merge gate), .buildkite/cuda/test-weekly.yml:155-174 'TTS · Moss · L4' looping only tests/e2e/...test_moss_tts_*.py with -m "slow and L4 and tts", and .buildkite/amd/test-amd-ready.yml:328-355 'ROCm · Model Executor GPU Coverage (R2-01)' with grade: NonBlocking and -m 'core_model and cuda and not nvidia_only and not (cards_2 or cards_3 or cards_4 or cards_5 or cards_6 or cards_7 or cards_8)' — the only lane that still collects it, NonBlocking on MI300. Sibling unchanged: tests/model_executor/models/moss_tts/test_local_state_kernels.py:9 pytestmark = [pytest.mark.core_model, pytest.mark.cuda]. Fix feasibility: pyproject.toml:303 "L4: [hardware-resource] [cuda] Tests that require L4 GPU"; tests/helpers/mark.py:295 def hardware_marks(*, res: dict[str, SkuSpec], num_cards: int | dict[str, int] = 1):.
Suggestion: pytestmark = [
pytest.mark.core_model,
pytest.mark.skipif(not torch.cuda.is_available(), reason="CUDA required"),
*hardware_marks(res={"cuda": "L4"}, num_cards=1),
]
Evidence for This diff's ready reorganization replaces the old SKU-agnostic `Engine&Model Ex…
This diff's ready reorganization replaces the old SKU-agnostic Engine&Model Executor Test (-m 'core_model and cuda') with two SKU-marked lanes — -m 'core_model and cuda and L4' (test-ready.yml:107) and -m 'core_model and cuda and H100' (:114) — so any core_model+cuda test without an SKU mark silently leaves CI. That already happened: tests/model_executor/models/moss_tts/test_local_state_kernels.py:9 and test_local_attention_heads.py:9-11 are pytestmark = [pytest.mark.core_model, pytest.mark.cuda] with no L4/H100 mark; they ran under the old selector, and nothing else picks them up (merge/ready Simple are cpu-only, test-merge.yml:17; weekly Moss runs only e2e moss files, test-weekly.yml:155-169; nightly has no tests/model_executor step). Guard the contract in tests/buildkite, which already asserts pipeline YAML (test_upload_pipeline.py:635-646): add a test that collects tests/{engine,model_executor,worker,worker_v2} and fails when a core_model+cuda item carries neither an L4 nor an H100 marker; in this PR, also mark the two moss_tts files or restore a catch-all lane.
Evidence: Changed by this diff: .buildkite/cuda/test-ready.yml:107 - "pytest -sv tests/engine/ tests/model_executor/ tests/worker_v2/ tests/worker/ -m 'core_model and cuda and L4' --run-level 'core_model'" and :114 same with and H100, replacing the removed job's -m 'core_model and cuda' — so a bare core_model+cuda item now matches neither lane. Unchanged by this diff, present in the PR-time tree: tests/model_executor/models/moss_tts/test_local_state_kernels.py:9 pytestmark = [pytest.mark.core_model, pytest.mark.cuda] (no L4/H100/cpu mark; the only other such file is test_local_attention_heads.py:10-11 pytest.mark.core_model, / pytest.mark.cuda,) — current trigger: both new ready lanes deselect it (and L4 and and H100 require marks only hardware_marks/hardware_test produce, tests/helpers/mark.py:295/318, and no conftest adds them), while the removed selector selected it; adverse effect: these two GPU unit-test files run on no CUDA lane at this head (merge/ready Simple are cpu-only: test-merge.yml:17 - "pytest -sv tests/model_executor -m 'core_model and cpu'"; weekly Moss covers only tests/e2e moss files: test-weekly.yml:155-169). Guard venue unchanged by this diff: tests/buildkite/test_upload_pipeline.py:639 _pipeline_dep_keys(Path(".buildkite/cuda/test-ready.yml")) inside test_pipeline_source_file_dependency_keys_are_registered (:635-646) shows tests/buildkite already validates pipeline YAML contracts.
Evidence for The body misdescribes the merged CI config
The body misdescribes the merged CI config. The diff adds one combined registry key engine_model_executor_worker_function (.buildkite/common/ci_source_file_dependencies.yml:226, sole consumer .buildkite/cuda/test-ready.yml:112); no keys named engine_model_executor_function / worker_function exist anywhere under .buildkite/. It also says tests/diffusion/models 'stays in its own 30-minute job', but both PR-gate pipelines rename that step to 'Simple · Diffusion Models Test' and cut it to timeout_in_minutes: 20 (.buildkite/cuda/test-merge.yml:20-21, .buildkite/cuda/test-ready.yml:19-20) — 30 minutes is the pre-PR value. The Test Plan's four separate per-tree commands also don't match the actual single combined four-tree command per SKU (.buildkite/cuda/test-ready.yml:104-115: pytest -sv tests/engine/ tests/model_executor/ tests/worker_v2/ tests/worker/ ... as one L4 step and one H100 step). Please correct the body to name the real key names, the 20-minute timeout, and the combined commands.
Evidence: .buildkite/common/ci_source_file_dependencies.yml:226 (added by this diff): engine_model_executor_worker_function: — the single combined registry key; its only consumer is .buildkite/cuda/test-ready.yml:112: source_file_dependencies: engine_model_executor_worker_function. Whole-word grep of .buildkite/ for \b(engine_model_executor_function|worker_function)\b: zero matches — no registry keys named as in the PR body exist. .buildkite/cuda/test-merge.yml:20-21: - label: "Simple · Diffusion Models Test" / timeout_in_minutes: 20; identical label+timeout at .buildkite/cuda/test-ready.yml:19-20 (pre-PR both were 'Simple · Diffusion Test' at 30). .buildkite/cuda/test-ready.yml:104-115: one group with a single combined four-tree command per SKU — :107 - "pytest -sv tests/engine/ tests/model_executor/ tests/worker_v2/ tests/worker/ -m 'core_model and cuda and L4' --run-level 'core_model'" and :114 the H100 twin — not four separate commands. Trigger/adverse effect: a reader routing jobs via the PR description greps for keys that do not exist and budgets 30m for a 20m job. The body-side quotes are external to the tree (per the fetch step's PR body; no gh tool in this step) — every tree-side assertion above was verified verbatim on the PR-time head.
🤖 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!
Omni ReviewBot: finding feedback[p1] The same lane split — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
PLEASE FILL IN THE PR DESCRIPTION HERE.
Contributor intent (required human answers)
1. What outcome do you want to achieve?
Keep CUDA ready and merge jobs inside their timeouts, and run the slow cases on the weekly pipeline instead of ready, merge, or nightly.
2. What is the highest-leverage variable for that outcome?
Which pipeline and GPU a job is attached to, and whether its
source_file_dependenciesis a shared registry key.3. How will this work move that variable?
Split the heavy ready CPU diffusion suite so
tests/diffusion/modelsstays in its own job and the other diffusion directories run with engine and entrypoints. Split engine/model-executor and worker ready jobs into L4 and H100 steps. Move Kandinsky6, JoyAI-Image-Edit accuracy, and the Higgs v3 full-response case onto weekly with theslowmark. Fold inline dependency lists intoci_source_file_dependencies.yml.4. Whose customer or user needs does this address, and what are those needs?
Contributors and CI maintainers who need ready/merge feedback without the slowest diffusion, TTS, and accuracy cases occupying those pipelines. Those cases still need a scheduled weekly run.
Purpose
tests/diffusion/models) stays in its own 30-minute job; the remainingtests/diffusiontree runs with entrypoints and engine. Engine/model-executor and worker tests are groups with separate L4 and H100 steps.advanced_modeltoslow.full_modeltoslow, hardware limited to H100 (cards_1), B200 mark removed.test_full_response_v1_preserves_audio:core_model/advanced_modeltoslow, dropped from ready. WeeklyTTS · H100already selectsslow and H100 and tts.source_file_dependencieslists with registry keys (engine_model_executor_function,worker_function,omni_qwen3_omni_shm_leak_function, reusing*qwen3_omni). Drop unuseddiffusion_sana_video_functionanddiffusion_joy_image_accuracyfilters so those weekly jobs are no longer diff-skipped. Removetests/buildkite/test_pi05_ci_routing.py, which asserted the old single diffusion CPU job.Test Plan
CI YAML and marker-only change. No new model runtime tests.
vLLM Version: N/A (pipeline YAML and pytest marks only)
vLLM-Omni Commit: d96bc72
Test Result
BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
Note
To avoid agent spam, vLLM-Omni limits contributors without write access to 5 open PRs. If this blocks well-intentioned critical work, please contact a committer to request bypass list access.
(anything written below this line will be removed by GitHub Actions)