test(multimodal): full-tensor parity for Qwen3 audio log-Mel vs transformers - #1910
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughA Python generator creates deterministic Qwen3-compatible audio MEL references, including resampled and batched cases. A Rust integration test loads the golden fixture and validates preprocessing tensors, tolerances, sums, shapes, contiguity, and batch metadata. ChangesAudio MEL parity
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Generator
participant WhisperFeatureExtractor
participant GoldenFixture
participant ParityTest
participant Qwen3AudioProcessor
Generator->>WhisperFeatureExtractor: compute deterministic log-mel references
Generator->>GoldenFixture: emit PCM, tensors, metadata, and sums
ParityTest->>GoldenFixture: load expected values
ParityTest->>Qwen3AudioProcessor: preprocess decoded or batched PCM
Qwen3AudioProcessor-->>ParityTest: return mel tensors and metadata
ParityTest->>GoldenFixture: compare values, shapes, sums, and metadata
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a Python script to generate a deterministic reference log-mel golden fixture using HuggingFace's WhisperFeatureExtractor for Qwen3 audio preprocessing, alongside a Rust integration test that verifies the Rust implementation's parity against this golden reference. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
e45756c to
3349b09
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/multimodal/tests/audio_mel_parity.rs`:
- Around line 330-350: Update the golden-case validation in the test loop to
collect or compare case names, then assert exact equality with the required
fixture matrix, including both resampling cases, short_clip, silence, and the
expected single/batch cases; preserve duplicate entries rather than
deduplicating names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 074054ff-2245-4c1b-88f4-29b371e3a165
📒 Files selected for processing (3)
crates/multimodal/scripts/generate_audio_mel_golden.pycrates/multimodal/tests/audio_mel_parity.rscrates/multimodal/tests/fixtures/golden/audio_mel_reference.json
| assert!(!golden.cases.is_empty(), "golden must contain cases"); | ||
| let mut saw_single = false; | ||
| let mut saw_batch = false; | ||
| for case in &golden.cases { | ||
| match case.kind.as_str() { | ||
| "single" => { | ||
| saw_single = true; | ||
| check_single(&processor, case); | ||
| } | ||
| "batch" => { | ||
| saw_batch = true; | ||
| check_batch(&processor, case); | ||
| } | ||
| other => panic!("{}: unknown case kind {other:?}", case.name), | ||
| } | ||
| } | ||
| assert!( | ||
| saw_single, | ||
| "golden must contain at least one single-clip case" | ||
| ); | ||
| assert!(saw_batch, "golden must contain the batched case"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Lock the fixture’s required case matrix.
The test only requires any single case and any batch case, so removing both resampling cases, short_clip, or silence would silently reduce the promised coverage. Assert the expected case names, including duplicates through exact equality.
Proposed fix
assert!(!golden.cases.is_empty(), "golden must contain cases");
- let mut saw_single = false;
- let mut saw_batch = false;
+ let mut case_names = golden
+ .cases
+ .iter()
+ .map(|case| case.name.as_str())
+ .collect::<Vec<_>>();
+ case_names.sort_unstable();
+ assert_eq!(
+ case_names,
+ vec![
+ "batch_16k",
+ "native_16k",
+ "resample_44100",
+ "resample_48000",
+ "short_clip",
+ "silence",
+ ],
+ "golden case coverage changed",
+ );
+
for case in &golden.cases {
match case.kind.as_str() {
"single" => {
- saw_single = true;
check_single(&processor, case);
}
"batch" => {
- saw_batch = true;
check_batch(&processor, case);
}
other => panic!("{}: unknown case kind {other:?}", case.name),
}
}
- assert!(
- saw_single,
- "golden must contain at least one single-clip case"
- );
- assert!(saw_batch, "golden must contain the batched case");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert!(!golden.cases.is_empty(), "golden must contain cases"); | |
| let mut saw_single = false; | |
| let mut saw_batch = false; | |
| for case in &golden.cases { | |
| match case.kind.as_str() { | |
| "single" => { | |
| saw_single = true; | |
| check_single(&processor, case); | |
| } | |
| "batch" => { | |
| saw_batch = true; | |
| check_batch(&processor, case); | |
| } | |
| other => panic!("{}: unknown case kind {other:?}", case.name), | |
| } | |
| } | |
| assert!( | |
| saw_single, | |
| "golden must contain at least one single-clip case" | |
| ); | |
| assert!(saw_batch, "golden must contain the batched case"); | |
| assert!(!golden.cases.is_empty(), "golden must contain cases"); | |
| let mut case_names = golden | |
| .cases | |
| .iter() | |
| .map(|case| case.name.as_str()) | |
| .collect::<Vec<_>>(); | |
| case_names.sort_unstable(); | |
| assert_eq!( | |
| case_names, | |
| vec![ | |
| "batch_16k", | |
| "native_16k", | |
| "resample_44100", | |
| "resample_48000", | |
| "short_clip", | |
| "silence", | |
| ], | |
| "golden case coverage changed", | |
| ); | |
| for case in &golden.cases { | |
| match case.kind.as_str() { | |
| "single" => { | |
| check_single(&processor, case); | |
| } | |
| "batch" => { | |
| check_batch(&processor, case); | |
| } | |
| other => panic!("{}: unknown case kind {other:?}", case.name), | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/multimodal/tests/audio_mel_parity.rs` around lines 330 - 350, Update
the golden-case validation in the test loop to collect or compare case names,
then assert exact equality with the required fixture matrix, including both
resampling cases, short_clip, silence, and the expected single/batch cases;
preserve duplicate entries rather than deduplicating names.
Add a multi-case, tolerance-based parity test for the pure-Rust Qwen3 audio log-mel frontend against HuggingFace transformers + torchaudio references. Qwen3-ASR / Qwen3-Omni consume audio through WhisperFeatureExtractor, so the golden generator constructs that extractor locally with Qwen3's preprocessor_config parameters (128 mel bins, 16 kHz, n_fft=400, hop_length=160) and calls its centered-STFT fbank routine directly (not the public __call__, which pads to 30 s) -- no weights or network needed. Non-16 kHz clips are first resampled with torchaudio.functional.resample, the kernel SMG's bandlimited_resample targets. Inputs and reference outputs are dumped to a checked-in JSON fixture the Rust test loads with include_str!. Cases: 16 kHz (mel only), 44.1 kHz and 48 kHz -> 16 kHz (resample path), a sub-n_fft clip (reflect-pad / frame-count edge), silence (log-norm floor), and a batch of three different-length 16 kHz clips exercising the padded [B, n_mels, max_frames] path plus feature_attention_mask and audio_feature_lengths. The bar is max-abs-diff on the full tensor, not bitwise equality, since the pure-Rust rustfft / resampler cannot be bit-identical to torch. Observed max-abs-diff per case is <= 4.6e-5; each case gates at 1e-3. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
3349b09 to
a1ab4b1
Compare
Description
Problem
#1905's audio log-Mel preprocessing had only unit spot-checks — ~10 hardcoded mel cells at
2e-4on a synthetic waveform — and no full-tensor parity against the model's actualtransformersfeature extractor.Solution
Add a full-tensor parity test comparing the pure-Rust Qwen3 log-Mel frontend against a committed golden generated from
transformers.WhisperFeatureExtractor(verified to be Qwen3-Omni/ASR's extractor). Deterministic seeded waveform; observed max-abs-diff 3.6e-5; tolerance 1e-3 + a sum sanity check. (Bitwise parity is not achievable for pure-Rustrustfftvs numpy/torch FFT; tolerance parity is the correctness gate.)Changes
crates/multimodal/tests/audio_mel_parity.rs(new): runsQwen3AudioProcessorlog-Mel on the golden PCM, asserts shape + max-abs-diff + sum.crates/multimodal/tests/fixtures/golden/audio_mel_reference.json(new): committed golden (PCM + mel), deterministic.crates/multimodal/scripts/generate_audio_mel_golden.py(new): regenerates the golden fromtransformers.Test Plan
cargo test -p llm-multimodal --test audio_mel_parity→ 1 passed (max-abs-diff 3.6e-5). Existing vision golden + audio lib tests still pass. Before: no full-tensor reference check; after: full mel tensor compared to the transformers golden within tolerance.Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
Tests
Chores