feat: add BEATs golden example model with audio facade wiring - #867
Merged
Merged
Conversation
Implement BEATs (Audio Pre-Training with Acoustic Tokenizers, Chen et al. ICML 2023) as the golden example model for issue #396. This model achieves SOTA results (50.6% mAP AudioSet-2M, 98.1% ESC-50) and demonstrates the full pattern all 87 audio models will follow. Architecture: patch embedding + positional encoding + transformer encoder + classification head. Supports both ONNX inference and native training. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove extra constructor, keep ONNX + native constructors only - Use LayerHelper.CreateDefaultBEATsLayers() for InitializeLayers instead of hand-rolled layer fields (matching GenreClassifier/AudioEventDetector pattern) - Add CreateDefaultBEATsLayers to LayerHelper.cs with full BEATs_iter3 paper architecture (patch projection, pre-LN, 12x Transformer encoder, post-LN, classification MLP) - Add thorough beginner-friendly documentation to every method explaining how it works in the context of the BEATs model specifically - Use standard layer-stack forward pass pattern via Predict() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add internal ConfiguredModel property to AiModelBuilder for domain extension methods to cast the model to specific interfaces - Create AiModelBuilder.Audio.cs with extension methods that cast ConfiguredModel to IAudioEventDetector<T> for DetectAudioEvents, DetectAudioEventsAsync, DetectSpecificAudioEvents, GetAudioEventProbabilities, StartAudioEventStreaming, etc. - Make CreateAsync internal on BEATs, AudioEventDetector, GenreClassifier, SceneClassifier, MusicSourceSeparator to hide implementation details behind the facade pattern - Audio models use ConfigureModel() (not separate Configure methods) since they implement IFullModel<T, Tensor<T>, Tensor<T>> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an audio extension surface to AiModelBuilder and introduces ~70 new audio model implementations plus many ModelOptions across classification, event detection, enhancement, fingerprinting, foundations, generation, and effects. Most models support ONNX/native dual modes, streaming sessions, per-model serialization, and windowing/statistics utilities; several CreateAsync factories were tightened (public → internal) and optimizer fields made nullable across components. BLOCKING: new model implementations must be audited for stubs, TODOs, and non-production code paths before merge. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(70,130,180,0.5)
participant Client
end
rect rgba(46,139,87,0.5)
participant AudioBuilderExtensions
participant AiModelBuilder
end
rect rgba(255,165,0,0.5)
participant Model as IAudioEventDetector<T>
participant MelSpec as MelSpectrogram
end
Client->>AudioBuilderExtensions: DetectAudioEvents(audio, threshold)
AudioBuilderExtensions->>AiModelBuilder: get ConfiguredModel
AiModelBuilder-->>AudioBuilderExtensions: IFullModel<T,...> instance
AudioBuilderExtensions->>Model: Detect(audio, threshold)
Model->>MelSpec: PreprocessAudio(rawAudio)
MelSpec-->>Model: melSpec
Model->>Model: Predict(melSpec)
Model->>Model: PostprocessOutput(logits)
Model-->>AudioBuilderExtensions: AudioEventResult<T>
AudioBuilderExtensions-->>Client: AudioEventResult<T>
Client->>AudioBuilderExtensions: StartAudioEventStreaming(sampleRate, threshold)
AudioBuilderExtensions->>Model: StartStreamingSession(sampleRate, threshold)
Model-->>AudioBuilderExtensions: IStreamingEventDetectionSession<T>
AudioBuilderExtensions-->>Client: IStreamingEventDetectionSession<T>
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds the BEATs (Audio Pre-Training with Acoustic Tokenizers) model implementation as a golden example for audio event detection, along with facade pattern wiring through AiModelBuilder extension methods. All existing audio model factory methods are internalized to enforce facade-only access.
Changes:
- Implements complete BEATs model with dual-mode support (ONNX inference and native training)
- Adds audio facade extensions enabling detection through AiModelBuilder
- Internalizes CreateAsync methods across all audio models to protect IP and enforce facade access
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Helpers/LayerHelper.cs | Adds CreateDefaultBEATsLayers factory method for Transformer-based audio architecture |
| src/Audio/Classification/BEATsOptions.cs | Configuration class with paper-default hyperparameters for BEATs model |
| src/Audio/Classification/BEATs.cs | Complete BEATs model implementation with IAudioEventDetector interface and streaming support |
| src/Audio/SourceSeparation/MusicSourceSeparator.cs | Internalizes CreateAsync to enforce facade-only access |
| src/Audio/Classification/SceneClassifier.cs | Internalizes CreateAsync to enforce facade-only access |
| src/Audio/Classification/GenreClassifier.cs | Internalizes CreateAsync to enforce facade-only access |
| src/Audio/Classification/AudioEventDetector.cs | Internalizes CreateAsync to enforce facade-only access |
| src/AiModelBuilder.cs | Adds internal ConfiguredModel property for domain-specific extension method access |
| src/AiModelBuilder.Audio.cs | Audio event detection extension methods for AiModelBuilder facade |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@src/Audio/Classification/BEATs.cs`:
- Around line 680-685: DetectAsync accepts a CancellationToken but it isn't
honored because Detect(...) doesn't check it; update the code so cancellation is
actually propagated by adding an overload/internal variant of Detect that takes
a CancellationToken (or change the existing Detect signature) and call
cancellationToken.ThrowIfCancellationRequested() at appropriate points inside
the window-processing loop (e.g., between windows/batches) so work can be
aborted, then have DetectAsync call the new Detect(audio, cancellationToken) so
cancellation requests cancel the running detection; reference DetectAsync,
Detect, AudioEventResult<T>, and Tensor<T> when locating/editing code.
- Around line 1447-1457: Replace the silent uniform-probability fallback in the
else branch (the block that creates the "fallback" array and returns it) with a
fail-fast exception: detect when neither ONNX nor native inference is available
and throw a clear InvalidOperationException (or custom exception) from the same
method (the method that currently returns the T[] probabilities) with a message
stating inference is unavailable and indicating the missing model/flags (e.g.,
ONNX file missing or native mode disabled) so callers cannot mistake garbage
0.01 scores for valid predictions.
- Around line 1265-1295: SerializeNetworkSpecificData is missing writes for
_options.FMin and _options.FMax while DeserializeNetworkSpecificData expects
them when recreating the MelSpectrogram; add writer.Write(_options.FMin) and
writer.Write(_options.FMax) in SerializeNetworkSpecificData (in the same region
where other _options fields like WindowSize/WindowOverlap are written) so
deserialization reads the correct values for _options.FMin/_options.FMax and
MelSpectrogram construction.
- Line 1078: The computed loss from
LossFunction.CalculateLoss(output.ToVector(), expected.ToVector()) is discarded;
add a public double? LastTrainingLoss property to the BEATs class and assign the
computed loss to it immediately after the calculation (inside the Train or
training-step method where LossFunction.CalculateLoss is called) so callers can
observe recent training loss; ensure the property is updated each iteration (and
make it volatile or use proper synchronization if training is multi-threaded).
… PANNs, CLAP) Add 6 audio classification models following the BEATs golden example pattern with ONNX + native constructors, options classes, LayerHelper entries, full XML docs, IAudioEventDetector implementation, serialization, and streaming. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 3 speech enhancement models following the golden example pattern with ONNX + native constructors, options classes, LayerHelper entries, full XML docs, IAudioEnhancer implementation, serialization, and streaming. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rmer, HTDemucs) Add 3 music source separation models following the golden example pattern with ONNX + native constructors, options classes, LayerHelper entries, full XML docs, IMusicSourceSeparator implementation, serialization, and multi-stem separation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 3 speaker recognition models following the golden BEATs pattern: - ECAPATDNNSpeaker: ISpeakerVerifier + ISpeakerEmbeddingExtractor - TitaNet: ISpeakerVerifier + ISpeakerEmbeddingExtractor - PyAnnote: ISpeakerDiarizer with overlap detection support Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add two speech emotion recognition models following the BEATs golden example pattern with ONNX/native constructors, IEmotionRecognizer interface, arousal/valence estimation, and time series analysis. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add three music information retrieval models: CREPE for monophonic pitch detection (IPitchDetector), Basic Pitch and Onsets and Frames for polyphonic music transcription (new IMusicTranscriber interface). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Neural Audio Fingerprint model implementing IAudioFingerprinter for learned audio identification using contrastive embeddings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add neural audio codec models with new IAudioCodec interface for encode/decode with residual vector quantization. New Generation directory for audio codec and generation models. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add self-supervised speech representation models with new IAudioFoundationModel<T> interface for embedding extraction. Includes shared CreateDefaultFoundationModelLayers LayerHelper entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Conformer uses macaron-style FF/Attn/Conv/FF blocks for ASR. CTC Decoder supports greedy and beam-search decoding with LM rescoring. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
StyleTTS 2 supports voice cloning and emotion control via diffusion. HiFi-GAN is a real-time neural vocoder for mel-to-waveform conversion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MP-SENet: dual-path magnitude+phase estimation for speech enhancement. FRCRN: frequency recurrence CRN, DNS Challenge 2022 winner. BandSplitRNN Enhancer: band-wise processing with cross-band fusion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…icStructureAnalyzer, MelodyExtractor, MusicTaggingTransformer) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rmerFP) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…E, FishSpeech) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ioLanguageModel interface Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Audio/Enhancement/BandSplitRNNEnhancer.cs`:
- Line 125: The method EnhanceWithReference currently ignores the reference
parameter by delegating to Enhance(audio); replace this unsafe placeholder with
an explicit rejection: in BandSplitRNNEnhancer.Make
EnhanceWithReference(Tensor<T> audio, Tensor<T> reference) throw a
NotSupportedException (or ArgumentException) with a clear message like
"Reference-guided enhancement is not supported by BandSplitRNNEnhancer" so
callers fail fast; alternatively, if you intend to implement reference-guided
processing, implement the logic here and call the new implementation instead of
Enhance — change the body of EnhanceWithReference to either throw or call the
real reference-aware routine rather than ignoring the reference.
- Around line 28-33: Update the XML usage example to show the facade-based flow:
replace direct instantiation of NeuralNetworkArchitecture<T> and
BandSplitRNNEnhancer<T> with the AiModelBuilder and AiModelResult APIs (use
AiModelBuilder to configure/load the "bsrnn_enhancer.onnx" model and call its
Build/Run or equivalent to get an AiModelResult, then call the result's method
to Enhance or obtain the clean audio). Ensure the example references
AiModelBuilder and AiModelResult instead of NeuralNetworkArchitecture and
BandSplitRNNEnhancer so it follows the facade-only usage guidance.
- Line 231: The CreateNewInstance implementation currently always returns a
native-mode BandSplitRNNEnhancer<T>; update it to preserve ONNX mode and the
model path from the current instance by checking the instance's ONNX
flag/property (e.g. IsOnnx or similar) and the stored onnx model path, and then
constructing the clone with the same mode and path (either by calling an
existing constructor that accepts an ONNX flag/path or by adding one), e.g.
return a new BandSplitRNNEnhancer<T>(Architecture, _options, isOnnx: true,
onnxModelPath: _onnxModelPath) when the current instance is ONNX-backed,
otherwise return the native-mode constructor as before.
In `@src/Audio/Enhancement/CMGAN.cs`:
- Line 260: CreateNewInstance currently always constructs a native
CMGAN<T>(Architecture, _options) which drops ONNX mode; modify CreateNewInstance
to check the _useNativeMode flag: if true return new CMGAN<T>(Architecture,
_options) as now, otherwise require ModelPath to be non-null/valid and return
the ONNX-backed instance that preserves ModelPath and ONNX mode (e.g. the CMGAN
constructor/initializer that accepts ModelPath and Architecture/_options for
ONNX). Ensure you validate ModelPath and throw/handle a clear error when ONNX
mode is requested but ModelPath is missing.
- Around line 139-150: The Enhance method currently overwrites the
EnhancementStrength property on every call and never applies it to the predicted
mask; move the default assignment from Enhance into the class constructors so
EnhancementStrength is initialized once from _options.EnhancementStrength (do
not set it inside Enhance), and before calling ApplyMask blend the predicted
mask with an identity/pass-through mask using the EnhancementStrength (e.g.,
interpolate between the predicted mask and a neutral mask of ones according to
EnhancementStrength) so the user-tuned strength actually scales the effect;
update references to mask, ApplyMask, Enhance, EnhancementStrength and
constructors accordingly.
In `@src/Audio/Enhancement/DeepFilterNet.cs`:
- Around line 263-266: Validate _fftSize and _hopSize before initializing STFT:
in the DeepFilterNet constructors that set _fftSize/_hopSize and call
NextPowerOfTwo, add explicit checks that _fftSize > 0, _hopSize > 0 and _hopSize
<= _fftSize, and throw a clear ArgumentException/ArgumentOutOfRangeException for
invalid inputs; also guard NextPowerOfTwo so it never returns 0 for non-positive
input (validate or throw before calling NextPowerOfTwo), then compute nFft =
NextPowerOfTwo(_fftSize) and proceed to instantiate ShortTimeFourierTransform<T>
only after validation; apply the same validation logic to the other constructor
sites that initialize STFT (the other DeepFilterNet constructors and the blocks
around the other NextPowerOfTwo usages).
- Around line 706-709: Replace the hard-coded smoothing coefficients 0.1 and 0.9
with a named, configurable value: add a smoothing parameter to
DeepFilterNetOptions (e.g., SmoothingFactor) or define a descriptive const in
the DeepFilterNet class, then use that parameter when computing the smoothed
complex values (where real/imag are updated from dfReal/dfImag). Update the code
that currently computes newReal/newImag and the subsequent weighted blend to use
SmoothingFactor and (1 - SmoothingFactor) rather than 0.1/0.9, and ensure the
option/const has a brief XML comment describing its purpose and typical range.
In `@src/Audio/Enhancement/FullSubNetPlus.cs`:
- Around line 154-155: The EnhanceWithReference method currently discards the
reference parameter; update FullSubNetPlus.EnhanceWithReference to explicitly
document this behavior with an XML doc comment stating that FullSubNetPlus does
not use the reference signal and that the method delegates to Enhance(Tensor<T>
audio), and keep the implementation as "=> Enhance(audio);". Alternatively, if
you prefer to surface the difference, replace the body with a
NotSupportedException mentioning that reference-based enhancement is not
supported; ensure the XML doc on EnhanceWithReference and/or the exception
message clearly references that FullSubNetPlus ignores reference inputs.
- Line 77: The constructor for FullSubNetPlus currently always allocates an
AdamWOptimizer<T, Tensor<T>, Tensor<T>> into the field _optimizer even when
running in ONNX inference mode; remove this unnecessary allocation by only
creating the AdamWOptimizer when not in ONNX mode (or make _optimizer nullable
and set it to null for ONNX), and update Train() to check _optimizer for null
before using it (or throw a clear NotSupportedException if training is attempted
in ONNX mode). Locate uses of _optimizer, the constructor that calls new
AdamWOptimizer<T, Tensor<T>, Tensor<T>>(this), and the Train() method to
implement the conditional allocation and null-check.
- Around line 263-273: DeserializeNetworkSpecificData restores _options.FftSize
and _options.HopLength but leaves the readonly _stft initialized with the
original constructor values, causing mismatched STFT behavior; make the _stft
field mutable (remove readonly or add a backing property) and after reading and
assigning _options (inside DeserializeNetworkSpecificData) reassign _stft to a
new STFT instance using the same constructor call/parameters you use in the
class constructor (pass the restored _options.FftSize and _options.HopLength),
disposing the previous _stft if necessary to avoid resource leaks.
In `@src/Audio/Enhancement/MPSENet.cs`:
- Around line 218-227: The _stft field is readonly and thus keeps old
FFTSize/HopLength after DeserializeNetworkSpecificData updates _options; make
_stft mutable (remove readonly and adjust initialization) and, at the end of
DeserializeNetworkSpecificData in MPSENet.DeserializeNetworkSpecificData,
recreate/reinitialize _stft using the updated _options (e.g., new
STFT(_options.FFTSize, _options.HopLength, ...)) so the runtime STFT matches
deserialized parameters; ensure any dependent state is reinitialized
consistently and null-checked if needed.
- Line 42: The field _lastPhase is a shared mutable Tensor<T>? used by
PreprocessAudio and PostprocessOutput, causing a TOCTOU race when Enhance() is
called concurrently; replace the field with a ThreadLocal<Tensor<T>?> (e.g.,
ThreadLocal<Tensor<T>?> _lastPhaseLocal) and update all usages in
PreprocessAudio and PostprocessOutput to read/write _lastPhaseLocal.Value, or
alternatively change PreprocessAudio to return a tuple/wrapper that carries the
phase alongside magnitude and have PostprocessOutput accept that bundle instead;
ensure only one of these approaches is applied and update references to
_lastPhase across Enhance(), PreprocessAudio, and PostprocessOutput accordingly.
- Line 100: EnhancementStrength is declared on MPSENet but never applied; update
the Enhance() method to use it by blending the enhanced signal with the original
input (or remove the property). Specifically, after computing the enhanced
buffer in Enhance(), clamp EnhancementStrength to [0,1], compute finalSample =
EnhancementStrength * enhancedSample + (1 - EnhancementStrength) *
originalSample for each channel/sample, and return that mixed buffer;
alternatively delete the EnhancementStrength property and its references if you
opt to remove it. Ensure you reference EnhancementStrength and Enhance() in the
change and add a brief comment or unit test to document the expected behavior.
In `@src/Audio/Enhancement/SpikingFullSubNet.cs`:
- Around line 131-132: Add an XML documentation comment to EnhanceWithReference
explaining that the reference parameter is intentionally ignored because this
SpikingFullSubNet implementation does not support reference-based enhancement
and therefore delegates to Enhance(audio); mention expected behavior (reference
is unused) so callers aren't confused and reference the method name
EnhanceWithReference and the delegate target Enhance for clarity.
- Line 186: The forward pass is compressed into a single hard-to-read line; in
the SpikingFullSubNet class replace the single-line `var c = input; foreach (var
l in Layers) c = l.Forward(c); return c;` with a multiline, explicit
implementation inside the Forward method: declare a descriptive variable (e.g.,
`var current = input;`), use a `foreach (var layer in Layers) { current =
layer.Forward(current); }`, then `return current;` to improve readability and
make it easier to set breakpoints or add debug/logging around `Layers` and
`Forward` calls.
- Around line 201-206: The UpdateParameters method currently compresses the loop
into a dense one-liner which hurts readability; refactor the body of
UpdateParameters (keep the initial _useNativeMode check and exception) so the
foreach over Layers is expanded into a clear multi-line loop: for each layer l
read its int c = l.ParameterCount, call l.UpdateParameters(parameters.Slice(idx,
c)) on its own line, then increment idx by c on its own line; preserve existing
behavior and variable names (UpdateParameters, _useNativeMode, Layers,
ParameterCount, parameters, idx).
- Around line 225-245: The SerializeNetworkSpecificData and
DeserializeNetworkSpecificData methods are hard to read because they cram
multiple read/write calls on single lines; refactor both methods so each
BinaryWriter.Write and BinaryReader.Read call is on its own line (e.g., separate
writes for _useNativeMode, _options.ModelPath, _options.SampleRate, etc., and
separate reads for _useNativeMode, mp, SampleRate, FftSize, HopLength,
NumFreqBins, NumFullBandLayers, FullBandHiddenSize, NumSubBandLayers,
SubBandHiddenSize, SpikingThreshold, TimeConstant, EnhancementStrength,
DropoutRate), preserve the existing logic for assigning _options.ModelPath from
mp and the conditional creation of OnnxEncoder = new OnnxModel<T>(p,
_options.OnnxOptions) when !_useNativeMode so behavior does not change.
In `@src/Audio/Enhancement/TFGridNet.cs`:
- Around line 70-82: Constructor TFGridNet currently allocates an
AdamWOptimizer<T, Tensor<T>, Tensor<T>> even when running in ONNX/inference mode
(_useNativeMode = false); defer or avoid this allocation by making the field
_optimizer nullable and only instantiating it when training is actually
supported (e.g., during Train() or when _useNativeMode is true), or by moving
the new AdamWOptimizer call out of the TFGridNet constructor and into the
initialization path used for native/training mode; update any usage sites of
_optimizer to guard for null.
- Around line 152-154: EnhanceWithReference currently ignores its reference
parameter and simply calls Enhance(audio); either implement reference-aware
processing by wiring the reference through (add/extend an internal method like
Enhance(audio, reference) or modify Enhance to accept the reference and use it
in the TF-GridNet pipeline) or make the limitation explicit: update
EnhanceWithReference to throw a NotSupportedException with a clear message and
update the XML doc for EnhanceWithReference to state that TF-GridNet does not
support reference-based enhancement; reference the existing Enhance and
EnhanceWithReference methods when making the change.
- Around line 139-150: The code sets EnhancementStrength in Enhance but never
uses it, so varying strength has no effect; modify Enhance to blend between the
original input and the model output using EnhancementStrength before computing
the inverse STFT: after ComputeSTFT(stft) and after obtaining 'output' from
Predict(...) or OnnxEncoder.Run(...), compute a weighted mix =
EnhancementStrength * output + (1 - EnhancementStrength) * stft (or blend
magnitudes/phases as appropriate), then pass that blended tensor into
ComputeISTFT(blended, audio.Length); keep ThrowIfDisposed, ComputeSTFT,
Predict/OnnxEncoder.Run, ComputeISTFT and the EnhancementStrength property names
to find where to insert the blending.
---
Duplicate comments:
In `@src/Audio/Enhancement/BandSplitRNNEnhancer.cs`:
- Line 131: Implement the previously requested body for
BandSplitRNNEnhancer.EstimateNoiseProfile(Tensor<T> noiseOnlyAudio): validate
noiseOnlyAudio (null/length), compute the noise spectral/profile using the same
algorithm from the earlier fix (e.g., frame/window FFT or band energy averaging
used by the enhancer), store the result into the class-level noise profile
field/property (e.g., _noiseProfile or NoiseProfile) so subsequent processing
uses it, and ensure thread-safety and error handling (throw ArgumentException or
log and return) consistent with other methods in BandSplitRNNEnhancer.
In `@src/Audio/Enhancement/CMGAN.cs`:
- Line 174: The method EstimateNoiseProfile(Tensor<T> noiseOnlyAudio) in CMGAN
is an empty stub and must either implement noise estimation or explicitly fail;
update the CMGAN.EstimateNoiseProfile implementation to perform noise profile
estimation (e.g., compute and store noise statistics from the provided
noiseOnlyAudio tensor into the instance’s noise/profile fields) or, if the model
cannot support runtime noise estimation, throw a documented
NotSupportedException with a clear message indicating that runtime noise
profiling is not supported and referencing any alternative API (e.g., a
SetNoiseProfile method) so callers know how to proceed; ensure the method
updates internal state used by Enhance/denoise code paths or consistently throws
so callers won’t silently continue with an uninitialized profile.
In `@src/Audio/Enhancement/FRCRN.cs`:
- Around line 218-227: DeserializeNetworkSpecificData updates
_options.SampleRate but does not propagate it to base.SampleRate; after reading
and assigning _options.SampleRate in DeserializeNetworkSpecificData, set
base.SampleRate = _options.SampleRate so the base class preprocessing uses the
updated sample rate (locate this in the DeserializeNetworkSpecificData method
near the _options.SampleRate assignment).
- Around line 122-129: The three placeholder methods EnhanceWithReference,
ProcessChunk, and EstimateNoiseProfile currently ignore caller intent; make them
fail-fast by replacing their bodies to throw a clear exception (e.g.,
NotImplementedException or NotSupportedException) with a descriptive message
referencing the method name and suggesting use of Enhance (for
EnhanceWithReference/ProcessChunk) or that noise-profile support is not
implemented (for EstimateNoiseProfile), so callers get an immediate, actionable
failure instead of silent no-ops; update EnhanceWithReference to either call
Enhance(audio, reference) when real logic exists or throw noting reference-based
enhancement is unsupported, change ProcessChunk to either delegate to Enhance or
throw indicating chunked processing is unsupported, and change
EstimateNoiseProfile to throw indicating noise-profile estimation is not
implemented.
- Line 229: The CreateNewInstance implementation always returns a native
FRCRN<T>, which drops ONNX mode and model path; modify CreateNewInstance to
check the current instance's ONNX configuration (e.g., inspect _options.UseOnnx
or _options.ModelPath, or test this is an OnnxFRCRN) and return a matching
clone: if ONNX is enabled or a model path exists, return a new
OnnxFRCRN<T>(Architecture, _options) (preserving _options.ModelPath/UseOnnx),
otherwise return new FRCRN<T>(Architecture, _options).
In `@src/Audio/Enhancement/FullSubNetPlus.cs`:
- Line 184: The method EstimateNoiseProfile(Tensor<T> noiseOnlyAudio) is an
empty stub and must explicitly document that it's intentionally a no-op; update
the method body to include a clear XML documentation summary and an inline
comment stating that FullSubNetPlus learns noise end-to-end and therefore does
not require an explicit noise-profile estimate, and keep the method as a no-op
(or alternatively log/debug a single-line message) so callers see an intentional
design choice rather than an accidental omission; reference the
EstimateNoiseProfile method and the FullSubNetPlus class in your change.
In `@src/Audio/Enhancement/MPSENet.cs`:
- Line 123: EnhanceWithReference currently ignores the reference; update it to
compute a noise profile from the reference and pass that into the enhancement
pipeline instead of calling Enhance(audio) directly. Specifically, implement or
call an estimator (e.g., EstimateNoiseProfile(reference)) inside
EnhanceWithReference and then invoke the enhancement routine that accepts a
profile (e.g., Enhance(audio, noiseProfile)) — or add an overload of Enhance
that accepts the profile — so the reference guides noise estimation and is
actually used in processing.
- Line 229: CreateNewInstance currently always constructs a native MPSENet via
new MPSENet<T>(Architecture, _options), which drops ONNX mode and the ONNX model
path; update CreateNewInstance in class MPSENet<T> to detect and preserve ONNX
mode and path (e.g., check the instance's ONNX flag/field like _isOnnx or
_onnxModelPath) and, when ONNX, call the ONNX-aware constructor or pass the
saved model path and mode into the new MPSENet<T> so the cloned/serialized
instance keeps the same mode and model path instead of switching to native.
- Line 129: The EstimateNoiseProfile method in MPSENet is an empty stub and must
compute and persist a noise profile for later use; implement it to accept the
noiseOnlyAudio Tensor<T>, compute a representative noise statistic (e.g.,
frame-wise power spectral density or mean/std across frames) and store the
result in a private field (e.g., _noiseProfile or noiseProfileTensor) on the
MPSENet instance, ensuring the stored profile type matches what
Enhance/processing methods expect; update or reference the stored profile in the
enhancement path (e.g., Enhance or any NoiseReduction-related methods) so they
use this computed profile, and add minimal validation (null/shape checks) and
thread-safety if the profile may be accessed concurrently.
In `@src/Audio/Enhancement/SpikingFullSubNet.cs`:
- Line 161: The method EstimateNoiseProfile(Tensor<T> noiseOnlyAudio) is
currently an unexplained empty method; update it to clearly express intentional
no-op behavior: add an XML documentation comment above EstimateNoiseProfile
stating why it is intentionally empty (e.g., noise profiling is handled by the
SNN pipeline or elsewhere), mention the parameter Tensor<T> noiseOnlyAudio in
the doc, and optionally add a single-line debug/no-op statement (e.g.,
Debug.Assert or a comment) inside the method to make the intent explicit to
maintainers instead of leaving a bare {} block.
In `@src/Audio/Enhancement/TFGridNet.cs`:
- Around line 172-174: EstimateNoiseProfile currently has an empty body;
implement a real noise-profile estimator in
TFGridNet.EstimateNoiseProfile(Tensor<T> noiseOnlyAudio) by computing a
per-band/nightly average noise spectrum (e.g., perform STFT on noiseOnlyAudio,
compute magnitude-squared (PSD) per frequency bin, average across frames) and
store the result in a class field (e.g., _noiseProfile or noiseProfile:
Tensor<T>) that other methods (e.g., the enhancement pipeline) will use; if
implementing STFT/PSD is out of scope, replace the empty stub with an explicit
XML doc on EstimateNoiseProfile explaining the architectural reason for a no-op
and either throw NotSupportedException or call a clearly named no-op helper
(e.g., LogNoNoiseEstimation()) so the behavior is explicit and reviewed.
…ng in 7 enhancers - Add STFT-based EstimateNoiseProfile with spectral subtraction in Enhance - EnhanceWithReference now uses reference as noise profile before enhancing - EnhancementStrength blending applied in CMGAN and TFGridNet - CreateNewInstance preserves ONNX mode for CMGAN, TFGridNet, MPSENet, BandSplitRNNEnhancer, FRCRN Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unnecessary optimizer allocation from ONNX constructors - Make optimizer fields nullable with ?. operator usage - Fix CreateNewInstance to preserve ONNX mode (use modelPath constructor) - Persist modelPath in ONNX constructors to _options - Fix serialization data loss (FeedForwardDim, FeatureEncoderDim) - Fix triple audio processing in emotion models (extract ComputeArousal/ValenceFromProbs) - Remove EnhancementStrength overwrite on every Enhance() call - Add MatchThreshold property to fingerprinting options - Add FindMatches with windowed cosine similarity to fingerprinting models - Fix WavLM: fail-fast null encoder, layerIndex validation, SampleRate restore - Reconstruct STFT in deserialization for FullSubNetPlus and MPSENet - Replace silent classification fallback with InvalidOperationException - Fix CLAP int overflow and OnnxModel iteration - Add proper biquad peaking EQ in NeuralParametricEQ - Fix DAC GetBitrate to return kbps Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cation, langid, vad, foundation, source separation models Remove AdamWOptimizer allocation from ONNX constructors (inference-only mode), make _optimizer nullable, use null-conditional operator for UpdateParameters, and persist modelPath in options for all affected audio models. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nalysis, tts, whisper, fingerprinting, enhancement models Remove optimizer allocation from ONNX constructors in 57 audio model files. Optimizers are only used during training, not inference. Loss functions use industry-standard defaults (MSE for enhancement/TTS, cross-entropy for ASR) per research paper recommendations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 59
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
src/Audio/Enhancement/ConvTasNet.cs (9)
677-682:⚠️ Potential issue | 🟠 MajorBLOCKING: Method ignores required parameter - simplified implementation.
EnhanceWithReferencecompletely ignores thereferenceparameter and just delegates toEnhance. The comment acknowledges this limitation ("Conv-TasNet doesn't use reference signal"). This violates the interface contract.Either implement reference-based enhancement (e.g., for echo cancellation) or throw
NotSupportedExceptionto make the limitation explicit to callers.Proposed fix: Make unsupported operation explicit
public Tensor<T> EnhanceWithReference(Tensor<T> audio, Tensor<T> reference) { - // Conv-TasNet doesn't use reference signal - // For echo cancellation, a different model would be more appropriate - return Enhance(audio); + throw new NotSupportedException( + "Conv-TasNet does not support reference-based enhancement. " + + "For echo cancellation, use a model designed for AEC such as SpeexDSP or RNNoise."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 677 - 682, EnhanceWithReference currently ignores the reference parameter and calls Enhance, violating the interface contract; change EnhanceWithReference(Tensor<T> audio, Tensor<T> reference) to explicitly signal non-support by throwing a NotSupportedException with a clear message like "Conv-TasNet does not support reference-based enhancement" (or implement reference-aware enhancement if you choose), and remove the silent delegation to Enhance; reference the EnhanceWithReference method and the existing Enhance(Tensor<T> audio) call to locate and update the code.
744-748:⚠️ Potential issue | 🔴 CriticalBLOCKING: Stub implementation with empty body.
EstimateNoiseProfileis required byIAudioEnhancer<T>but has an empty body with only a comment. This is a stub. Per coding guidelines, all methods must have production-ready implementations.If Conv-TasNet architecturally cannot use noise profiles, throw
NotSupportedExceptionwith a clear message explaining why and suggesting alternatives.Proposed fix: Make unsupported operation explicit
public void EstimateNoiseProfile(Tensor<T> noiseOnlyAudio) { - // Conv-TasNet is trained end-to-end and doesn't use explicit noise profiles - // This could be extended to adapt the model for specific noise types + // Conv-TasNet operates end-to-end without explicit noise modeling. + // Noise adaptation requires fine-tuning on domain-specific data. + // For noise-profile-based enhancement, consider SpectralSubtractor or Wiener filter. }Or if the interface contract requires throwing:
public void EstimateNoiseProfile(Tensor<T> noiseOnlyAudio) { - // Conv-TasNet is trained end-to-end and doesn't use explicit noise profiles - // This could be extended to adapt the model for specific noise types + throw new NotSupportedException( + "Conv-TasNet is trained end-to-end and does not use explicit noise profiles. " + + "For noise-profile-based enhancement, use SpectralSubtractor or similar."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 744 - 748, EstimateNoiseProfile currently has an empty stub; replace the blank implementation in ConvTasNet.EstimateNoiseProfile(Tensor<T> noiseOnlyAudio) with a thrown NotSupportedException that explains Conv-TasNet is end-to-end and does not use explicit noise profiles and suggests alternatives (e.g., use a noise-profile-based enhancer or adapt training for specific noise types); ensure the exception message is clear and references the method and recommended alternatives so callers understand why it’s unsupported and what to use instead.
282-286:⚠️ Potential issue | 🟠 MajorBLOCKING: Empty method body is a stub.
InitializeLayers()has an empty body with only a comment explaining why layers aren't initialized. This is a stub implementation. Per coding guidelines, methods must have complete, production-ready implementations.If Conv-TasNet genuinely doesn't use the base class layer system, override should explicitly document this is intentional (not "handled manually" which implies future work) or throw
NotSupportedExceptionif calling this method is an error.Proposed fix: Make intent explicit
protected override void InitializeLayers() { - // Layers are handled manually for Conv-TasNet's specific architecture - // The encoder, TCN, and decoder don't map directly to standard layer types + // Conv-TasNet uses custom encoder/TCN/decoder architecture that doesn't + // map to the standard layer abstraction. All weights are initialized + // directly in the constructor. This override intentionally does nothing. }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 282 - 286, The override InitializeLayers() in ConvTasNet currently contains only a comment and must explicitly express intent; replace the empty stub with a clear failure or documentation—either implement layer setup if it should use the base-layer system, or (preferred) throw a NotSupportedException from InitializeLayers() with a descriptive message like "ConvTasNet manages layers manually; InitializeLayers is not supported" and add an XML doc comment on the ConvTasNet.InitializeLayers override explaining that layer initialization is intentionally handled elsewhere so callers fail fast if invoked; locate the method by name InitializeLayers in the ConvTasNet class to apply this change.
471-502:⚠️ Potential issue | 🔴 CriticalBLOCKING: Simplified implementation with identity projection.
The
BottleneckProjectmethod explicitly states "Identity-like projection for simplicity" (line 492). This is not a production-ready bottleneck projection - it's a placeholder that simply copies values diagonally rather than performing a proper learned linear transformation.A real bottleneck projection requires weights (
_bottleneckWeight) and performsoutput = input @ weight + bias.Proposed fix: Implement proper linear projection
+ // Add field at class level: + private T[] _bottleneckWeight; + private T[] _bottleneckBias; + private Tensor<T> BottleneckProject(Tensor<T> input) { int batchSize = input.Shape[0]; int numFrames = input.Shape[1]; int inputDim = input.Shape[2]; var projected = new T[batchSize * numFrames * _bottleneckDim]; - // Simple linear projection for (int b = 0; b < batchSize; b++) { for (int f = 0; f < numFrames; f++) { for (int d = 0; d < _bottleneckDim; d++) { - T sum = _numOps.Zero; - for (int i = 0; i < inputDim && i < _bottleneckDim; i++) + T sum = _bottleneckBias[d]; + for (int i = 0; i < inputDim; i++) { int inIdx = b * numFrames * inputDim + f * inputDim + i; - if (i == d && inIdx < input.Length) - { - sum = input.Data.Span[inIdx]; // Identity-like projection for simplicity - } + int wIdx = d * inputDim + i; + sum = _numOps.Add(sum, _numOps.Multiply(input.Data.Span[inIdx], _bottleneckWeight[wIdx])); } int outIdx = b * numFrames * _bottleneckDim + f * _bottleneckDim + d; projected[outIdx] = sum; } } } return new Tensor<T>(projected, new[] { batchSize, numFrames, _bottleneckDim }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 471 - 502, BottleneckProject currently uses an identity-like diagonal copy; replace it with a proper learned linear projection: for each output element compute sum += input[b,f,i] * _bottleneckWeight[i,d] over inputDim, add _bottleneckBias[d] (if present), and store into projected[outIdx]; ensure you reference the existing members _bottleneckDim, _bottleneckWeight and _bottleneckBias, use _numOps.Zero to initialize the accumulation, respect input.Data length/shape checks when indexing input.Data.Span and weight storage, and return the Tensor<T> with shape [batchSize, numFrames, _bottleneckDim].
1128-1143:⚠️ Potential issue | 🔴 CriticalBLOCKING: Deserialization discards all values - broken implementation.
DeserializeNetworkSpecificDatareads 11 values from the stream but assigns them all to discard (_). This means deserialization does not restore the model state. The only value actually used isEnhancementStrengthat the end.This breaks round-trip serialization - a serialized and deserialized model will not match the original.
Proposed fix: Validate deserialized values match current model
protected override void DeserializeNetworkSpecificData(BinaryReader reader) { - // Read configuration values for validation - _ = reader.ReadBoolean(); // IsOnnxMode - _ = reader.ReadInt32(); // SampleRate - _ = reader.ReadInt32(); // _encoderDim - _ = reader.ReadInt32(); // _kernelSize - _ = reader.ReadInt32(); // _stride - _ = reader.ReadInt32(); // _numSources - _ = reader.ReadInt32(); // _bottleneckDim - _ = reader.ReadInt32(); // _hiddenDim - _ = reader.ReadInt32(); // _numBlocks - _ = reader.ReadInt32(); // _numRepeats - _ = reader.ReadInt32(); // _tcnKernelSize + bool isOnnxMode = reader.ReadBoolean(); + int sampleRate = reader.ReadInt32(); + int encoderDim = reader.ReadInt32(); + int kernelSize = reader.ReadInt32(); + int stride = reader.ReadInt32(); + int numSources = reader.ReadInt32(); + int bottleneckDim = reader.ReadInt32(); + int hiddenDim = reader.ReadInt32(); + int numBlocks = reader.ReadInt32(); + int numRepeats = reader.ReadInt32(); + int tcnKernelSize = reader.ReadInt32(); + + if (isOnnxMode != IsOnnxMode || sampleRate != SampleRate || + encoderDim != _encoderDim || kernelSize != _kernelSize || + numSources != _numSources) + { + throw new InvalidOperationException( + "Serialized model configuration does not match current model."); + } + EnhancementStrength = reader.ReadDouble(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 1128 - 1143, DeserializeNetworkSpecificData currently discards all deserialized values (using _), so the stored network configuration isn't validated or restored; change it to read into local variables and validate them against the instance fields (or assign them when appropriate). Specifically, in DeserializeNetworkSpecificData read IsOnnxMode, sampleRate, and the integers into temporaries and compare each to the corresponding fields like _encoderDim, _kernelSize, _stride, _numSources, _bottleneckDim, _hiddenDim, _numBlocks, _numRepeats, _tcnKernelSize (and SampleRate) and throw an informative InvalidDataException if any mismatch is found; keep reading EnhancementStrength into EnhancementStrength as already done. Ensure the method name DeserializeNetworkSpecificData and the field names (_encoderDim, _kernelSize, _stride, _numSources, _bottleneckDim, _hiddenDim, _numBlocks, _numRepeats, _tcnKernelSize, EnhancementStrength) are used so the correct fields are validated or restored.
1146-1159:⚠️ Potential issue | 🟠 MajorBug:
CreateNewInstancedoes not preserve ONNX mode.When the original model is in ONNX mode (loaded from a
.onnxfile),CreateNewInstancecreates a native training instance instead. This breaks cloning/copying for ONNX models.The method should check
IsOnnxModeand call the appropriate constructor.Proposed fix: Preserve ONNX mode in clone
protected override IFullModel<T, Tensor<T>, Tensor<T>> CreateNewInstance() { + if (IsOnnxMode && OnnxModel != null) + { + // Cannot clone ONNX model without original path - return native equivalent + // Consider storing modelPath in _options to support proper cloning + throw new NotSupportedException( + "Cloning ONNX-loaded models is not supported. Store the model path in options for recreation."); + } + return new ConvTasNet<T>( Architecture, sampleRate: SampleRate, encoderDim: _encoderDim, kernelSize: _kernelSize, bottleneckDim: _bottleneckDim, hiddenDim: _hiddenDim, numBlocks: _numBlocks, numRepeats: _numRepeats, tcnKernelSize: _tcnKernelSize, - numSources: _numSources); + numSources: _numSources, + options: _options); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 1146 - 1159, CreateNewInstance currently always constructs a native ConvTasNet<T> which loses ONNX mode; update CreateNewInstance to check the instance's IsOnnxMode and, when true, construct the ONNX-backed clone (use the ConvTasNet<T> constructor that preserves/accepts ONNX mode or the one that accepts the ONNX model path/state) instead of the native constructor so cloned models retain ONNX behavior; touch CreateNewInstance and use IsOnnxMode and the ONNX-specific ConvTasNet<T> constructor/parameters to branch accordingly.
91-94:⚠️ Potential issue | 🔴 CriticalBLOCKING: Placeholder field reserved for future implementation.
_tcnStatesis declared with a pragma warning suppression and a comment stating it is "Reserved for future streaming implementation." Per coding guidelines, placeholders and future enhancement comments are blocking issues requiring immediate fix.Either implement streaming state properly or remove this field entirely.
Proposed fix: Remove placeholder field
-#pragma warning disable CS0414 // Reserved for future streaming implementation - private T[][]? _tcnStates; -#pragma warning restore CS0414🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 91 - 94, The field _tcnStates is a placeholder reserved for future streaming and is blocking per guidelines; remove the unused declaration and its surrounding pragma directives from the ConvTasNet class (delete the lines declaring _tcnStates and the `#pragma` warning disable/restore CS0414), and also remove any dead references to _tcnStates elsewhere in ConvTasNet (e.g., initialization, Reset/Dispose, or processing helpers) so there are no leftover compile or logical dependencies; if streaming is to be implemented now instead, replace the placeholder with a proper streaming state implementation (add explicit state initialization, Clear/Reset methods and state updates used by the processing methods) rather than suppressing warnings.
852-878:⚠️ Potential issue | 🔴 CriticalBLOCKING: Simplified gradient computation that only updates decoder.
The
ComputeGradientsmethod is explicitly labeled "Simplified gradient computation" and only populates gradients for the decoder weights (lines 871-875). The encoder and mask gradient arrays remain zero-initialized and are never computed. This is not a production-ready backpropagation implementation.A proper implementation requires:
- Full chain rule through decoder → mask estimation → TCN → bottleneck → encoder
- Gradient accumulation for all weight tensors
- Proper loss gradient computation (SI-SNR gradient, not just
pred - targ)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 852 - 878, ComputeGradients currently only updates decoder gradients and uses a naive pred - targ; replace this with a full backpropagation pass that computes the SI-SNR loss gradient w.r.t. the network output, then backpropagates through the mask estimation, temporal convolutional network (TCN), bottleneck, and encoder to accumulate gradients into the encoder, mask, and decoder arrays (identified by _encoderWeight, _maskWeight, _decoderWeight) using the library numeric ops (_numOps) and Tensor<T> inputs (predicted, target); implement chain-rule updates for each layer/module, convert SI-SNR derivative to per-sample output gradients, propagate those gradients through mask multiplication and each TCN/bottleneck layer, and add the resulting parameter gradients into the corresponding entries of gradients["encoder"], gradients["mask"], and gradients["decoder"] instead of leaving encoder/mask zero-initialized.
308-321:⚠️ Potential issue | 🟡 MinorSimplified implementation: Enhancement strength blending is incomplete.
The comment at line 313-314 explicitly states "Blend enhanced with original would require original signal, For now, just scale the output." This is a simplified implementation that doesn't match the expected behavior of
EnhancementStrength(which should blend between original and enhanced, not just scale).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Enhancement/ConvTasNet.cs` around lines 308 - 321, The current branch in ConvTasNet that checks EnhancementStrength only scales the enhanced output instead of blending with the original; update the implementation to perform a proper blend using EnhancementStrength and its inverse: locate the block where EnhancementStrength, strengthT and invStrength are computed (symbols: EnhancementStrength, _numOps, strengthT, invStrength, modelOutput) and replace the scaling loop with an elementwise blend that computes result[i] = _numOps.Add(_numOps.Multiply(enhancedSample, strengthT), _numOps.Multiply(originalSample, invStrength)); ensure you read original samples from the original input tensor (matching shapes/lengths), use the same Tensor<T> construction for the result, and remove the temporary comment about skipping blend.src/Audio/Fingerprinting/PANNsModel.cs (1)
676-815:⚠️ Potential issue | 🔴 CriticalBlocking: training path is stubbed and gradients are placeholder-only.
TrainthrowsNotImplementedException, andUpdateWeightsuses a placeholder gradient (“assume unit embedding”) plus skips CNN gradients. This is not production‑ready and can silently produce invalid training results. Implement at least a head‑only fine‑tuning path with real gradients, or remove training from the public surface.✅ Minimal head‑only fine‑tuning implementation (concrete fix)
public override void Train(Tensor<T> input, Tensor<T> expected) { if (IsOnnxMode) { throw new InvalidOperationException("Cannot train in ONNX inference mode."); } - - // Full training from scratch is not yet implemented. - // ... - throw new NotImplementedException( - "Full PANNs training from scratch is not yet implemented. " + - "Use ONNX mode with pre-trained weights for inference, or " + - "implement transfer learning by fine-tuning the classification head."); + var embedding = ExtractEmbedding(input); + var logits = ComputeLogits(embedding); + _ = ComputeBCELoss(logits, expected); + UpdateHeadWeights(embedding, expected); } -private void UpdateWeights(Tensor<T> predicted, Tensor<T> target) +private void UpdateHeadWeights(Tensor<T> embedding, Tensor<T> target) { - // Compute output gradients (dL/dlogits for BCE loss with sigmoid) - int numClasses = Math.Min(predicted.Length, target.Length); - var outputGrad = new double[numClasses]; - for (int i = 0; i < numClasses; i++) - { - double p = Sigmoid(_numOps.ToDouble(predicted.Data.Span[i])); - double t = _numOps.ToDouble(target.Data.Span[i]); - outputGrad[i] = (p - t) / numClasses; // dL/dz = sigmoid(z) - target - } - ... - // Simplified: assume unit embedding for gradient computation - ... - if (_optimizer is null) throw new InvalidOperationException("Optimizer not initialized. Use the training constructor."); - var updatedParams = _optimizer.UpdateParameters(paramVector, gradientVector); + if (_optimizer is null) throw new InvalidOperationException("Optimizer not initialized. Use the training constructor."); + + int batchSize = embedding.Shape[0]; + var logits = ComputeLogits(embedding); + var fcWeightGrad = new double[_fcWeight.Length]; + var fcBiasGrad = new double[_fcBias.Length]; + + for (int b = 0; b < batchSize; b++) + { + for (int c = 0; c < _numClasses && c < _fcBias.Length; c++) + { + int logIdx = b * _numClasses + c; + if (logIdx >= logits.Length || logIdx >= target.Length) continue; + + double p = Sigmoid(_numOps.ToDouble(logits.Data.Span[logIdx])); + double t = _numOps.ToDouble(target.Data.Span[logIdx]); + double g = (p - t) / batchSize; + fcBiasGrad[c] += g; + + for (int e = 0; e < _embeddingDim; e++) + { + int embIdx = b * _embeddingDim + e; + int wIdx = c * _embeddingDim + e; + if (embIdx < embedding.Length && wIdx < fcWeightGrad.Length) + { + fcWeightGrad[wIdx] += g * _numOps.ToDouble(embedding.Data.Span[embIdx]); + } + } + } + } + + var gradients = new List<T>(); + foreach (var g in fcWeightGrad) gradients.Add(_numOps.FromDouble(g)); + foreach (var g in fcBiasGrad) gradients.Add(_numOps.FromDouble(g)); + + var parameters = new List<T>(); + parameters.AddRange(_fcWeight); + parameters.AddRange(_fcBias); + + var updatedParams = _optimizer.UpdateParameters( + new Vector<T>(parameters.ToArray()), + new Vector<T>(gradients.ToArray())); + + int idx = 0; + for (int i = 0; i < _fcWeight.Length && idx < updatedParams.Length; i++, idx++) + { + _fcWeight[i] = updatedParams[idx]; + } + for (int i = 0; i < _fcBias.Length && idx < updatedParams.Length; i++, idx++) + { + _fcBias[i] = updatedParams[idx]; + } }This keeps the feature extractor frozen while providing a real, production‑safe training path for the head. If full fine‑tuning is required, extend with cached activations + ConvBlock backprop.
As per coding guidelines: "Production Readiness (CRITICAL - Flag as BLOCKING) — Stubs/Placeholders ... Simplified implementations ... are blocking issues requiring immediate fix."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Fingerprinting/PANNsModel.cs` around lines 676 - 815, Train currently throws NotImplementedException and UpdateWeights uses placeholder gradients; implement a proper head‑only fine‑tuning path: modify Train to accept/run a head‑only mode (no ConvBlock updates), have the forward pass cache the embedding vector into a private field (e.g., _cachedEmbedding) during inference so UpdateWeights can use the real embedding values, replace the "assume unit embedding" logic in UpdateWeights by computing fcWeightGrad using _cachedEmbedding (dL/d_fcWeight[e,c] = outputGrad[c] * cachedEmbedding[e]) and fcBiasGrad = outputGrad, compute dL/d_embedding from fcWeight and outputGrad only if the embedding head is trainable, keep all CNN/ConvBlock weights frozen, validate _optimizer is initialized and throw a clear error if not, assemble gradients only for the head parameters (_fcWeight, _fcBias, and optionally embedding head weights) and feed them to _optimizer.UpdateParameters, and remove placeholder comments so the public Train method provides a real, safe head‑only training path.
- Validate modelPath for ONNX constructors (AudioSep, CLAP, CRNNEventDetector, HTSAT) - Validate streaming sampleRate (AST, BEATs, PANNs) - Persist FMin/FMax in serialization/deserialization (AST, AudioMAE, EAT, PANNs) - Preserve ONNX mode in CreateNewInstance (AudioMAE, AudioLDMClassifier, FDYSED, PANNs, Data2Vec2, HuBERT, MERT, Wav2Vec2, SpikingFullSubNet) - Validate layerIndex bounds (HuBERT, MERT, Data2Vec2, Wav2Vec2) - Apply EnhancementStrength in enhancement pipeline (SpikingFullSubNet, FullSubNetPlus, BandSplitRNNEnhancer, TFGridNet) - Use noise profile for spectral subtraction (SpikingFullSubNet, FullSubNetPlus, TFGridNet) - Rebuild STFT after deserialization (CMGAN, FRCRN) - Propagate SampleRate after deserialization (MPSENet) - Remove unused _optimizer field (ConvTasNet) - Fix softmax numerical stability (WavLMSER) - Make WavLMSER internal - Validate NumClasses/EmotionLabels and window/hop/sample-rate (WavLMSER) - Guard minMatchLength (GraFPrint, PeakNetFP) - Align FrameCount with embedding frames (ConformerFP, GraFPrint, NeuralFP, PeakNetFP) - Clamp event endTime to audio duration (AudioLDMClassifier) - Fix AudioSuperResolution blending for upsampled audio - Improve DAC RVQ quantization mapping and throw on missing ONNX decoder - Document NeuralParametricEQ reference behavior - Rehydrate optimizer after deserialization (PANNsModel) - Guard null optimizer in training (AudioMAE) - Expand condensed MergeEvents algorithm (AudioMAE) - Throw on STFT/mask length mismatch (FullSubNetPlus) - Remove redundant comments (ConvTasNet, PANNsModel) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 202 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 56
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Audio/Fingerprinting/PANNsModel.cs (2)
674-694:⚠️ Potential issue | 🔴 CriticalBLOCKING: training path is stubbed and uses placeholder gradients.
TrainthrowsNotImplementedException, andUpdateWeightsuses simplified/placeholder gradient logic. This is explicitly non‑production. Either implement full backprop/fine‑tuning or make the model inference‑only and remove placeholder training code.Minimal inference‑only fix (until full training is implemented)
public override void Train(Tensor<T> input, Tensor<T> expected) { if (IsOnnxMode) { throw new InvalidOperationException("Cannot train in ONNX inference mode."); } - - // Full training from scratch is not yet implemented. - // ... - throw new NotImplementedException( - "Full PANNs training from scratch is not yet implemented. " + - "Use ONNX mode with pre-trained weights for inference, or " + - "implement transfer learning by fine-tuning the classification head."); + throw new NotSupportedException( + "PANNsModel native training is not supported in this build. " + + "Use ONNX inference or implement full backprop/fine-tuning."); } private void UpdateWeights(Tensor<T> predicted, Tensor<T> target) { - // placeholder gradient logic - ... - if (_optimizer is null) throw new InvalidOperationException("Optimizer not initialized. Use the training constructor."); - var updatedParams = _optimizer.UpdateParameters(paramVector, gradientVector); - ... + throw new NotSupportedException( + "Training updates are not supported until full backprop is implemented."); }As per coding guidelines: “Stubs/Placeholders: Methods with
throw new NotImplementedException()…” and “Simplified implementations… hardcoded values instead of proper logic…”Also applies to: 714-814
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Fingerprinting/PANNsModel.cs` around lines 674 - 694, The Train method and UpdateWeights currently contain stubbed/placeholder training logic; make this class strictly inference-only until full backprop/finetune is implemented by (1) keeping Train guarded by IsOnnxMode and throwing a clear NotSupportedException/InvalidOperationException (e.g., in Train) to indicate training is not supported, (2) remove or replace any simplified/placeholder logic inside UpdateWeights in PANNsModel so it also throws NotSupportedException (or is removed) instead of using hardcoded gradients, and (3) audit any callers of UpdateWeights/Train to ensure they handle the NotSupportedException or never call training functions while using the ONNX inference flow (reference symbols: Train, UpdateWeights, PANNsModel, IsOnnxMode).
634-657:⚠️ Potential issue | 🔴 CriticalBLOCKING:
FindMatchesis a placeholder (hardcoded threshold, ignoresminMatchLength).This is simplified and not production‑ready. Implement real matching or at least honor
minMatchLengthand use a configurable threshold (e.g., viaPANNsModelOptions).Suggested fix (align with other fingerprinting models)
public IReadOnlyList<FingerprintMatch> FindMatches( AudioFingerprint<T> query, AudioFingerprint<T> reference, int minMatchLength = 10) { - double similarity = ComputeSimilarity(query, reference); - - if (similarity > 0.7) - { - return new List<FingerprintMatch> - { - new FingerprintMatch - { - QueryStartTime = 0, - ReferenceStartTime = 0, - Duration = Math.Min(query.Duration, reference.Duration), - Confidence = similarity, - MatchCount = 1 - } - }; - } - - return new List<FingerprintMatch>(); + if (minMatchLength <= 0) + throw new ArgumentOutOfRangeException(nameof(minMatchLength)); + + var matches = new List<FingerprintMatch>(); + int embDim = Math.Max(1, _embeddingDim); + int queryFrames = query.Data.Length / embDim; + int refFrames = reference.Data.Length / embDim; + if (queryFrames < minMatchLength || refFrames < minMatchLength) return matches; + + double threshold = _options.MatchThreshold; // add to PANNsModelOptions if missing + for (int rStart = 0; rStart <= refFrames - Math.Min(queryFrames, minMatchLength); rStart++) + { + int matchLen = Math.Min(queryFrames, refFrames - rStart); + double sim = 0, normQ = 0, normR = 0; + for (int f = 0; f < matchLen; f++) + for (int d = 0; d < embDim && + (f * embDim + d) < query.Data.Length && + ((rStart + f) * embDim + d) < reference.Data.Length; d++) + { + double q = _numOps.ToDouble(query.Data[f * embDim + d]); + double r = _numOps.ToDouble(reference.Data[(rStart + f) * embDim + d]); + sim += q * r; normQ += q * q; normR += r * r; + } + double denom = Math.Sqrt(normQ) * Math.Sqrt(normR); + double cosSim = denom > 1e-8 ? sim / denom : 0; + if (cosSim >= threshold && matchLen >= minMatchLength) + { + double timePerFrame = query.Duration / Math.Max(1, queryFrames); + matches.Add(new FingerprintMatch + { + QueryStartTime = 0, + ReferenceStartTime = rStart * timePerFrame, + Duration = matchLen * timePerFrame, + Confidence = cosSim, + MatchCount = matchLen + }); + rStart += matchLen - 1; + } + } + return matches; }As per coding guidelines: “Simplified implementations: Code that takes shortcuts like hardcoded values instead of proper logic…”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Audio/Fingerprinting/PANNsModel.cs` around lines 634 - 657, The current FindMatches method in PANNsModel.cs is a placeholder that uses a hardcoded similarity threshold and ignores the minMatchLength parameter; update the FindMatches(AudioFingerprint<T> query, AudioFingerprint<T> reference, int minMatchLength) implementation to (1) read a configurable similarity threshold from PANNsModelOptions instead of the hardcoded 0.7, (2) require that any reported match has at least minMatchLength frames/samples (use query.Duration/Reference.Duration or their frame counts to determine duration/length), and (3) produce matches by scanning/aligning the fingerprints (or at minimum by validating overall similarity per-segment) so Duration and MatchCount reflect the actual matched length; ensure you reference and use the existing ComputeSimilarity method only as part of this logic and update the returned FingerprintMatch fields (QueryStartTime, ReferenceStartTime, Duration, Confidence, MatchCount) accordingly.
…files - Add modelPath validation (null/whitespace + File.Exists) in ONNX constructors for AST, BEATs, PANNs, EAT, AudioSuperResolution, Data2Vec2, HuBERT, ConformerFP, GraFPrint, NeuralFP - Add sampleRate validation in streaming sessions for AudioMAE, AudioSep, CRNNEventDetector, HTSAT, CLAP, AudioLDMClassifier - Add FMin/FMax serialization/deserialization for AudioSep, CRNNEventDetector, HTSAT, CLAP - Fix CreateNewInstance to preserve ONNX mode for AudioSep, CRNNEventDetector, HTSAT, CLAP - Wire EnhancementStrength property to options with clamping for FullSubNetPlus, SpikingFullSubNet, TFGridNet - Apply spectral subtraction using noise profile for BandSplitRNNEnhancer, MPSENet, CMGAN - Add optimizer null check in Train for AudioLDMClassifier, Data2Vec2, HuBERT, MERT - Replace magic numbers with named constants in FDYSED - Clamp center frequency to 95% Nyquist in NeuralParametricEQ - Add case-insensitive emotion label lookup in WavLMSER - Make GraFPrint and NeuralFP internal - Add XML docs for MERT pass-through methods - Expand dense one-liners to multi-line in BandSplitRNNEnhancer, SpikingFullSubNet, MERT Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Require exact layerWeights length match with transformer layer count and normalize weights to sum to 1.0 in HuBERT, Data2Vec2, and MERT foundation models. Prevents partial weights from producing unpredictable scale. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 80 out of 202 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Feb 17, 2026
6 tasks done
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BEATs<T>) - complete implementation of the BEATs (Audio Pre-Training with Audio Tokenizers) model for audio event detection, following the golden example pattern established by AudioEventDetectorAiModelBuilder.Audio.cs) - extension methods enabling audio event detection through the AiModelBuilder facade pattern (DetectAudioEvents, GetAudioEventProbabilities, StartAudioEventStreaming, etc.)BEATs Model Details
IAudioEventDetector<T>implementation including streaming sessionsCreateDefaultBEATsLayers()for BEATs_iter3 paper architectureFacade Pattern
ConfigureModel()(since they implement IFullModel via inheritance chain)ConfiguredModeltoIAudioEventDetector<T>for domain operationsCreateAsync) made internal to enforce facade accessCloses #396 (partial - golden example model)
Test plan
ConfigureModel()and audio extension methods work🤖 Generated with Claude Code
Summary by CodeRabbit