feat(metrics): add comprehensive evaluation metrics for image/audio/video - #594
Conversation
- Add IMultimodalEmbedding<T> interface for text/image encoding - Create ClipNeuralNetwork<T> extending NeuralNetworkBase - Add IServableMultimodalModel<T> interface for REST API serving - Create EmbeddingsController with endpoints for text/image embeddings - Add ServableClipModel<T> adapter connecting CLIP to serving - Update ModelRepository with multimodal model support - Add multimodal properties to ModelInfo and ModelEntry 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ideo Adds exhaustive evaluation metrics for multimodal AI outputs: Image Quality Metrics: - KernelInceptionDistance (KID) - unbiased alternative to FID using MMD - CLIPScore and AestheticScore - text-image alignment evaluation Audio Quality Metrics: - WordErrorRate (WER) and CharacterErrorRate (CER) - transcription accuracy - ShortTimeObjectiveIntelligibility (STOI) - speech intelligibility - ScaleInvariantSignalToDistortionRatio (SI-SDR) - audio source separation - SignalToNoiseRatio (SNR) - audio quality measurement - PerceptualSpeechQuality (PESQ) - perceptual speech quality estimation Video Quality Metrics: - FrechetVideoDistance (FVD) - temporal coherence evaluation - VideoPSNR and VideoSSIM - per-frame quality metrics - TemporalConsistency - frame-to-frame coherence - VideoQualityIndex - comprehensive video quality assessment Closes #281 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@ooples has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 20 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request introduces comprehensive multimodal embedding infrastructure, adds standardized evaluation metrics for audio and video domains, and refactors CLIP-based neural networks to expose a unified multimodal API across six model implementations. Key changes include new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive evaluation metrics for multimodal AI outputs, covering image quality (KID, CLIPScore, AestheticScore), audio quality (WER, CER, STOI, SI-SDR, SNR, PESQ), and video quality (FVD, VideoPSNR, VideoSSIM, TemporalConsistency, VideoQualityIndex). The implementation includes extensive unit tests and serving infrastructure for multimodal models, particularly CLIP.
Key Changes:
- Implements 20+ evaluation metrics for image, audio, and video modalities
- Adds CLIP neural network implementation with multimodal embedding support
- Extends serving infrastructure with embeddings controller and multimodal model support
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/AiDotNet.Tests/UnitTests/Metrics/EvaluationMetricsTests.cs | Comprehensive test suite covering audio (WER, CER, STOI, SI-SDR, SNR), image (PSNR, SSIM, mIoU), and video (VPSNR, VSSIM, TemporalConsistency) metrics |
| src/NeuralNetworks/ClipNeuralNetwork.cs | CLIP model implementation with text/image encoding and zero-shot classification |
| src/Metrics/VideoQualityMetrics.cs | Video quality metrics including VPSNR, VSSIM, temporal consistency, and flicker detection |
| src/Metrics/KernelInceptionDistance.cs | KID metric for image generation quality using MMD with polynomial kernel |
| src/Metrics/FrechetVideoDistance.cs | FVD metric for video generation quality with multiple frame sampling strategies |
| src/Metrics/CLIPScore.cs | Text-image alignment evaluation and aesthetic scoring using CLIP embeddings |
| src/Metrics/AudioMetrics.cs | Audio quality metrics including WER, CER, STOI, SI-SDR, SNR, and PESQ approximation |
| src/Interfaces/IMultimodalEmbedding.cs | Interface defining multimodal embedding capabilities for text and images |
| src/AiDotNet.Serving/* | Extensions to serving infrastructure for multimodal model support |
💡 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}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/Metrics/CLIPScore.cs (1)
369-388: The hardcoded aesthetic prompts were already noted in a previous review. Skipping duplicate comment.
🧹 Nitpick comments (6)
src/AiDotNet.Serving/Models/IServableMultimodalModel.cs (1)
138-162: Consider movingModalityenum to its own file.The project follows a one-type-per-file convention per the PR objectives. While the enum is closely related to the interface, separating it would maintain consistency.
src/AiDotNet.Serving/Services/ModelRepository.cs (1)
244-261: Consider addingIsMultimodalcheck before casting.The method returns
nullvia cast failure if the model doesn't implementIServableMultimodalModel<T>, but it might be clearer and slightly more efficient to checkentry.IsMultimodalfirst, similar to howGetModelvalidatesNumericType.🔎 Proposed improvement
public IServableMultimodalModel<T>? GetMultimodalModel<T>(string name) { if (!_models.TryGetValue(name, out var entry)) { return null; } + // Early return if not a multimodal model + if (!entry.IsMultimodal) + { + return null; + } + // Verify the numeric type matches var expectedType = GetNumericType(typeof(T)); if (entry.NumericType != expectedType) { throw new InvalidOperationException( $"Model '{name}' uses numeric type '{entry.NumericType}' but was requested with type '{expectedType}'"); } // Try to cast to multimodal model return entry.Model as IServableMultimodalModel<T>; }src/AiDotNet.Serving/Models/ServableClipModel.cs (1)
121-155: Consider adding input validation to delegation methods.The
EncodeText,EncodeTextBatch,EncodeImage,EncodeImageBatch,ComputeSimilarity, andZeroShotClassifymethods directly delegate to_clipModelwithout validating inputs. While the underlying model may handle validation, adding null checks here would provide consistent error messages at the API boundary and fail faster.🔎 Example for EncodeText
public Vector<T> EncodeText(string text) { + if (string.IsNullOrWhiteSpace(text)) + { + throw new ArgumentException("Text cannot be null or empty", nameof(text)); + } return _clipModel.EncodeText(text); }src/Metrics/VideoQualityMetrics.cs (1)
418-421: Magic number in BilinearSample clamping.The value
1.001inMath.Min(height - 1.001, h)is unclear. This appears to prevent edge-case floating point issues, but a named constant or comment would clarify intent.+// Small epsilon to ensure h0+1 doesn't exceed bounds due to floating-point precision +const double epsilon = 0.001; // Clamp to valid range -h = Math.Max(0, Math.Min(height - 1.001, h)); -w = Math.Max(0, Math.Min(width - 1.001, w)); +h = Math.Max(0, Math.Min(height - 1 - epsilon, h)); +w = Math.Max(0, Math.Min(width - 1 - epsilon, w));src/Metrics/KernelInceptionDistance.cs (1)
307-337: Subset sampling uses rejection sampling - potential performance issue for large subsets.When
subsetSizeapproachesn, the rejection sampling loop may spin many times to find unused indices. Consider using Fisher-Yates shuffle for O(n) guaranteed performance.🔎 Fisher-Yates alternative
private Matrix<T> SampleSubset(Matrix<T> features, int subsetSize, Random random) { int n = features.Rows; int d = features.Columns; - // Generate random indices - var indices = new int[subsetSize]; - var used = new bool[n]; - for (int i = 0; i < subsetSize; i++) - { - int idx; - do - { - idx = random.Next(n); - } while (used[idx]); - used[idx] = true; - indices[i] = idx; - } + // Fisher-Yates partial shuffle for O(subsetSize) selection + var indices = Enumerable.Range(0, n).ToArray(); + for (int i = 0; i < subsetSize; i++) + { + int j = random.Next(i, n); + (indices[i], indices[j]) = (indices[j], indices[i]); + } // Create subset matrix var subset = new Matrix<T>(subsetSize, d);src/NeuralNetworks/ClipNeuralNetwork.cs (1)
419-480: Placeholder embedding generators will not produce meaningful CLIP embeddings.
GenerateTextEmbeddingandGenerateImageEmbeddinguse hash-based and statistics-based placeholder logic instead of actual ONNX inference. These will not provide semantic embeddings - text/image pairs will not have meaningful similarity scores. Ensure callers understand these are stubs awaiting real implementation.Consider adding a warning log or making this limitation more visible in the public API documentation. Would you like me to help draft documentation or logging statements?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/AiDotNet.Serving/Controllers/EmbeddingsController.cssrc/AiDotNet.Serving/Models/IServableMultimodalModel.cssrc/AiDotNet.Serving/Models/ModelInfo.cssrc/AiDotNet.Serving/Models/ServableClipModel.cssrc/AiDotNet.Serving/Services/IModelRepository.cssrc/AiDotNet.Serving/Services/ModelEntry.cssrc/AiDotNet.Serving/Services/ModelRepository.cssrc/Interfaces/IMultimodalEmbedding.cssrc/Metrics/AudioMetrics.cssrc/Metrics/CLIPScore.cssrc/Metrics/FrechetVideoDistance.cssrc/Metrics/KernelInceptionDistance.cssrc/Metrics/VideoQualityMetrics.cssrc/NeuralNetworks/ClipNeuralNetwork.cstests/AiDotNet.Tests/UnitTests/Metrics/EvaluationMetricsTests.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/AiDotNet.Serving/Services/ModelEntry.cssrc/AiDotNet.Serving/Models/ModelInfo.cssrc/AiDotNet.Serving/Models/IServableMultimodalModel.cssrc/Interfaces/IMultimodalEmbedding.cssrc/AiDotNet.Serving/Services/IModelRepository.cssrc/AiDotNet.Serving/Controllers/EmbeddingsController.cssrc/AiDotNet.Serving/Models/ServableClipModel.cssrc/Metrics/CLIPScore.cssrc/Metrics/FrechetVideoDistance.cssrc/AiDotNet.Serving/Services/ModelRepository.cssrc/Metrics/VideoQualityMetrics.cssrc/NeuralNetworks/ClipNeuralNetwork.cstests/AiDotNet.Tests/UnitTests/Metrics/EvaluationMetricsTests.cssrc/Metrics/KernelInceptionDistance.cssrc/Metrics/AudioMetrics.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/AiDotNet.Serving/Services/ModelEntry.cssrc/AiDotNet.Serving/Models/ModelInfo.cssrc/AiDotNet.Serving/Models/IServableMultimodalModel.cssrc/Interfaces/IMultimodalEmbedding.cssrc/AiDotNet.Serving/Services/IModelRepository.cssrc/AiDotNet.Serving/Controllers/EmbeddingsController.cssrc/AiDotNet.Serving/Models/ServableClipModel.cssrc/Metrics/CLIPScore.cssrc/Metrics/FrechetVideoDistance.cssrc/AiDotNet.Serving/Services/ModelRepository.cssrc/Metrics/VideoQualityMetrics.cssrc/NeuralNetworks/ClipNeuralNetwork.cstests/AiDotNet.Tests/UnitTests/Metrics/EvaluationMetricsTests.cssrc/Metrics/KernelInceptionDistance.cssrc/Metrics/AudioMetrics.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (Windows)
- GitHub Check: CodeQL Analysis
🔇 Additional comments (39)
src/AiDotNet.Serving/Services/ModelEntry.cs (1)
17-21: LGTM!Clean addition of the
IsMultimodalproperty following the existing property conventions in the class. The XML documentation clearly describes the purpose.src/AiDotNet.Serving/Models/ModelInfo.cs (1)
59-78: LGTM!The multimodal properties are well-structured with appropriate nullability for optional metadata. The XML documentation clearly explains each property's purpose and its relevance to multimodal models.
src/AiDotNet.Serving/Services/IModelRepository.cs (1)
71-88: LGTM!The new multimodal methods follow the established API patterns of the interface. The documentation is clear and consistent with the existing methods.
src/AiDotNet.Serving/Controllers/EmbeddingsController.cs (7)
28-34: LGTM!Constructor properly validates dependencies with null checks and follows standard DI patterns.
51-118: LGTM!The endpoint has proper input validation, model capability checks, and comprehensive error handling. The switch statement covers the supported numeric types with an appropriate fallback for unsupported types.
135-202: LGTM!Consistent implementation pattern with
EncodeText. Proper validation and error handling.
219-293: LGTM!Proper validation for both embedding arrays and consistent error handling pattern.
397-417: LGTM!Clean implementation with appropriate 404 handling for missing models.
419-524: LGTM!Helper methods are well-structured with clear responsibilities. The type conversion utilities properly handle the generic numeric types.
532-743: LGTM!DTOs are well-documented with appropriate property defaults and nullable types for optional fields. Grouping related DTOs in a single file with regions is acceptable for controller-specific types.
src/AiDotNet.Serving/Models/IServableMultimodalModel.cs (1)
1-136: LGTM!The interface is well-designed with comprehensive documentation. The remarks sections with beginner explanations are particularly helpful for API consumers.
src/Interfaces/IMultimodalEmbedding.cs (1)
1-81: LGTM!Clean interface design with appropriate separation from the serving layer. The documentation clearly explains the purpose and usage patterns for multimodal embeddings.
src/AiDotNet.Serving/Services/ModelRepository.cs (2)
135-162: LGTM!The multimodal property extraction logic is properly guarded by the
IsMultimodalcheck, avoiding unnecessary reflection for non-multimodal models.
206-236: LGTM!The
LoadMultimodalModelimplementation correctly setsIsMultimodal = trueand follows the established validation patterns.tests/AiDotNet.Tests/UnitTests/Metrics/EvaluationMetricsTests.cs (6)
15-106: LGTM!Comprehensive WER tests covering identical strings, complete differences, substitutions, empty inputs, and detailed output. The batch computation test correctly validates the averaging behavior.
123-168: LGTM!Good CER test coverage including the
ignoreWhitespaceoption validation.
170-251: LGTM!Audio signal processing metric tests (STOI, SI-SDR, SNR) appropriately use range assertions given the nature of these metrics. The deterministic random seed ensures reproducibility.
257-387: LGTM!Image metrics tests properly validate PSNR, SSIM, and MeanIoU with both identical and different inputs. The expected value assertions (100.0 for identical PSNR, 1.0 for identical SSIM/mIoU) align with metric definitions.
392-548: LGTM!Video metrics tests comprehensively cover per-frame analysis, temporal consistency, and the composite VideoQualityIndex. The smooth vs. flickering video tests effectively validate the temporal consistency metric behavior.
554-613: LGTM!Edge case tests properly validate that the metrics throw
ArgumentExceptionfor mismatched inputs. This ensures robust error handling in the metric implementations.src/AiDotNet.Serving/Models/ServableClipModel.cs (2)
35-39: LGTM! Constructor properly validates required dependencies.The null checks with descriptive
ArgumentNullExceptionmessages follow best practices for fail-fast initialization.
75-91: LGTM! The Predict method implementation is correct.Input validation is present, and the conversion from generic
Vector<T>todouble[]vianumOps.ToDoubleis appropriate for the CLIP model's expected input format.src/Metrics/FrechetVideoDistance.cs (3)
71-92: LGTM! Constructor has appropriate validation and sensible defaults.Parameter validation for
featureNetworkandfeatureDimensionis correct. Default values (400 features, 16 frames) align with standard I3D model configurations.
344-394: LGTM! Statistics computation is mathematically correct.The mean and covariance calculations properly use unbiased estimator
1/(n-1)for sample covariance. The minimum 2-sample check prevents division by zero.
437-542: Newton-Schulz iteration implementation is well-designed.Good numerical stability practices: symmetrization of the product matrix, regularization with epsilon, Frobenius norm scaling, and convergence tolerance check. The 15-iteration limit is reasonable for typical cases.
src/Metrics/VideoQualityMetrics.cs (2)
60-103: LGTM! Per-frame PSNR computation with comprehensive statistics.The implementation correctly extracts frames, computes per-frame PSNR, and aggregates statistics (mean, min, max). The use of generic numeric operations maintains type flexibility.
511-541: LGTM! VideoQualityIndex provides comprehensive composite scoring.The weighted combination of PSNR, SSIM, temporal consistency, and flicker metrics is well-structured. Normalization of PSNR to 0-1 scale (clamped at 50 dB) is reasonable for typical video quality ranges.
src/Metrics/CLIPScore.cs (4)
49-86: LGTM! CLIPScore implementation follows standard methodology.Input validation is thorough, and the scoring methodology (cosine similarity scaled to 0-100) aligns with the referenced CLIPScore paper.
80-84: Score scaling discards negative similarity information.Using
Math.Max(0, simDouble) * 100.0maps all negative cosine similarities to 0, losing potentially useful information about semantic dissimilarity. Consider whether this is intentional or if a different mapping (like the one inComputeDirectionalSimilarityusing(sim + 1) * 50) would be more appropriate.Also applies to: 139-142
205-216: LGTM! Harmonic mean for caption scoring is well-motivated.Using harmonic mean to combine image-text and text-reference scores appropriately penalizes cases where either score is low, ensuring the caption is both image-relevant and reference-similar.
518-531: Unable to verify the softmax temperature concern due to inaccessible code and empty code snippet.The review comment references code at lines 518-531 in src/Metrics/CLIPScore.cs, but the provided code snippet is empty, making it impossible to inspect the actual implementation. The repository could not be accessed to retrieve the code.
Regarding the underlying concern about temperature values: web search confirms that OpenAI's CLIP uses a learnable softmax temperature initialized at τ = 0.07, with typical values in the 0.01–0.1 range. The paper notes that CLIP clips logits to prevent scaling by more than 100 as a safeguard. A temperature value of 100 would indeed be unusually high compared to these standards.
However, without accessing the actual code, I cannot verify:
- Whether the temperature parameter is actually set to 100
- Whether 100 refers to temperature or another parameter (e.g., a clipping bound or scaling multiplier)
- How the temperature value is used in the softmax calculation
Please provide the code snippet or ensure the repository is accessible for proper verification.
src/Metrics/KernelInceptionDistance.cs (2)
76-100: LGTM! Constructor has appropriate validation and sensible KID defaults.Default parameters (2048 features, degree 3, 100 subsets, 1000 subset size) align with standard KID configurations from the literature.
144-174: LGTM! Variance estimation via subset sampling is correctly implemented.The fixed seed ensures reproducibility across runs, and the mean/std calculation properly handles edge cases where subsets cannot be formed.
src/NeuralNetworks/ClipNeuralNetwork.cs (2)
78-97: LGTM! Constructor properly validates all required dependencies.Null checks for encoder paths and tokenizer with descriptive exceptions, plus sensible defaults for CLIP model dimensions (512 embedding, 77 max sequence, 224 image size).
518-532: Same high temperature (100) as in CLIPScore - verify this is intentional.CLIP typically uses a learned temperature parameter around 0.01-0.07 where logits are divided by temperature. Here, logits are multiplied by 100, which is equivalent to a very low temperature (0.01), producing sharp probability distributions. Verify this matches expected CLIP behavior.
src/Metrics/AudioMetrics.cs (4)
29-105: LGTM! WordErrorRate implementation is correct and comprehensive.The DP-based edit distance algorithm correctly computes substitutions, insertions, and deletions. Edge cases (empty strings, mismatched lengths) are properly handled. The
ComputeDetailedmethod provides useful debugging information.
274-303: LGTM! Standard Levenshtein distance implementation.The DP algorithm is correct with proper base case initialization and optimal substructure recurrence.
741-787: LGTM! Segmental SNR with proper frame-level processing.Silent frame skipping and clamping to [-10, 35] dB range prevents extreme values from skewing the average. This is a well-known practice in speech quality assessment.
831-855: PESQ approximation is clearly documented as simplified.The remarks explicitly state this is not the official ITU-T P.862 implementation. The combination of STOI and segmental SNR provides a reasonable approximation for quick quality assessments.
Add missing LoadMultimodalModel and GetMultimodalModel implementations to test repository classes to satisfy the IModelRepository interface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…epositories" This reverts commit 26bc90f.
Implement LoadMultimodalModel<T> and GetMultimodalModel<T> methods in: - FakeModelRepository (InferenceControllerTests.cs) - TestModelRepository - InMemoryModelRepository (ModelStartupServiceHashVerificationTests.cs) These methods were added to IModelRepository interface and the test implementations were missing them, causing CS0535 build errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- ModelRepository.cs: add early return for non-multimodal models to improve code clarity and avoid unnecessary property lookups - EmbeddingsController.cs: handle empty predictions dictionary to prevent InvalidOperationException when calling First() on empty collection - AudioMetrics.cs: replace NegativeInfinity with -100.0 for SI-SDR to ensure correct conversion across all numeric types - FrechetVideoDistance.cs: improve video format detection heuristic comment and use RandomHelper.CreateSeededRandom for reproducible random sampling per codebase standards - KernelInceptionDistance.cs: correct misleading comment to indicate that MMD-squared is returned (as is standard for KID) - VideoQualityMetrics.cs: add isChannelsFirst parameter to VideoSSIM for API consistency with VideoPSNR - ClipNeuralNetwork.cs: throw NotSupportedException in deserialization to make limitation explicit instead of silently discarding values 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add constructor parameters for custom positive and negative aesthetic prompts in AestheticScore class. Users can now customize prompts for different aesthetic scoring use cases while maintaining sensible defaults. - Added positivePrompts and negativePrompts optional constructor params - Changed static readonly prompts to DefaultPositivePrompts/DefaultNegativePrompts - Instance fields store either custom or default prompts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Resolve merge conflicts keeping PR review fixes - Fix security issue: replace new Random(42) with RandomHelper.CreateSeededRandom(42) in KernelInceptionDistance - Add missing IMultimodalEmbedding interface methods to multimodal neural networks: - BlipNeuralNetwork - Blip2NeuralNetwork - FlamingoNeuralNetwork - Gpt4VisionNeuralNetwork - LLaVANeuralNetwork - VideoCLIPNeuralNetwork 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
src/Metrics/FrechetVideoDistance.cs (2)
158-159: Format detection heuristic remains fragile despite added comment.The heuristic
videos.Shape[1] > 3to distinguish NTCHW from NCTHW format was previously flagged. While a comment now acknowledges the limitation, the underlying issue persists: videos with exactly 3 frames or unusual channel counts (e.g., RGBA = 4 channels) will be misidentified.
266-274: Fixed seed still produces identical frame sequences across videos.The code was updated from
new Random(42)toRandomHelper.CreateSeededRandom(42), but this still creates a deterministic sequence with a fixed seed each timeSampleFrameIndicesis called. Every video will get identical "random" frame indices, defeating the purpose of theRandomsampling strategy.
🧹 Nitpick comments (25)
tests/AiDotNet.Serving.Tests/ModelStartupServiceHashVerificationTests.cs (1)
76-77: Consider using expression-bodied syntax for conciseness.The block syntax works correctly, but expression-bodied syntax (
=> null;) is more concise for single-statement members.🔎 Proposed refactor
- public IServableMultimodalModel<T>? GetMultimodalModel<T>(string name) - => null; + public IServableMultimodalModel<T>? GetMultimodalModel<T>(string name) => null;src/Metrics/KernelInceptionDistance.cs (4)
42-42: Remove unused Engine property.The
Engineproperty is declared but never used in this class.🔎 Proposed fix
- private readonly INumericOperations<T> _numOps; - private IEngine Engine => AiDotNetEngine.Current; + private readonly INumericOperations<T> _numOps;
123-126: Validate tensor shape compatibility beyond dimensionality.The validation only checks that both tensors are 4D but doesn't ensure they have compatible channel, height, and width dimensions. Mismatched shapes could lead to issues in downstream feature extraction.
🔎 Proposed fix
if (realImages.Shape.Length < 4 || generatedImages.Shape.Length < 4) { throw new ArgumentException("Images must be 4D tensors [N, C, H, W]"); } + if (realImages.Shape[1] != generatedImages.Shape[1] || + realImages.Shape[2] != generatedImages.Shape[2] || + realImages.Shape[3] != generatedImages.Shape[3]) + { + throw new ArgumentException( + "Real and generated images must have matching [C, H, W] dimensions"); + }
192-229: Consider batch processing for improved performance.Extracting features one image at a time in a loop (lines 205-221) may be inefficient for large datasets. If the
FeatureNetwork.Predictmethod supports batch inputs, processing multiple images together could significantly improve performance.
308-338: Consider more efficient sampling for large subset sizes.The rejection sampling approach (lines 316-324) works well when
subsetSizeis much smaller thann, but could become inefficient ifsubsetSizeapproachesn. Consider using a Fisher-Yates partial shuffle for better worst-case performance.🔎 Example alternative approach
private Matrix<T> SampleSubset(Matrix<T> features, int subsetSize, Random random) { int n = features.Rows; int d = features.Columns; - // Generate random indices + // Generate random indices using partial Fisher-Yates var indices = new int[subsetSize]; - var used = new bool[n]; + var pool = new int[n]; + for (int i = 0; i < n; i++) + { + pool[i] = i; + } for (int i = 0; i < subsetSize; i++) { - int idx; - do - { - idx = random.Next(n); - } while (used[idx]); - used[idx] = true; - indices[i] = idx; + int j = random.Next(i, n); + indices[i] = pool[j]; + pool[j] = pool[i]; } // Create subset matrix var subset = new Matrix<T>(subsetSize, d); for (int i = 0; i < subsetSize; i++) { for (int j = 0; j < d; j++) { subset[i, j] = features[indices[i], j]; } } return subset; }src/Metrics/VideoQualityMetrics.cs (7)
199-199: Remove unused default parameter.The
isChannelsFirstparameter has a default value, but all call sites (lines 189-190) explicitly pass this parameter, making the default unreachable.🔎 Proposed fix
-private Tensor<T> ExtractFrame(Tensor<T> video, int frameIdx, int height, int width, int channels, bool isChannelsFirst = false) +private Tensor<T> ExtractFrame(Tensor<T> video, int frameIdx, int height, int width, int channels, bool isChannelsFirst)
272-302: AddisChannelsFirstparameter for format consistency.
VideoPSNRandVideoSSIMsupport both TCHW and THWC formats via theisChannelsFirstparameter, butTemporalConsistencymethods assume THWC only. This inconsistency can cause incorrect results when users switch between metrics with TCHW data.Consider adding
isChannelsFirstsupport toComputeWithFlow,ComputeSimple, andComputeFlickermethods.Also applies to: 313-343, 350-379
404-404: Extract magic normalization constants.The value
10.0appears in multiple normalization formulas (lines 404, 506) without explanation. These should be named constants with documentation explaining the rationale.🔎 Proposed fix
+ // Normalization factor for MSE to 0-1 range (assumes typical MSE is 0-0.1) + private const double MseNormalizationFactor = 10.0; + private T ComputeFrameDifference(Tensor<T> frame1, Tensor<T> frame2) { // ... double mseDouble = _numOps.ToDouble(mse); - double normalized = Math.Min(1.0, mseDouble * 10); + double normalized = Math.Min(1.0, mseDouble * MseNormalizationFactor); return _numOps.FromDouble(normalized); }Apply similar changes to line 404.
Also applies to: 506-506
449-450: Clarify the magic constant 1.001.The value
1.001is used for clamping, likely to avoid edge cases at exact boundaries. Consider adding a comment or using a named constant to explain this choice.
541-571: AddisChannelsFirstparameter for API consistency.
VideoQualityIndex.ComputehardcodesisChannelsFirst=falseat line 544, which means it only supports THWC format. This is inconsistent withVideoPSNRandVideoSSIMthat accept this parameter.🔎 Proposed fix
public VideoQualityResult<T> Compute(Tensor<T> predicted, Tensor<T> groundTruth) +public VideoQualityResult<T> Compute(Tensor<T> predicted, Tensor<T> groundTruth, bool isChannelsFirst = false) { // Compute spatial quality metrics - var psnrStats = _vpsnr.ComputeWithStats(predicted, groundTruth, false); - T ssim = _vssim.Compute(predicted, groundTruth); + var psnrStats = _vpsnr.ComputeWithStats(predicted, groundTruth, isChannelsFirst); + T ssim = _vssim.Compute(predicted, groundTruth, isChannelsFirst);
553-558: Consider parameterizing normalization and weighting constants.The PSNR normalization divisor (50.0) and quality weights (0.3, 0.4, 0.2, 0.1) are hardcoded. For high-quality videos with PSNR > 50 dB, the normalization saturates at 1.0. Consider making these configurable or documenting the assumptions.
105-136: Consider extracting shared frame extraction logic.
ExtractFrameis implemented in three classes with similar logic. This creates maintenance overhead. OnceTemporalConsistencygainsisChannelsFirstsupport, consider extracting a shared helper method (e.g., in aVideoHelperutility class).Also applies to: 199-229, 477-489
src/Metrics/AudioMetrics.cs (2)
781-784: Consider behavior when all frames are silent.Returning zero when
validFrames == 0may be misleading, as 0 dB suggests equal signal and noise power. Consider returning a special sentinel value or documenting this edge case behavior.🔎 Alternative: Return NaN or throw for undefined case
if (validFrames == 0) { - return _numOps.Zero; + return _numOps.FromDouble(double.NaN); // Undefined SNR for silent signal }
810-826: Unused_sampleRatefield.The
_sampleRatefield is assigned in the constructor but never read. It's passed to theShortTimeObjectiveIntelligibilityconstructor, so the field itself can be removed unless it's intended for future use.🔎 Suggested fix
private readonly INumericOperations<T> _numOps; - private readonly int _sampleRate; private readonly ShortTimeObjectiveIntelligibility<T> _stoi; private readonly SignalToNoiseRatio<T> _snr; @@ -823,7 +822,6 @@ public class PerceptualSpeechQuality<T> where T : struct } _numOps = MathHelper.GetNumericOperations<T>(); - _sampleRate = sampleRate; _stoi = new ShortTimeObjectiveIntelligibility<T>(sampleRate); _snr = new SignalToNoiseRatio<T>(); }src/Metrics/FrechetVideoDistance.cs (5)
72-93: Consider validating theframesPerClipconstructor parameter.The constructor validates
featureNetworkandfeatureDimension, butframesPerClipis assigned without validation. SinceFramesPerCliphas a public setter, invalid values (≤ 0) could cause issues inSampleFrameIndicesandExtractVideoClip.🔎 Suggested validation
public FrechetVideoDistance( ConvolutionalNeuralNetwork<T> featureNetwork, int featureDimension = 400, int framesPerClip = 16) { if (featureNetwork == null) { throw new ArgumentNullException(nameof(featureNetwork), "A pre-trained 3D feature extraction network is required for FVD computation"); } if (featureDimension < 1) { throw new ArgumentOutOfRangeException(nameof(featureDimension), "Feature dimension must be at least 1"); } + if (framesPerClip < 1) + { + throw new ArgumentOutOfRangeException(nameof(framesPerClip), + "Frames per clip must be at least 1"); + } _numOps = MathHelper.GetNumericOperations<T>(); FeatureNetwork = featureNetwork; FeatureDimension = featureDimension; FramesPerClip = framesPerClip; SamplingStrategy = FrameSamplingStrategy.Uniform; }
299-340: Remove unused variablespatiotemporalSize.Line 318 declares
int spatiotemporalSize = tensor.Length / channels;but this variable is never used. Consider removing it or using it if it was meant to replace repeated calculations oftensor.Length / channelsin the loop.🔎 Suggested fix
private T[] GlobalAveragePool(Tensor<T> tensor) { // ... early return logic ... int channels = tensor.Shape[1]; var pooled = new T[channels]; - int spatiotemporalSize = tensor.Length / channels; for (int c = 0; c < channels; c++) { T sum = _numOps.Zero; int count = 0; // Sum over all spatial and temporal dimensions for (int idx = 0; idx < tensor.Length / channels; idx++) { int flatIdx = c * (tensor.Length / channels) + idx; if (flatIdx < tensor.Length) { sum = _numOps.Add(sum, tensor.GetFlat(flatIdx)); count++; } } pooled[c] = count > 0 ? _numOps.Divide(sum, _numOps.FromDouble(count)) : _numOps.Zero; } return pooled; }
219-242: Consider optimizing frame data copying if this becomes a performance bottleneck.The nested loops copy video frames element-by-element. For large videos with high resolution, this could be slow. If profiling shows this is a bottleneck, consider using
Buffer.BlockCopy,Span<T>operations, or tensor slicing APIs for more efficient data transfer.
153-154: Consider thread safety implications of modifying shared FeatureNetwork state.The method saves and restores
FeatureNetwork.IsTrainingMode, but if multiple threads callExtractVideoFeaturesconcurrently, race conditions could occur where threads interleave mode changes and restoration. If concurrent usage is expected, consider using a lock or documenting that the class is not thread-safe.🔎 Potential fix using lock
+ private readonly object _networkLock = new object(); + private Matrix<T> ExtractVideoFeatures(Tensor<T> videos) { int numVideos = videos.Shape[0]; var features = new Matrix<T>(numVideos, FeatureDimension); + lock (_networkLock) + { bool originalTrainingMode = FeatureNetwork.IsTrainingMode; FeatureNetwork.SetTrainingMode(false); try { // ... feature extraction logic ... } finally { FeatureNetwork.SetTrainingMode(originalTrainingMode); } + } }Also applies to: 180-182
250-297: Consider documenting behavior whenFramesPerClipexceeds video length.All three sampling strategies handle the case where
FramesPerClip > totalFramesby clamping indices or sampling with replacement. While this prevents errors, it may produce unexpected results (e.g., duplicate frames). Consider documenting this behavior or adding a validation/warning whenFramesPerClipis unreasonably large relative to the video length.src/Metrics/CLIPScore.cs (1)
146-171: ConstraintextWeightto a sane range to avoid surprising combined scores
ComputeCombinedScoreassumestextWeightin[0,1]but doesn’t enforce it. Callers passing values outside that range will implicitly give negative or >1 weight to one term.Consider clamping or validating
textWeight(e.g. throw on out-of-range, ortextWeight = Math.Clamp(textWeight, 0.0, 1.0)) so the combined score is always a convex combination of text and image scores.src/NeuralNetworks/Blip2NeuralNetwork.cs (1)
2493-2580: HardenConvertToTensoragainst malformed or non-square image dataThis
ConvertToTensorhas the same assumptions as in Flamingo:
- Fixed
channels = 3.- Computes
size = (int)Math.Sqrt(imageData.Length / channels)and uses[channels, size, size].- Stops filling when
idx >= imageData.Length, which can silently drop tail values if the length is not exactly3 * size * size.For serving scenarios where callers can send arbitrary flattened arrays, it’s safer to:
- Check
imageData.Length % 3 == 0.- Verify
size * size * channels == imageData.Length.- Optionally ensure
size == _imageSizeto match the model’s expected resolution.- Throw a clear
ArgumentExceptionwhen the shape doesn’t match expectations.Given this helper is now repeated across multiple models, consider extracting a shared utility (e.g.,
ImageTensorHelper.CreateChwTensor<T>(double[] data, int channels, int expectedSize)) to keep checks and semantics consistent.src/NeuralNetworks/LLaVANeuralNetwork.cs (1)
1285-1371: AlignConvertToTensorwith configured image geometry and fail fast on mismatchHere too,
ConvertToTensorinfers a square[3, size, size]tensor fromimageData.Lengthwithout checking:
- That
imageData.Lengthis divisible by 3.- That
size * size * 3 == imageData.Length.- That
sizeis compatible with_imageSizeused elsewhere in the model.Given
LLaVANeuralNetwork’s vision stack is configured around_imageSize, it would be safer to:
- Compute
expectedPixels = 3 * _imageSize * _imageSizeand requireimageData.Length == expectedPixels, or- At least validate that
size == _imageSizeand throw anArgumentExceptionif not.This avoids subtle bugs where mis-sized or non-square inputs propagate through the pipeline with distorted spatial structure.
src/NeuralNetworks/VideoCLIPNeuralNetwork.cs (1)
1609-1695: Validate flattened image length inConvertToTensorto avoid corrupt frame tensorsIn the VideoCLIP context,
EncodeImageandEncodeImageBatchtreat eachdouble[]as a frame, andConvertToTensor:
- Fixes
channels = 3.- Infers
sizeviaMath.Sqrt(imageData.Length / 3).- Allocates
[3, size, size]and stops whenidx >= imageData.Length.If callers pass non-square images, mismatched resolutions, or arrays with an off-by-one length, you’ll silently construct an incorrect frame tensor that then flows into your frame/temporal encoders.
Given VideoCLIP’s temporal stack is sensitive to per-frame features, it’d be better to:
- Require
imageData.Length == 3 * _imageSize * _imageSize(or at least ensuresize == _imageSizeandsize * size * 3 == imageData.Length).- Throw a clear exception when the check fails.
That keeps bad inputs from causing hard-to-diagnose downstream issues.
src/NeuralNetworks/BlipNeuralNetwork.cs (1)
1981-2067: Consider extracting shared multimodal embedding adapter logic.The entire
IMultimodalEmbedding Interface (Standard API)region is duplicated betweenBlipNeuralNetwork,Gpt4VisionNeuralNetwork, and likely other neural network classes. TheConvertToTensor,EncodeTextBatch, andEncodeImageBatchimplementations are identical.Consider extracting this common logic to:
- A base class helper method for
ConvertToTensor- Extension methods for batch encoding
- A shared adapter class
This would reduce duplication and make the validation fixes easier to apply consistently.
src/NeuralNetworks/ClipNeuralNetwork.cs (1)
514-528: Softmax temperature is hardcoded.The temperature value of 100.0 is hardcoded in the
Softmaxmethod. While 100 is a typical CLIP value, in actual CLIP models this is a learned parameter. Consider making it configurable via a constructor parameter or property.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/AiDotNet.Serving/Controllers/EmbeddingsController.cssrc/AiDotNet.Serving/Services/ModelRepository.cssrc/Metrics/AudioMetrics.cssrc/Metrics/CLIPScore.cssrc/Metrics/FrechetVideoDistance.cssrc/Metrics/KernelInceptionDistance.cssrc/Metrics/VideoQualityMetrics.cssrc/NeuralNetworks/Blip2NeuralNetwork.cssrc/NeuralNetworks/BlipNeuralNetwork.cssrc/NeuralNetworks/ClipNeuralNetwork.cssrc/NeuralNetworks/FlamingoNeuralNetwork.cssrc/NeuralNetworks/Gpt4VisionNeuralNetwork.cssrc/NeuralNetworks/LLaVANeuralNetwork.cssrc/NeuralNetworks/VideoCLIPNeuralNetwork.cstests/AiDotNet.Serving.Tests/ModelStartupServiceHashVerificationTests.cstests/AiDotNet.Serving.Tests/TestModelRepository.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/AiDotNet.Serving/Services/ModelRepository.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/AiDotNet.Serving/Controllers/EmbeddingsController.cssrc/NeuralNetworks/LLaVANeuralNetwork.cssrc/NeuralNetworks/Blip2NeuralNetwork.cssrc/Metrics/FrechetVideoDistance.cssrc/NeuralNetworks/FlamingoNeuralNetwork.cssrc/NeuralNetworks/BlipNeuralNetwork.cstests/AiDotNet.Serving.Tests/ModelStartupServiceHashVerificationTests.cssrc/NeuralNetworks/VideoCLIPNeuralNetwork.cssrc/NeuralNetworks/Gpt4VisionNeuralNetwork.cssrc/NeuralNetworks/ClipNeuralNetwork.cssrc/Metrics/KernelInceptionDistance.cssrc/Metrics/VideoQualityMetrics.cstests/AiDotNet.Serving.Tests/TestModelRepository.cssrc/Metrics/CLIPScore.cssrc/Metrics/AudioMetrics.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/AiDotNet.Serving/Controllers/EmbeddingsController.cssrc/NeuralNetworks/LLaVANeuralNetwork.cssrc/NeuralNetworks/Blip2NeuralNetwork.cssrc/Metrics/FrechetVideoDistance.cssrc/NeuralNetworks/FlamingoNeuralNetwork.cssrc/NeuralNetworks/BlipNeuralNetwork.cstests/AiDotNet.Serving.Tests/ModelStartupServiceHashVerificationTests.cssrc/NeuralNetworks/VideoCLIPNeuralNetwork.cssrc/NeuralNetworks/Gpt4VisionNeuralNetwork.cssrc/NeuralNetworks/ClipNeuralNetwork.cssrc/Metrics/KernelInceptionDistance.cssrc/Metrics/VideoQualityMetrics.cstests/AiDotNet.Serving.Tests/TestModelRepository.cssrc/Metrics/CLIPScore.cssrc/Metrics/AudioMetrics.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
🔇 Additional comments (15)
src/Metrics/VideoQualityMetrics.cs (1)
578-619: LGTM!The result class follows appropriate C# conventions for DTOs. The
default!suppressions are acceptable since all properties are initialized inVideoQualityIndex.Computebefore the object is returned to callers.src/Metrics/AudioMetrics.cs (3)
29-189: LGTM! Well-implemented WER with detailed error tracking.The dynamic programming approach correctly computes edit distance while tracking individual operation counts. The case-insensitive tokenization and handling of edge cases (empty strings) are appropriate.
203-304: LGTM! Standard CER implementation.The Levenshtein distance computation and whitespace handling are correctly implemented. Edge cases are properly handled.
566-673: LGTM! Correct SI-SDR implementation with appropriate edge case handling.The scale-invariant projection and residual computation follow the standard SI-SDR formula. The -100/+100 dB clamping for edge cases is appropriate.
src/Metrics/FrechetVideoDistance.cs (3)
342-395: LGTM: Statistics computation is well-implemented.The method correctly validates minimum sample count, computes the sample mean, centers features, and calculates the sample covariance matrix using the unbiased estimator (n-1 denominator). The use of engine operations for matrix multiplication is appropriate.
435-543: LGTM: Newton-Schulz iteration includes good numerical practices.The implementation of
ComputeTraceSqrtCovProductuses Newton-Schulz iteration with several important numerical stability techniques: symmetrization of the product matrix, regularization (eps = 1e-6), Frobenius norm scaling, and convergence tolerance checking. The algorithm follows established practices for computing matrix square roots.Ensure comprehensive unit tests validate this numerical algorithm with known matrix inputs to confirm correctness, especially for edge cases (near-singular matrices, ill-conditioned covariances).
546-565: LGTM: Enum is well-defined and documented.The
FrameSamplingStrategyenum provides clear options for frame sampling with descriptive XML documentation for each strategy.src/AiDotNet.Serving/Controllers/EmbeddingsController.cs (1)
365-371: TopLabel guard correctly handles empty prediction setsThe updated
TopLabelassignment now safely handles the case wherepredictionsis empty by returning an empty string instead of callingFirst()on an empty sequence. This removes the previousInvalidOperationExceptionrisk without changing behavior when predictions are present.src/NeuralNetworks/FlamingoNeuralNetwork.cs (1)
1262-1348: MakeConvertToTensorvalidate image shape instead of inferring silentlyThe new IMultimodalEmbedding wrappers are fine, but
ConvertToTensorcurrently:
- Assumes 3 channels and a square image.
- Infers side length as
size = (int)Math.Sqrt(pixels).- Fills a
[3, size, size]tensor while stopping whenidx >= imageData.Length.If
imageData.Lengthis not exactly3 * N * NorN != _imageSize, the method will quietly truncate or mis-shape the tensor, which can be hard to debug and may violate assumptions in downstream vision code.Consider:
- Validating that
imageData.Length % 3 == 0and thatsize * size * 3 == imageData.Length.- Optionally also asserting
size == _imageSize(or explicitly documenting that variable sizes are supported).- Throwing an
ArgumentExceptionwhen the shape is inconsistent, rather than proceeding with a partially filled tensor.You might also want to factor this pattern into a shared helper to avoid repeating the same logic (and checks) in other multimodal models.
src/NeuralNetworks/Gpt4VisionNeuralNetwork.cs (2)
1728-1745: Empty batch handling looks good.The methods correctly handle empty input collections by returning a 0 x EmbeddingDimension matrix, which is a sensible design choice that avoids exceptions for edge cases while maintaining dimensional consistency.
Also applies to: 1756-1774
1722-1725: LGTM! Clean delegation to existing methods.The adapter methods correctly delegate to existing internal implementations, providing a clean standard API surface while reusing validated logic.
Also applies to: 1748-1753, 1777-1781
src/NeuralNetworks/BlipNeuralNetwork.cs (1)
1984-2043: LGTM! Standard API implementation follows established patterns.The adapter methods correctly delegate to existing implementations and handle edge cases appropriately.
src/NeuralNetworks/ClipNeuralNetwork.cs (3)
201-206: LGTM! Deserialization limitation is now explicit.The method correctly throws
NotSupportedExceptionwith a clear message, addressing the concern from the previous review. This makes the limitation explicit rather than silently discarding data.
65-65: LGTM! Training limitation is properly communicated.The
SupportsTrainingproperty andTrainmethod consistently indicate that training is not supported, with a clear exception message.Also applies to: 147-152
418-476: Placeholder embedding functions do not perform actual ONNX model inference.The
GenerateTextEmbeddingandGenerateImageEmbeddingmethods use deterministic mathematical functions (trigonometric and statistical operations) rather than running inference through trained CLIP models. The code explicitly comments "This is a placeholder that should be replaced with actual ONNX inference."These placeholder embeddings cannot produce semantically meaningful representations. For example,
GenerateTextEmbeddinggenerates values usingMath.Sin((double)seed * 0.000001)based on a token hash—this has no relationship to text semantics and will not align with image embeddings in a shared space.Clarify whether this stub implementation is intended for testing only or should be replaced with actual ONNX model inference for production use.
- Remove duplicate method declarations in TestModelRepository (Critical) - Add parameter validation to STOI constructor in AudioMetrics - Add input validation for shape consistency in VideoPSNR and VideoSSIM - Add image dimension/channel validation to ConvertToTensor in 6 neural network classes - Update ComputeBandEnergy documentation to clarify approximation behavior - Add empty prompt set validation to AestheticScore constructor - Fix inconsistent empty batch handling in ClipNeuralNetwork (return empty matrix) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Throw ArgumentException for null/empty paths (not ArgumentNullException) - Add FileNotFoundException for non-existent model files - Validate null/empty checks before file existence checks - Path validation comes before other parameter validations - All 8 ClipNeuralNetworkTests now pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
…D overflow Adds the gradient/L2/tape-walk/pure-train probes and the TensorMatMul micro-test used to localize the ACEStep training NaN to the Tensors double GELU kernel's unclamped tanh-via-exp decomposition (naive preAct=21.657723, scalar GELU=21.657723, kernel value=NaN). Fixed upstream in AiDotNet.Tensors PR #594; validated end-to-end against a locally packed 0.92.6 prerelease: ForwardPass/Clone AfterTraining now pass, DifferentInputs transforms from NaN into a finite uniform-output collapse (separate residual-free-stack architecture issue). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…LAS spin-park) 0.94.2 lands the fixes this PR's ACEStep work depends on plus two broadly-beneficial improvements: - #594 SIMD double GELU/Tanh/Mish tanh-via-exp ±20 clamp — fixes the Inf/Inf=NaN at GELU input >=~19.8 that NaN'd ACEStep training (ForwardPass/Clone AfterTraining) and any double model with activations past ~20. - #589/#590 park idle StreamingWorkerPool workers instead of yield-spin — the conv-throughput busy-spin (47% wasted CPU) filed from this work. - #593 7-9x faster fused-MLP compiled training step + #588 GPU/CPU parity fixes. Verified against the released 0.94.2 (not a local pack): ACEStep ForwardPass/Clone/DifferentInputs AfterTraining all pass. Native packages bumped in lockstep; restore confirmed all four exist at 0.94.2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Adds exhaustive evaluation metrics for multimodal AI outputs covering image, audio, and video quality assessment.
Image Quality Metrics
Audio Quality Metrics
Video Quality Metrics
Test Plan
Closes #281
🤖 Generated with Claude Code