Repository navigation
test(tts): unified accuracy battery — every served checkpoint, one command - #102
hsliuustc0106 wants to merge 4 commits into
Conversation
…mmand scripts/acc_all_checkpoints.py runs the SAME battery across the fleet — 4× Qwen3-TTS checkpoints (preset/clone/design paths) + VoxCPM2 (zero-shot en/zh) — through the real serving surface (service.speech_bytes), one subprocess per model per the one-heavy-model rule: duration sanity + native sample rate HNR vs the per-path floors calibrated in the per-family batteries ASR round-trip vs the synthesized text (tests/asr_oracle.py, the intelligibility gate whisper-base hybrid built by scripts/build_asr_oracle.py — new to Qwen3-TTS, previously VoxCPM2-only) The Base clone path exercises the real voice-object request with the CustomVoice run's own vivian output as its reference clip. Floors are fleet tripwires (min measured minus margin; whisper-base wobbles ±0.2 at draw level) — the per-family batteries stay the precise gates. --calibrate reports stats for recalibration; exit 1 on any breach. First full run: 24/24 draws green, ~5 min on M4 (Qwen3 draws are seed-reproducible; VoxCPM2 varies ~2 dB with kernel nondeterminism, floors carry margin). Carries tests/asr_oracle.py + scripts/build_asr_oracle.py as the canonical home; if #98 merges first, drop its copies on rebase (same files).
…gs, all fixed Self-review per the repo's review protocol before merge: 1. worker didn't set HF_HUB_OFFLINE=1 — every weight-gated test file does; without it a flaky network hangs model loads. Fixed. 2. dual argparse parser at __main__ (add_help=False + parse_known_args then re-parse) was fragile — consolidated into one build_parser(). 3. --calibrate claimed 'suggested floors' but only printed a hint — now aggregates the run's rows into per-path suggested floor lines. 4. rt None rendered as nan — now '-'. 5. (introduced during this pass, caught by the re-run gate): the row print was dedented out of the per-line loop, printing only the last draw per checkpoint — fixed; full battery re-run after every change is the discipline that caught it. Calibration outcome from the re-runs: the zh round-trip is run-unstable at whisper-base quality — same seeds, 0.95/0.98 one run, 0.29/0.38 the next (matching the 0.39–0.97 fixed-config spread in the #88 equivalence data) — so zero-zh's rt_floor is None (report-only, noted inline) until a stronger zh oracle exists; zh keeps its HNR floor. English gates unchanged. Full battery after the fixes: 24/24 green.
|
Must-check review pass applied (per the repo review protocol, before any external review): 26ce634 carries 5 findings + fixes —
Calibration finding from the re-runs: the zh round-trip is run-unstable at whisper-base quality — same seeds scored 0.95/0.98 in one run and 0.29/0.38 in the next (consistent with the 0.39–0.97 fixed-config spread in the #88 equivalence data), so Verdict after the pass: merge-ready — 24/24 green, ruff clean, CI green on 3164c51 and re-queued on 26ce634. |
…r (delegated reviewer) (#107) .claude/skills/ encodes the review discipline this repo has accumulated so it stops living in one agent's memory: - precheck-pr — the author-side MUST-CHECK before a PR is ready: freeze the diff, machine gates (ruff E9,F + discover), the weight-gated test taxonomy (offline env, snapshot gate, ReleaseAfterClass, one heavy model per process), the A/B + probe-gated perf rule (fresh baselines, probe value with every number, thermal order-bias), the audio-quality gates (fleet battery, HNR floors, ASR round-trip with the zh caveat, paired-seed equivalence for 'same quality, faster' claims, audition for quality-affecting defaults), the re-run-after-every-fix gate, and parallel-session safety (worktrees get wiped; commit early). Born from #102's pass: five findings in a test-only diff, one introduced by the pass itself and caught by the re-run. - review-pr — the reviewer-side protocol: head-SHA freeze, verify library claims against the pinned .venv, evidence bars by PR type (perf / audio / loop-compile / serving — incl. the worker-thread requirement for anything load-adjacent), inline posting mechanics (-F for ints, commit_id), own-PR verdict fallback (--comment). CONTRIBUTING.md points at precheck-pr as the must-run step before opening a PR. Self-applied: ruff clean, full 219-test discover green (docs-only diff; the skills themselves are the deliverable). Co-authored-by: hsliu_ustc <hsliu_ustc@noreply.gitcode.com>
Resolve scripts/README.md (both sides appended entries — keep asr_roundtrip.py from #104 plus acc_all_checkpoints.py from this PR). No code conflicts: tests/asr_oracle.py stays at tests/ top level beside audio_metrics.py (only test_* files for the tts package moved into tests/tts/ per #106), and the battery invokes itself via --_worker subprocesses, so no test-module paths needed updating.
… compound default, resolution-honest statistics Review findings (EJqy, M1 Max), all addressed: 1. requires_oracle returned the decorator unapplied — the equivalence gate had NEVER RUN (unittest reported ok on a returned decorator object, with only a DeprecationWarning). Fixed and applied to the #102 copy of the file as well; skip path verified (skips cleanly without the oracle — previously a vacuous pass). 2. The timestep gate now covers en + en2 + zh. 3. Running the gates for real changed the defaults: t=6 vs t=10 at the 8/4-quantized weights degrades en round-trip −0.18 mean over 6 paired seeds (4/6 negative, ~2.5σ above oracle noise — the original #88 study validated t=6 on BF16 blocks in isolation and missed the knob interaction). t=8 measures +0.03 mean vs t=10 → default is now t=8 under quantization. RTF 0.57-0.61 → 0.80-0.81 quiet (probe 8.4-8.9 ms), still 2.5× vs the 2.00 baseline. New standing gate (quant-vs-bf16, paired seeds, both models in one process) + statistics made resolution-honest: per-draw absolute floor 0.5 for en/en2 is asserted (catastrophic detection — the rope-sign bug scored 0.2-0.3), sub-0.1 paired deltas are printed as EVIDENCE not asserted (per-pair oracle noise ±0.2-0.4, n=6 SE ~0.07 — a hard -0.03 gate flips on draw luck, which is exactly what the first re-runs showed); zh is report-only like the fleet battery (whisper-base zh spans 0.3-1.0 on clean speech). Weights integrity re-verified by sha256 after a cache wipe forced a re-download (unrelated to review). Batteries: 50/50 (loop + e2e + service).
| python scripts/acc_all_checkpoints.py --model voxcpm2 # one checkpoint (substring match) | ||
|
|
||
| Floors without a calibrated value yet are None and enforced as "report only" | ||
| until set. VLLM_OMNI_* env vars pass through (e.g. VLLM_OMNI_VOXCPM2_QUANT) |
There was a problem hiding this comment.
[P3] Docstring cites a nonexistent env var as the passthrough example. VLLM_OMNI_VOXCPM2_QUANT appears nowhere in src (it is #98's var, and only in its branch); the only VoxCPM2 env var on this tree is VLLM_OMNI_VOXCPM2_EAGER (tts/voxcpm2.py:105,176; voxcpm2_loop.py:33,52). An operator setting the documented var would silently measure the default path. Fix: use VLLM_OMNI_VOXCPM2_EAGER as the example.
| print(f"{key}: worker failed (rc={proc.returncode}): {proc.stderr.strip()[-400:]}") | ||
|
|
||
| if args.calibrate: | ||
| print("\ncalibration — suggested floors (min measured, rounded down; add your own margin):") |
There was a problem hiding this comment.
[P3] "Rounded down" is wrong — the suggested floor can exceed the measured minimum. :.1f/:.2f round to nearest, not down: a measured min of 3.76 dB yields a suggested floor of 3.8, which is above the min and would fail (hnr > floor is strict) the very run that produced it. Fix: math.floor(hnr_min * 10) / 10 (and * 100 / 100 for rt).
| "ref": "mlx-community/Qwen3-TTS-12Hz-1.7B-Base-4bit", | ||
| "kind": "qwen3", | ||
| "paths": [ | ||
| {"name": "clone", "text": "This sentence is spoken through the cloned reference voice.", "voice": {"ref_clip": True}, "hnr_floor_db": 0.0, "rt_floor": 0.55}, |
There was a problem hiding this comment.
[P3] Clone-row floor provenance claim doesn't hold. The docstring says floors are "calibrated in those per-family files", but the per-family files gate the Base clone path at CATASTROPHIC_HNR_DB = −5.0 (tests/tts/test_icl_stream.py:128, test_clone_scaffold.py:267), not this battery's 0.0. Stricter than the family calibration is the safe direction — just annotate the clone row as this battery's own calibration rather than citing the family files.
| #!/usr/bin/env python3 | ||
| """Unified TTS accuracy battery — every served checkpoint, one command (#88-adjacent). | ||
|
|
||
| The per-family test files (test_tts_generate, test_small_custom_voice, |
There was a problem hiding this comment.
[P3] "every served checkpoint × its served paths" overclaims — one buffered path per checkpoint. The battery exercises only service.speech_bytes once per checkpoint; VoxCPM2's served clone (voice dict) and design (instructions) paths (voxcpm2.py:261+, variants.py SERVED_BY_PATH) are absent, as are the streaming surfaces. Those stay covered by the per-family tests, but the header's path claim is broader than the battery. Fix: soften to "its buffered entry path" or add the missing VoxCPM2 rows.
| if isinstance(voice, dict) and voice.get("ref_clip"): | ||
| clip = base64.b64encode(REF_CLIP.read_bytes()).decode() | ||
| voice = {"ref_audio": clip, "ref_text": REF_TEXT} | ||
| for seed in SEEDS[:draws]: |
There was a problem hiding this comment.
[P3] --draws is silently capped at 3. SEEDS[:draws] with SEEDS = (11, 12, 13): --draws 10 runs 3 draws with no warning; --draws 0 runs none. Fix: validate 1 <= draws <= len(SEEDS) in build_parser() or derive seeds from the count.
| seconds = samples.size / rate | ||
| hnr = pcm_hnr_db(samples, sr=rate) | ||
| rt = round_trip_similarity(oracle, mx.array(samples), rate, path.get("asr_ref", path["text"])) if oracle else None | ||
| if path.get("save_ref") and seed == SEEDS[0] and not REF_CLIP.exists(): |
There was a problem hiding this comment.
[P3] Stale reference clip is silently reused. /tmp/acc_battery_ref.wav from a previous run (older checkpoint revision, different quant, pre-refactor audio) is reused as the clone reference with no freshness check, while the docstring claims the battery "always measures what would actually be served". Fix: embed a short text-hash in the clip filename or add --refresh-ref.
| return max(_similarity(transcript, ref) for ref in references) | ||
|
|
||
|
|
||
| def requires_oracle(test): |
There was a problem hiding this comment.
[P3] The fixed requires_oracle has no in-repo consumer at this head. The fix itself is correct (decorator now invoked on test), but nothing applies @requires_oracle here — the consumers live on #98's branch — so there is no signal if it ever regresses vacuous again, which is exactly how #98's copy went unnoticed. Fix: land the consuming tests or add a one-line meta-test asserting it returns a skipping wrapper.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Delegated review @ 7eece0d — verdict: approve-with-nits (7 nits posted inline, none blocking).
The substance holds up well. Verified at head (read-only):
- The
requires_oraclefix is correct — decorator now invoked ontest(tests/asr_oracle.py:123), and an exhaustive sweep of the whole test tree for vacuous-decorator patterns (returned-uncalledskipUnless/skipIf/pytest.mark, decorator factories) found no other instances. The tworequires_mlx_audio = unittest.skipUnless(...)aliases are correctly constructed and applied at all 8 sites. - All 5 self-review findings from 26ce634 are addressed at head (offline env in-worker, single parser with suppressed
--_worker,--calibrateaggregation, rt cell printing, zero-zh report-only). - CI safety is structural, not accidental:
unittest discovercollects onlytest*.py(neither new tests file matches), andasr_oracle.py's top-level imports are stdlib-only — mlx_audio is only imported inside guarded functions/worker subprocesses. - The coverage claim checks out against src: served fleet = CV17/CV06/VD17/BASE17 + VoxCPM2-4bit (bench list +
DEFAULT_MODEL+ voxcpm2.py), and the battery's MODELS contains exactly those five — no missing or unreachable checkpoint. (The "× its served paths" sub-claim is the one overclaim, nit posted.) - Isolation is stronger than the ReleaseAfterClass pattern (one checkpoint per subprocess, sequential workers).
Questions: (1) The rt floors have no committed calibration data — were the English floors validated across multiple full batteries (the 26ce634 note records a 0.29–0.98 zh wobble; is vivian's 0.70 comfortably above the en wobble band)? (2) The battery fails loud when weights are absent rather than skipping per the weight-gated convention — intended as an enforcement script for the weight machine only? (3) Is silently reusing the ref clip the intended reproducibility tradeoff (nit posted)?
Not executed (weight-gated): the battery and oracle — evidence reviewed only.
…ncoder quantization (#88) (#98) * perf(tts): #88 groundwork — solver knobs in the closure cache key + ASR round-trip oracle closures_for keys by (model, thread, inference_timesteps, cfg_value): the solver's t_span and cfg strength bake into the trace at build, so a runtime knob change now rebuilds instead of silently serving stale traces (the timestep sweep worked around this by clearing the cache). tests/asr_oracle.py + scripts/build_asr_oracle.py: the 'same audio quality' gate for optimization PRs — a local whisper-base (hybrid dir: mlx-community npz weights converted to safetensors + openai processor files; no single small repo carries both in mlx_audio.stt's layout) transcribes synthesized audio and scores it against the input text. Skips cleanly where the oracle isn't built; HNR floors remain the always-on gate. * perf(tts): VoxCPM2 3.5× — timestep knee + 8/4-bit DiT/encoder quantization, quality-gated (#88) Two serving-default changes, each behind the new ASR round-trip oracle (tests/asr_oracle.py; whisper-base hybrid built by scripts/build_asr_oracle.py — mlx-community npz weights converted to safetensors + openai processor files, no single small repo carries both in mlx_audio.stt's layout; transcripts trimmed to the reference span against short-clip tail hallucination, CJK references pin language=zh and accept simplified/traditional variants): 1. inference_timesteps 10 → 6 — the measured knee. Paired-seed gate (same noise draws, only solver steps differ; the paired median absorbs the oracle's decode wobble): en +0.012 / en2 +0.009 / zh −0.008 medians, means flat, HNR above floor. 5 estimator passes per patch instead of 9. Kept as a standing weight-gated test. 2. Load-time quantization of the two blocks the mlx-community conversion left bf16 (376M params ≈ 715 MiB streamed 9×/1× per 20 ms patch — the measured bandwidth floor). Default is the mix that passed the paired gate: 8-bit DiT + 4-bit encoder (en means 0.951→0.940, en2/zh up); pure 4-bit was marginal on en (0.951→0.857) and is opt-in via VLLM_OMNI_VOXCPM2_QUANT=4bit; =off restores bf16 blocks. Also: solver knobs join the closure cache key (t_span/cfg bake into the trace); the drift test compares the pre-divergence prefix with a two-patch stop tolerance (quantized weights sit the stop argmax on near-ties); bench gains --quant/--timesteps; parity battery mirrors the config knobs into the library call. Quiet-M4 numbers (probe 7.5 ms, the honest anchor): baseline (bf16, t=10, library eager) RTF 2.00 bf16 + t=6 + compiled loop RTF 1.03 (1.9×) 8/4-quant + t=6 + compiled loop RTF 0.57-0.61 (3.5×) ← default Battery: 14/14 weight-gated (bitwise eager parity, drift envelope, instruct/clone, escape env, worker thread, ASR equivalence gate) + 35 CI-safe. Interleaved-in-one-process A/Bs that ran quant AFTER bf16 understate quant by thermal order-bias — the standalone probe-gated runs are the reported numbers. Human audition pending: /tmp/voxcpm2_q8_{en,en2,zh}.wav vs /tmp/voxcpm2_bf16_{en,en2,zh}.wav. * fix(#98 review): gates that actually run — vacuous-decorator fix, t=8 compound default, resolution-honest statistics Review findings (EJqy, M1 Max), all addressed: 1. requires_oracle returned the decorator unapplied — the equivalence gate had NEVER RUN (unittest reported ok on a returned decorator object, with only a DeprecationWarning). Fixed and applied to the #102 copy of the file as well; skip path verified (skips cleanly without the oracle — previously a vacuous pass). 2. The timestep gate now covers en + en2 + zh. 3. Running the gates for real changed the defaults: t=6 vs t=10 at the 8/4-quantized weights degrades en round-trip −0.18 mean over 6 paired seeds (4/6 negative, ~2.5σ above oracle noise — the original #88 study validated t=6 on BF16 blocks in isolation and missed the knob interaction). t=8 measures +0.03 mean vs t=10 → default is now t=8 under quantization. RTF 0.57-0.61 → 0.80-0.81 quiet (probe 8.4-8.9 ms), still 2.5× vs the 2.00 baseline. New standing gate (quant-vs-bf16, paired seeds, both models in one process) + statistics made resolution-honest: per-draw absolute floor 0.5 for en/en2 is asserted (catastrophic detection — the rope-sign bug scored 0.2-0.3), sub-0.1 paired deltas are printed as EVIDENCE not asserted (per-pair oracle noise ±0.2-0.4, n=6 SE ~0.07 — a hard -0.03 gate flips on draw luck, which is exactly what the first re-runs showed); zh is report-only like the fleet battery (whisper-base zh spans 0.3-1.0 on clean speech). Weights integrity re-verified by sha256 after a cache wipe forced a re-download (unrelated to review). Batteries: 50/50 (loop + e2e + service). * fix(#98 review r2): quant mode in the closure-cache key + pooled zh evidence in the timestep gate Reviewer round 2 (EJqy @ fc9aeb3): 1. Cache key — agreed and done: _quant_signature (projection class + bits for the DiT estimator and feature encoder) joins (model, thread, timesteps, cfg). Traces bake in the weights; a model quantized after its closures were built must not reuse them. CI-safe regression test with stub modules. 2. zh timestep finding — addressed with statistics rather than a revert: their single-sentence -0.173 (their whisper decode) vs our pooled multi-sentence studies on the same comparison: standalone n=18 mean +0.003 ± 0.066 (t=0.05); the gate now pools THREE zh sentences x 4 seeds and prints the pooled mean (this run: n=12, -0.001). Single- sentence zh is sentence-level oracle noise — the same sentence's deltas swing -0.42..+0.41 across runs on identical weights. zh stays report-only (fleet-battery rule); en/en2 catastrophic floors unchanged; audition wavs /tmp/voxcpm2_zh_t{10,8}.wav for the human gate. If a listener hears the difference on their machine, inference_timesteps=10 is the documented opt-out and script-aware solver depth is the queued follow-up. Battery: 15/15 weight-gated + 36 CI-safe service tests. --------- Co-authored-by: hsliu_ustc <hsliu_ustc@noreply.gitcode.com>
Answers "do we have a unified acc test for all tts models?" — now yes.
What
scripts/acc_all_checkpoints.py: the same battery across the whole served fleet — 4× Qwen3-TTS checkpoints (1.7B/0.6B CustomVoice presets, Base cloning, VoiceDesign) + VoxCPM2 (zero-shot en/zh) — each through the real request surface (service.speech_bytes), each checkpoint in its own subprocess (the one-heavy-model rule; checkpoints stack past 16 GB in one process):tests/asr_oracle.py) — the intelligibility gate, previously VoxCPM2-only, now covering Qwen3-TTS for the first time (upstream vllm-omni applies round-trip on all their TTS models)The Base clone path exercises the real base64 voice-object request using the CustomVoice run's own vivian output as the reference clip (known transcript).
--calibratereports measured stats; exit code 1 on any breach — CI-able later.Design notes
test_tts_generate,test_small_custom_voice,test_voice_design,test_icl_stream,test_voxcpm2_*) stay the precise gates.tests/asr_oracle.py+scripts/build_asr_oracle.pyas the canonical home; overlaps perf(tts): VoxCPM2 3.5× — timestep knee + quality-gated 8/4-bit DiT/encoder quantization (#88) #98's copies of the same files — whichever merges first wins, the other drops them on rebase.First full run (this M4)
24/24 draws green across 8 paths in ~5 min — table in the commit message;
--calibrateoutput drove the floor values.