fix(audio): make the mel front-end mandatory and replace fabricated spectrograms - #2027
Conversation
AudioNeuralNetworkBase.MelSpec was nullable, and 132 models wrote some form of 'if (MelSpec is not null) return MelSpec.Forward(raw); return rawAudio;'. When no front-end had been assigned that forwarded the RAW WAVEFORM -- rank 1, no time axis -- into a transformer encoder. It did not fail there; it failed later inside an attention layer, naming no model. MelSpec is now non-null, defaulting to a transform built for SampleRate, and PreprocessAudio went abstract -> virtual with that default. 122 identical overrides were therefore DELETED rather than edited: the fallback was already unreachable once MelSpec could not be null. RequireTimeAxis rejects rank-1 features at the boundary, naming the model. AudioEventDetector stops flattening the spectrogram before a stack built on sequence, and reduces the frame axis by temporal max pooling (the multiple-instance-learning standard for weakly-labelled sound event detection). Tempogram and MadmomBeatTracker declared SampleRate/FftSize/HopLength and never used them for preprocessing; both now build a front-end from their own options.
…e a helper Two layers enforced a constraint in practice but never declared it, so violations surfaced deep inside a helper rather than at the boundary. ConditionalRandomFieldLayer.EnsureInitialized refused to run until _sequenceLength > 0, but it allocates nothing: the transition matrix [numClasses, numClasses] and the start/end scores [numClasses] are built and initialized by the lazy ctor, and the method's only real statement is _isInitialized = true. The precondition was copy-pasted from the sibling lazy layers, where it IS correct because those size tensors against a dimension only the first input reveals. Here it made ParameterCount and GetParameters() throw before the first forward, contradicting the layer's own remarks. OnFirstForward is gated on _firstForwardRan / IsShapeResolved, never on _isInitialized, so shape resolution is unaffected. GatedDeltaNetLayer.ForwardTraced read modelDim off the input and then never consulted it; the reshape uses the declared _modelDimension regardless, so a feature axis one step off died as an element-count failure inside Engine.Reshape, naming neither the layer nor the offending axis. Sequence length stays unvalidated: it is genuinely dynamic and threaded through the recurrence.
… the papers specify AudioVisualEventLocalizationNetwork.ComputeSpectrogram collapsed the whole waveform into a single [128] RMS-energy vector -- no FFT, no mel filterbank, no log, and NO TIME AXIS, so the attention stack had no sequence to attend over. Its bins indexed time chunks, not frequency. Replaced with the pipeline the paper specifies (Tian et al., ECCV 2018), built on the existing MelSpectrogram at VGGish geometry -- 16 kHz, 25 ms window, 10 ms hop, 64 mel bins over 125-7500 Hz, log(mel + 0.01) -- feeding a new VGGishAudioEmbedding (Hershey et al., ICASSP 2017) that reduces each 96-frame patch to one embedding, giving a genuine [segments, 128] sequence. ForwardForTraining had inherited the base's sequential walk over Layers, which is not this model's topology: Layers holds two independent encoder stacks, temporal and cross-modal attention, and four task heads. Chaining them feeds each stage an activation the next was never built to accept, and skips the audio front-end entirely. It now mirrors PredictCore. AudioVisualCorrespondenceNetwork's hand-rolled spectrogram computed bins as (bin+1)/128 -- no FFT, no filterbank -- and is now the real transform at its own geometry. MelSpectrogram gained an opt-in log(mel + offset) mode alongside its dB path, because VGGish specifies the offset form and dB is a different scale. The audio-embedding widths are threaded through CreateNewInstance, DeepCopy and both serialization halves; omitting any one rebuilds the model at a different shape and restores trained parameters onto it.
…guard AudioFrontEndContractTests covers the mechanism rather than enumerating models: the base default and the boundary guard are what every audio model inherits, so verifying those covers the 206 that exist today and the ones added tomorrow. A per-model list would rot, and instantiating every model needs per-model architecture arguments. The AVEL fixture passes explicit audio-embedding widths (64/32) so the conformance suite does not build a paper-scale VGGish per test.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Important Review skippedToo many files! This PR contains 141 files, which is 41 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (141)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…backing files 0.127.0 is the last release whose StreamingTensorPool leaks its backing store. The pool deletes its directory in Dispose, which never runs when the test host is killed or runs out of memory, and for this suite that is the ordinary outcome rather than the exotic one. 0.128.0 opens the backing file with FileOptions.DeleteOnClose, so the kernel reclaims it even on an abnormal exit, and sweeps the directories earlier runs already stranded. Measured on a developer machine running this suite: a single interrupted run left three orphaned backing.bin files totalling 97 GB in the temp directory. That filled the drive far enough that Windows began compressing files to reclaim space, which silently broke a Docker VHDX, because a compressed virtual disk fails every write. All four packages move together because they are published and consumed as a lockstep set, and 0.128.0 of each is on nuget.org. Restore resolves all four at 0.128.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… guards accept Two are dead stores. The idx++ on the last layer claim advanced a counter nothing reads again, and embeddingSize was computed and never used; the embedding's size comes from the tensor the layer returns. Removing the increment is behaviour-preserving because it was the final claim, and a comment now says an appended layer has to restore it. The third is not a simplification. CodeQL reads !(off > 0.0) as off <= 0.0, but logOffset is a double? and every comparison against NaN is false, so the rewrite would ADMIT NaN where the guard exists to reject it. A NaN offset reaches log(mel + NaN) and turns the whole spectrogram into NaN far from this constructor. Spelled the NaN case out instead, which satisfies the rule and keeps the rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…olvers-and-control Two files conflicted; both were cases where master and this branch had independently fixed the same underlying defect. ConditionalRandomFieldLayer.EnsureInitialized - took this branch's side. Both sides removed the sequence-length precondition that made GetParameters and ParameterCount throw before the first forward. Master kept the _isInitialized flag and only dropped the throw; this branch observed the flag had become write-only once the throw was gone and deleted the field along with its four assignments. The branch's change is a strict superset, and it is also the only side that compiles here: the auto-merge already took the field's deletion, so master's hunk referenced a field that no longer exists. AudioVisualEventLocalizationNetwork - took master's side in all three hunks, keeping this branch's visual-encoder work. Both sides fixed "the audio encoder has no sequence axis to attend over", but master's #2027 does it properly: real log-mel at VGGish geometry, a trainable VGGish embedding over 96-frame patches, and a traced concatenation plus MeanOverSegments so gradients survive. This branch's _melSpectrogram returned the raw mel and is fully subsumed. Master's constructor parameters had already auto-merged in, so its fields were required regardless. Kept from this branch, because master only touched audio: - SplitFrameIntoPatches and the VISUAL_PATCH_* constants, which give the visual attention stack its own sequence axis (16x48, preserving the 768-value budget the visual input projection was built for). - The GlobalAveragePool rewrite. It now averages per feature across steps instead of chunking a flat buffer, which is what both branches need now that audio is [segments, features] and visual is [patches, features]. Also removed SPECTROGRAM_FFT_LENGTH and SPECTROGRAM_HOP_LENGTH: their only consumer was the front-end master replaced, and their docs referenced SPECTROGRAM_BINS, which master deleted, leaving a dangling cref. Verification: - src/AiDotNet.csproj builds clean on net10.0 and net471, 0 errors, no new warnings attributable to either resolved file. - CompositeLayerLazyCtorIssue1213Tests (CRF lazy ctor) 7/7. - IntegrationTests.Solvers + FunctionMinimizationIntegrationTests 152/152. - VGGish + MelSpectrogram + AudioVisualCorrespondence 37/37, 1 skipped. - Logistic/Multinomial/Beta/DeepSurv/DeepHit/QuantileRegressionLinearProgram /CausalDiscoveryDeterminism 151/151. Pre-existing, not caused by this merge: - AudioVisualEventLocalizationNetworkTests.Clone_AfterTraining_ShouldPreserve LearnedWeights fails identically on origin/master (~4% relative error there, ~5% here; the absolute delta differs only because the visual patching changes the network's numeric scale). Issue #1221 class. - DeepSurvTests.CollinearFeatures_ShouldNotCrash failed once in a batch run and passed in isolation and on an identical re-run of the same batch.
…olvers-and-control Two files conflicted; both were cases where master and this branch had independently fixed the same underlying defect. ConditionalRandomFieldLayer.EnsureInitialized - took this branch's side. Both sides removed the sequence-length precondition that made GetParameters and ParameterCount throw before the first forward. Master kept the _isInitialized flag and only dropped the throw; this branch observed the flag had become write-only once the throw was gone and deleted the field along with its four assignments. The branch's change is a strict superset, and it is also the only side that compiles here: the auto-merge already took the field's deletion, so master's hunk referenced a field that no longer exists. AudioVisualEventLocalizationNetwork - took master's side in all three hunks, keeping this branch's visual-encoder work. Both sides fixed "the audio encoder has no sequence axis to attend over", but master's #2027 does it properly: real log-mel at VGGish geometry, a trainable VGGish embedding over 96-frame patches, and a traced concatenation plus MeanOverSegments so gradients survive. This branch's _melSpectrogram returned the raw mel and is fully subsumed. Master's constructor parameters had already auto-merged in, so its fields were required regardless. Kept from this branch, because master only touched audio: - SplitFrameIntoPatches and the VISUAL_PATCH_* constants, which give the visual attention stack its own sequence axis (16x48, preserving the 768-value budget the visual input projection was built for). - The GlobalAveragePool rewrite. It now averages per feature across steps instead of chunking a flat buffer, which is what both branches need now that audio is [segments, features] and visual is [patches, features]. Also removed SPECTROGRAM_FFT_LENGTH and SPECTROGRAM_HOP_LENGTH: their only consumer was the front-end master replaced, and their docs referenced SPECTROGRAM_BINS, which master deleted, leaving a dangling cref. Verification: - src/AiDotNet.csproj builds clean on net10.0 and net471, 0 errors, no new warnings attributable to either resolved file. - CompositeLayerLazyCtorIssue1213Tests (CRF lazy ctor) 7/7. - IntegrationTests.Solvers + FunctionMinimizationIntegrationTests 152/152. - VGGish + MelSpectrogram + AudioVisualCorrespondence 37/37, 1 skipped. - Logistic/Multinomial/Beta/DeepSurv/DeepHit/QuantileRegressionLinearProgram /CausalDiscoveryDeterminism 151/151. Pre-existing, not caused by this merge: - AudioVisualEventLocalizationNetworkTests.Clone_AfterTraining_ShouldPreserve LearnedWeights fails identically on origin/master (~4% relative error there, ~5% here; the absolute delta differs only because the visual patching changes the network's numeric scale). Issue #1221 class. - DeepSurvTests.CollinearFeatures_ShouldNotCrash failed once in a batch run and passed in isolation and on an identical re-run of the same batch.
What this fixes
The R1 class from the master CI baseline (run
32147073004at9f73afe31, 318 failing tests): the29 tests failing with
MultiHeadAttentionLayer requires rank>=2 input; got rank 1.That single symptom turned out to be three unrelated defects:
1. The mel front-end was optional, and its absence forwarded the raw waveform
AudioNeuralNetworkBase.MelSpecwas nullable, and 132 models wrote some form of:That does not fail where it happens. It fails much later and much less legibly, inside an attention
layer, with nothing naming the model that produced it.
MelSpecis now non-null, defaulting to a transform built forSampleRate.PreprocessAudiowentabstract->virtualwith that default, so 122 identical overrides weredeleted rather than edited — the fallback was already unreachable once
MelSpeccould not be null.RequireTimeAxisguard rejects rank-1 features at the boundary, naming the model._melSpectrogramfield with the same fallback now fall back to thebase's guaranteed front-end instead of to raw audio.
2. AudioVisualEventLocalizationNetwork: fake DSP, and a training path that was never its topology
ComputeSpectrogramcollapsed the whole waveform into a single[128]RMS-energy vector — no FFT,no mel filterbank, no log, and no time axis, so attention had no sequence to attend over. The
"bins" indexed time chunks, not frequency.
Replaced with the real pipeline the paper specifies (Tian et al., ECCV 2018), built on the existing
MelSpectrogramat VGGish geometry — 16 kHz, 25 ms window, 10 ms hop, 64 mel bins over 125-7500 Hz,log(mel + 0.01)— feeding a newVGGishAudioEmbedding(Hershey et al., ICASSP 2017) that reduceseach 96-frame patch to one embedding, yielding a genuine
[segments, 128]sequence.Separately,
ForwardForTraininginherited the base's sequential walk overLayers, which is notthis model's topology:
Layersholds two independent encoder stacks, temporal and cross-modalattention, and four task heads. Chaining them feeds each stage an activation the next was never built
to accept, and skips the audio front-end entirely. It now mirrors
PredictCore. This is the sameclass the repo already documents at
CausalGANGenerator.cs:573.3. AudioEventDetector flattened the spectrogram before a stack built on sequence
ClassifyWithNativeflattened[frames, mels]into one vector beforePredict, while the stack isDense -> LayerNorm -> PositionalEncoding(maxFrames, hiddenDim) -> MultiHeadAttention xN— layersthat exist only to operate over time. The sequence now passes through, and the frame axis is reduced
by temporal max pooling, the multiple-instance-learning standard for weakly-labelled sound event
detection (Wang et al. 2018; PANNs, TASLP 2020) and the aggregation that matches this model's own
framing of independent per-class presence.
4. Two layers enforced a shape constraint without ever stating it
The other two genuine R1 failures were the same species as each other: a constraint the layer
applies in practice but never declares, so it surfaces from inside a helper rather than at the
boundary.
ConditionalRandomFieldLayer.EnsureInitializedrefused to run until_sequenceLength > 0, but itallocates nothing -- the transition matrix
[numClasses, numClasses]and the start/end scores[numClasses]are built and initialized by the lazy ctor, and the method's only real statement is_isInitialized = true. The precondition was copy-pasted from the sibling lazy layers (RBFLayer,RBMLayer,ObliviousDecisionTreeLayer,PrimaryCapsuleLayer,DigitCapsuleLayer), where it iscorrect because those size tensors against a dimension only the first input reveals. Here it made
ParameterCountandGetParameters()throw before the first forward, contradicting the layer's ownremarks. Removed.
OnFirstForwardis gated on_firstForwardRan/IsShapeResolved, never on_isInitialized, so shape resolution is unaffected.GatedDeltaNetLayer.ForwardTracedreadmodelDimoff the input and then never consulted it -- thereshape uses the declared
_modelDimensionregardless -- so a feature axis one step off died as anelement-count failure inside
Engine.Reshape, naming neither the layer nor the offending axis. Itnow states the constraint at the boundary. Sequence length is deliberately left unvalidated: it is
genuinely dynamic and threaded through the recurrence, so only the feature axis is fixed by the
projection weights.
Also included
MelSpectrogramgained an opt-inlog(mel + offset)compression mode alongside its dB path,because VGGish specifies the offset form and dB is a different scale.
AudioVisualCorrespondenceNetwork's hand-rolled spectrogram (bins computed as(bin+1)/128, noFFT or filterbank) replaced with the real transform at its own geometry.
MadmomBeatTrackerandTempogramdeclaredSampleRate,FftSizeandHopLengthin options andthen never used them for preprocessing — their onset networks read raw sample amplitudes. Both now
build a front-end from their own options.
AudioFrontEndContractTests: 5 contracts covering the base default and the guard.What this deliberately does NOT do
The
MultiHeadAttentionLayerrank>=2 contract is unchanged. It is the only thing that caught anyof this. Relaxing it to accept rank-1 would have turned all 29 tests green while leaving attention a
no-op over a single token, and would have un-caught the same defect across every audio model.
Scope note
Of the 48 tests the CI analysis labelled R1, 17 are not shape failures at all — they are
parameter/clone/serialize defects owned by PR #2004, verified by reading their throw sites:
Layer type ... is not supported for deserializationDeserializationHelper, changed by #2004ParameterCount reports N but GetParameters() returned MCloning ... changed parameter-manifest fingerprintMatrix dimensions incompatible(3 tabular generators)The classifier that produced the CI report matched bare substrings ("not supported", "shape"), so its
other category counts should be treated as upper bounds.
Local verification
AVEL family, run serially (
--blame-hang,xunit.parallelizeTestCollections=false):21 failing on master -> 1. Zero occurrences of
requires rank>=2 input; got rank 1anywhere inthe run. Builds clean on net10.0 src, net471 src and net10.0 tests.
A note for whoever runs these locally: under the DEFAULT parallel configuration two testhosts run
the AVEL and AVC families concurrently, together reach ~17 GB, and the OS kills one -- which presents
as "Test host process crashed" and is easy to misread as a stack overflow or an infinite loop in a
specific test.
--blame-hangprints "All tests finished running, Sequence file will not begenerated" when nothing actually hung, which is what separates "slow" from "wedged"; a frozen log
looks identical in both cases.
The one remaining AVEL failure is not from this PR
Clone_AfterTraining_ShouldPreserveLearnedWeightsstill fails, with||Δ|| = 1.138E-003against||trained|| = 2.875E-002-- about 4% relative. It was already failingon master (as one of the 21 rank-1 crashes); removing the crash exposed a defect underneath it.
It is not specific to this model. 11 models fail
Clone_AfterTrainingon master -- AudioGen,Cutie, Nougat, RecurrentGemma, GraphGeneration, InternImage, LiquidStateMachine, QuantumNeuralNetwork
among them -- across the R1, C1 and N1 classes. Evidence that it is the shared round-trip and not
VGGishAudioEmbedding:diffL2 << mag, so this is not the "all weights dropped" class;AudioVisualCorrespondenceNetwork, which gets a real mel front-end here but has no compositelayer, clones correctly;
VGGishAudioEmbeddingis inLayers, so the base serialization walk covers it, and it registersall 14 children through
RegisterSubLayerwithout overriding the parameter surface.Left for the clone/parameter-manifest work rather than patched around here.
AudioEventDetectorTests.DifferentInputs_AfterTrainingalso still fails, with a message and anL2 = 0.000E+000byte-identical to the master baseline. Untouched by this PR, and outside the R1scope it addresses. Its being unchanged also rules out the theory that the new mel front-end
flattens the constant probe inputs (
[0.1,...]vs[0.9,...]) by discarding DC -- it failedidentically before any front-end existed.
Acceptance check against the master CI baseline
Method and caveats follow
AiDotNet-CI-Failure-Analysis.md: compare failing TEST counts, neverfailing shard names, and never treat a cancelled or runner-killed shard as green.
Before — master run
32182588897at264780d8b2(fix(optimizers): ... (#2009)), the most recent merge to master.After — this PR, run
32264274637at81e2b0e996.1. Scale
264780d8b281e2b0e996CodeQL Analysis, not a test shard)Every test shard completed in this run; the only cancelled job is
CodeQL Analysis.2. Does it fix what it claims? — R1, yes: 29 → 1
The 29 R1 rows in the baseline CSV (
MultiHeadAttentionLayer requires rank>=2 input; got rank 1):264780d8b228 of 29 fixed. The single holdout is
AudioVisualEventLocalizationNetworkTests.Clone_AfterTraining_ShouldPreserveLearnedWeights— aclone round-trip failure (C1), which is #2004's scope, not R1's. It is discussed above.
36 tests flip master-fail → PR-pass, and they land exactly where the three defects were:
AudioVisualEventLocalizationNetworkTestsAudio.Classification.ClassificationTests(AudioEventDetector)UnitTests.SpeechRecognitionmechanism tests3. Does it increase failing tests elsewhere? — 14 appear; 1 is a real, reproducing new failure
Full disclosure of all 14, classified by whether the comparison is even valid:
(a) Not comparable — the shard produced no
Total tests:summary on one or both sides (6)ComputerVisionExtendedIntegrationTests.DETRSetLoss_Gradient_IsNonZeroPointCloudMedicalSegmentationIntegrationTests.SegMamba_Predict_ReturnsOutputSTCConnectorLayerTests.Serialize_DeserializeSVTRThinPlateSplineLayerTests.Serialize_DeserializeSVTRThinPlateSplineLayerTests.TapeGradientSubpixelConvolutionalLayerTests.Serialize_Deserialize(b) Pre-existing instability — already failing in the #2006 baseline, passed in #2009 (3)
SparseVariationalGaussianProcessTests.NoiseVarianceRecovery,DenseBlockLayerTests.Serialize_Deserialize,DenseBlockTests.Serialize_Deserialize. These flip between master runs without any PR.(c) Genuine deltas on shards that completed identically on both sides (5)
SpyNetLayerTests.TapeGradient_ShouldMatchNumericalGradient05331385e3and81e2b0e996), never on masterDDPMModelTests.Clone_ShouldProduceIdenticalOutput05331385e3, where the shard completedNEATTests.Clone_AfterTraining_ShouldPreserveLearnedWeights05331385e3NEATTests.ForwardPass_ShouldBeFinite_AfterTraining05331385e3SAM2Tests.DifferentInputs_AfterTrainingThe honest headline: 36 fixed, 1 reproducible new failure, net −22.
The one that needs a decision:
SpyNetLayerTests.TapeGradientIt is the only addition that reproduces across two independent PR runs at different SHAs while
never appearing on master. It is not reachable from the model changes here — the
LayerHelpereditis additive and confined to
CreateAudioVisualEventLocalizationLayers, appended last precisely sothe
[idx++]contract is unchanged. The only library-wide mechanism in this PR is the dependencybump:
A numeric-vs-finite-difference gradient check is exactly the kind of assertion a tensor-backend
release can move. That bump is load-bearing here (0.127.0's
StreamingTensorPoolleaks its backingstore on an abnormal exit — 97 GB of orphans measured), so the choice is to accept this one test as
a known consequence of the bump or to chase it separately.
4. Method
Set differences computed with
grep -Fxf— notecommmis-collates these fully-qualified names andsilently under-reports the intersection.
Correction (post-merge):
SpyNetLayerTests.TapeGradientis NOT caused by the Tensors bumpThe section above attributed this test to
AiDotNet.Tensors 0.127.0 → 0.128.0, on the grounds thatit was the only library-wide mechanism in this PR. Cross-referencing against PR #2025 disproves that:
9f73afe31b(#2006)264780d8b2(#2009)102d97e6cd05331385e381e2b0e996PR #2025 changes 5 test-harness files, touches neither
Directory.Packages.propsnor SpyNet, and isbased on
9f73afe31b— it carries no Tensors bump at all, yet shows the same failure. 3/3 in PRruns against 0/2 in master runs makes this order- or environment-dependent, not a consequence of the
dependency bump. It needs its own investigation and should not be recorded against this change.
Also, for the record: the "Failed jobs 57 → 58" row counts jobs inside
Build & SonarCloud. ThePR page counts check-runs across all eight workflows, which is 58 → 59, and the +1 there is
CodeQL Analysis—cancelledas a job, renderedfailas a check, and not a test shard.