Repository navigation
[Benchmark] Add local OmniInteract performance cases - #6817
amy-why-3459 merged 8 commits into
Conversation
|
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 was classified as CI work. CI owner: @yenuo26 @natureofnature, 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. |
33bb1a4 to
80591df
Compare
hsliuustc0106
left a comment
There was a problem hiding this comment.
LGTM — static review at 80591df. The gate's asserted schema keys, artifact names, WAV format, and the repo@rev fallback routing (patch.py:384-392 → HfFileSystem repo@revision) all check out against the writers, and the module-scoped server keeps the model start shared. One P3 open (inline): the reported E2E ran with a local OMNIINTERACT_ROOT, so please report one run of the env-unset fallback path the nightly will actually take — or the archive size / download+extract time — so the first scheduled run is known to fit the 120-min budget.
|
Author self-review at
No additional blocking issue found in self-review. |
|
@amy-why-3459 PTAL |
|
Reviewed e9fd631. The Nightly contract itself is right: 12 real-time videos, no accuracy score, Request changes on the wiring, not the assertions. The existing MiniCPM-o Duplex Nightly step is already red on the two interrupt cases ( This YAML keeps all of that in one pytest, then raises the step from 50 to 300 minutes. After merge the new gate has no independent green, a warm run still pays the known interrupt failures plus ~30 min of videos, and a cold node can occupy Please split. Do not xfail the interrupt tests in this CI PR.
Also:
|
e9fd631 to
1d9414d
Compare
|
@amy-why-3459 Addressed at
Remote-container checks on the final files: Buildkite tests |
| mirror_hardwares: h100_1 | ||
|
|
||
| - label: ":full_moon: Omni · MiniCPM-o 4.5 OmniInteract Nightly" | ||
| timeout_in_minutes: 300 |
| - pytest -s -v tests/e2e/online_serving/test_minicpmo_4_5_duplex_expansion.py -m "full_model and cuda and H100 and omni and cards_1" --run-level "full_model" | ||
| mirror_hardwares: h100_1 | ||
|
|
||
| - label: ":full_moon: Omni · MiniCPM-o 4.5 OmniInteract Nightly" |
There was a problem hiding this comment.
Please don't include Nightly in label and please indicate the test type, such as Function or Perf.
There was a problem hiding this comment.
Addressed in 68b3dab. The step label is now MiniCPM-o 4.5 · OmniInteract Perf Test; Nightly is no longer part of the label.
| assert "key: upload-weekly-pipeline" in rendered | ||
|
|
||
|
|
||
| def test_minicpmo_omniinteract_nightly_isolated_from_duplex() -> None: |
There was a problem hiding this comment.
I think this test is redundant. Maybe it's better to remove it.
There was a problem hiding this comment.
Removed the dedicated Buildkite string-matching regression test. The step now uses the standard perf JSON runner and existing pipeline validation.
|
|
||
|
|
||
| @hardware_test(res={"cuda": "H100"}, num_cards=1) | ||
| @pytest.mark.skipif( |
There was a problem hiding this comment.
Please write the performance test cases in JSON format and place them under tests/dfx/perf/tests
There was a problem hiding this comment.
Addressed in 68b3dab. The three subset cases now live in tests/dfx/perf/tests/test_minicpmo_4_5_omniinteract.json and run through tests/dfx/perf/scripts/run_benchmark.py. The generic runner preserves num_warmups: 0 and asserts the OmniInteract lifecycle/artifact summary.
Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: Ruirui Yang | Rein <73573651+R2-Y@users.noreply.github.com>
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
1d9414d to
68b3dab
Compare
|
@ZacheryAU PTAL |
|
@amy-why-3459 PTAL |
| } | ||
|
|
||
|
|
||
| def _resolve_num_warmups(params: dict[str, Any], *, default: int) -> int: |
There was a problem hiding this comment.
This function is kind of non-negative integer validating helper, which could be centralized to metrics/utils.py later if several modules need it.
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: Ruirui Yang | Rein <73573651+R2-Y@users.noreply.github.com> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com>
Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: Ruirui Yang | Rein <73573651+R2-Y@users.noreply.github.com> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com> Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: Ruirui Yang | Rein <73573651+R2-Y@users.noreply.github.com> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com>
Purpose
This is the local performance-benchmark split from #5102, following the OmniInteract benchmark runner merged in #6522.
tests/dfx/perf/testsJSON format.1q1a,1q1a_math, and1qna, with four deterministic measured videos per subset (12 total), zero benchmark warmups, and maximum concurrency two.org/repoandorg/repo@revisionfilesystem paths.The generic performance runner gains only the two capabilities needed by these cases: an explicit
num_warmups: 0override and OmniInteract summary/artifact assertions. This PR does not change model/runtime code and does not score answer accuracy.Test Plan
vLLM Version: 0.28.0
Real-model E2E tested commit:
8b2b6095e1b4af2aa95c6d10243274c4aed6e6a1Current PR head:
e50be2da737c9115f41f7294050145d1120beab1Test Result
The JSON performance workload completed on one H800, using one shared model server:
3 passedin2300.13s(38m 20s).12/12 passed; every subset reportedartifacts_complete=true.67 passed.3 tests collected.run_benchmark.pypath and the target JSON.1q1a1q1a_math1qnaThe two successful-but-ineligible cases were excluded from
official_eval_manifest.jsonlbecause response audio crossed the fixed video playback horizon (audio_clipped). Their transport, response lifecycle, and required-artifact checks passed; official eligibility is intentionally separate from local benchmark completion.The real-model run used the existing local OmniInteract archive and was not rerun after the subsequent merge from
main. The final PR-only follow-up removes only the redundant local-launcher subprocess test; it does not change the measured workload. On a cache miss, the checked-in configuration downloads the pinned 7.81 GiB archive intoHF_HOME; subsequent local runs reuse that cache. Each subset also sends one endpoint-readiness request before its four measured cases.BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
(anything written below this line will be removed by GitHub Actions)