Repository navigation
[CI/Build] Fix Qwen3-Omni Talker benchmark workload #8057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
491193e
5352010
d7f0f55
092a470
beb9650
3c459f1
57a471d
3dda9d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| [ | ||
| { | ||
| "test_name": "test_qwen3_omni", | ||
| "description": "random row: fixed workload (Thinker 900 / Talker 1536 tokens, min_tokens == max_tokens) for comparable audio RTF; H100 baseline pending recalibration, see #8100", | ||
| "mark": [ | ||
| { | ||
| "hardware_marks": { | ||
|
|
@@ -43,17 +44,28 @@ | |
| "random_output_len": 900, | ||
| "ignore_eos": true, | ||
| "percentile-metrics": "ttft,tpot,itl,e2el,audio_rtf,audio_ttfp,audio_duration", | ||
| "baseline": { | ||
| "H100": { | ||
| "mean_audio_rtf": [ | ||
| 0.169, | ||
| 0.2041, | ||
| 0.2382, | ||
| 0.3177, | ||
| 0.4595 | ||
| ] | ||
| } | ||
| } | ||
| "seed": 0, | ||
| "random_range_ratio": 0.0, | ||
| "extra_body": { | ||
| "return_stage_metrics": true, | ||
| "sampling_params_list": [ | ||
| { | ||
| "temperature": 0.0, | ||
| "min_tokens": 900, | ||
| "max_tokens": 900, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Thinker sampling_params_list[0] (JSON:53) sets only max_tokens=900 and ignore_e… Thinker sampling_params_list[0] (JSON:53) sets only max_tokens=900 and ignore_eos. _assert_fixed_stage_workloads keeps a stage only when min_tokens==max_tokens (run_benchmark.py:272), so nightly never checks stage "0". The accept-path unit test uses this same Thinker-only-max_tokens shape and snapshots only stage "1" (test_runner_metadata.py:720). A short Thinker still passes. README calibration requires Thinker 900 and notes ignore_eos does not lock length. Add min_tokens=900 so stage "0" enters fixed_lengths. Evidence: tests/dfx/perf/tests/test_qwen3_omni_no_async_chunk.json:51-55 Suggestion: { |
||
| "ignore_eos": true | ||
| }, | ||
| { | ||
| "temperature": 0.9, | ||
| "top_k": 50, | ||
| "repetition_penalty": 1.05, | ||
| "seed": 0, | ||
| "min_tokens": 1536, | ||
| "max_tokens": 1536 | ||
| } | ||
| ] | ||
| }, | ||
| "baseline": {} | ||
| }, | ||
| { | ||
| "dataset_name": "random-mm", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1203,6 +1203,27 @@ def _update_output_stage_metrics_from_payload( | |
| output.stage_metrics.update(stage_snapshot) | ||
|
|
||
|
|
||
| # Per-request stage fields persisted in benchmark results. Full snapshots carry | ||
| # per-token latency lists, which would inflate every chat-omni result file. | ||
| _REQUEST_STAGE_METRIC_FIELDS = ( | ||
| defs.NUM_TOKENS_OUT, | ||
| "finish_reason", | ||
| defs.AUDIO_FRAMES, | ||
| f"{defs.AUDIO_DURATION}_s", | ||
| ) | ||
|
|
||
|
|
||
| def _compact_request_stage_metrics(snapshot: object) -> dict[str, dict] | None: | ||
| """Keep the stage fields used for workload checks; empty snapshots become None.""" | ||
| if not isinstance(snapshot, dict) or not snapshot: | ||
| return None | ||
| return { | ||
| stage: {field: metrics[field] for field in _REQUEST_STAGE_METRIC_FIELDS if field in metrics} | ||
| for stage, metrics in snapshot.items() | ||
| if isinstance(metrics, dict) | ||
| } | ||
|
|
||
|
|
||
| def _apply_chat_stage0_token_timings(output: MixRequestFuncOutput) -> bool: | ||
| """Apply native Stage 0 timings from a chat response snapshot.""" | ||
| stage_metrics = output.stage_metrics | ||
|
|
@@ -3635,6 +3656,14 @@ def measured_ttft(output: RequestFuncOutput) -> float | None: | |
| "input_lens": [output.prompt_len for output in outputs], | ||
| "errors": [output.error for output in outputs], | ||
| } | ||
| # Preserve request order, including missing snapshots, so CI can verify | ||
| # fixed stage workloads without parsing logs or storing audio payloads. | ||
| request_stage_metrics = [ | ||
| _compact_request_stage_metrics(getattr(output, "stage_metrics", None)) for output in outputs | ||
| ] | ||
| if any(request_stage_metrics): | ||
| result["request_stage_metrics"] = request_stage_metrics | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it is only added here, it will merely be written into the result file and will not appear in the benchmark's printed output. I'm not sure whether this matches your expectation.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional: the runner reads the saved result JSON, so the check doesn't depend on stdout, and printing 128 per-request snapshots would flood the log. I added a one-line summary after the check passes ( |
||
|
|
||
| # Plain-vLLM backends (e.g. the vLLM-text perf config) return upstream | ||
| # RequestFuncOutput objects without the Mix duplex fields; read them | ||
| # tolerantly or the whole benchmark result is discarded ("fallback to | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to add a validation here that the talker needs to reach max_token, then shouldn't the thinker also add a validation that it reaches max_tokens after ignore_eos takes effect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Thinker now has
min_tokens == max_tokens == 900, so stage 0 enters the same check. Behavior is unchanged (Thinker has no extra stop tokens and was already 900 on all 276 local requests), but it is now enforced. Added reject cases for a missing/short Thinker.