Fix issue 376 and related bugs
…lculators
This commit implements comprehensive unit tests for four specialized loss function fitness calculators as requested in issue #376:
- DiceLossFitnessCalculator: Tests for medical imaging and segmentation scenarios
- JaccardLossFitnessCalculator: Tests for object detection using IoU metrics
- ContrastiveLossFitnessCalculator: Tests for similarity learning (face recognition)
- CosineSimilarityLossFitnessCalculator: Tests for document similarity and embeddings
Test Coverage Includes:
- Perfect predictions (zero loss)
- Worst case scenarios (maximum loss)
- Partial overlaps and varying degrees of similarity
- Edge cases (division by zero, empty sets, all zeros)
- Probabilistic predictions
- Different numeric types (float, double)
- Real-world scenarios (medical imaging, object detection, etc.)
- Proper null handling and exception testing
Each test file contains 20+ comprehensive test cases covering:
- Constructor behavior with different data set types
- Mathematical correctness of loss calculations
- IsBetterFitness comparison logic
- ModelEvaluationData integration
- Float and double type support
- Magnitude invariance (for cosine similarity)
- Imbalanced data handling
The tests follow the existing project patterns and conventions used in other loss function tests (e.g., MeanSquaredErrorLossTests, HuberLossTests).
Resolves #376
User Story / Context
- Reference: [US-XXX] (if applicable)
- Base branch:
merge-dev2-to-master
Summary
- What changed and why (scoped strictly to the user story / PR intent)
Verification
- [ ] Builds succeed (scoped to changed projects)
- [ ] Unit tests pass locally
- [ ] Code coverage >= 90% for touched code
- [ ] Codecov upload succeeded (if token configured)
- [ ] TFM verification (net46, net6.0, net8.0) passes (if packaging)
- [ ] No unresolved Copilot comments on HEAD
Copilot Review Loop (Outcome-Based)
Record counts before/after your last push:
- Comments on HEAD BEFORE: [N]
- Comments on HEAD AFTER (60s): [M]
- Final HEAD SHA: [sha]
Files Modified
- [ ] List files changed (must align with scope)
Notes
- Any follow-ups, caveats, or migration details
[!WARNING]
Rate limit exceeded
@ooples has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 19 seconds before requesting another review.
⌛ How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the
@coderabbitai reviewcommand as a PR comment. Alternatively, push new commits to this PR.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.
📥 Commits
Reviewing files that changed from the base of the PR and between 4d1842a15b39cc3013e9c2a27e9f853764c8c036 and f92a93f325e0772c2378c99b486c378d435cdd1b.
📒 Files selected for processing (3)
.github/workflows/pr-tests.yml(1 hunks)commitlint.config.js(2 hunks)src/LossFunctions/RotationPredictionLoss.cs(2 hunks)
Summary by CodeRabbit
Release Notes
-
Tests
- Added comprehensive unit test suites for fitness calculators (ContrastiveLoss, CosineSimilarityLoss, DiceLoss, JaccardLoss) covering edge cases, multiple data types, and domain scenarios.
- Updated fairness evaluator and transaction tests with corrected test data and expectations.
-
Bug Fixes
- Fixed tensor handling for empty/scalar inputs in data conversion utilities.
- Improved backward pass efficiency in neural network layers using caching and optimized computations.
- Enhanced cropping layer to properly handle image data format conversions.
-
Chores
- Updated CI/CD workflow to enforce net8.0 framework for test execution.
✏️ Tip: You can customize this high-level summary in your review settings.
Walkthrough
Adds comprehensive unit tests for four loss calculators and multiple internal fixes: tensor conversion edge handling, adjusted bias broadcasting and projection in layers, NHWC↔NCHW cropping/grads, rotation output type handling, ModelStats empty-input guard, and CI test workflow tightening.
Changes
| Cohort / File(s) | Change Summary |
|---|---|
Loss Function Test Suites tests/AiDotNet.Tests/UnitTests/FitnessCalculators/ContrastiveLossFitnessCalculatorTests.cs, tests/AiDotNet.Tests/UnitTests/FitnessCalculators/CosineSimilarityLossFitnessCalculatorTests.cs, tests/AiDotNet.Tests/UnitTests/FitnessCalculators/DiceLossFitnessCalculatorTests.cs, tests/AiDotNet.Tests/UnitTests/FitnessCalculators/JaccardLossFitnessCalculatorTests.cs |
New, extensive unit test classes covering constructors, many CalculateFitnessScore scenarios (positive/negative/edge cases), float/double/Tensor inputs, ModelEvaluationData dataset selection, null handling, IsHigherScoreBetter/IsBetterFitness semantics, and domain scenarios. |
CI Workflow .github/workflows/pr-validation.yml |
Test step now passes --framework net8.0 to dotnet dotcover test, removed continue-on-error: true, and added clarifying comments about required tests. |
Tensor Conversion Helpers src/Helpers/ConversionsHelper.cs |
Improved Tensor<T> → Matrix/Vector conversions: handle scalar/empty tensors (return empty), reshape 1D tensors to 1xN row matrices, and flatten 3D+ tensors to 2D appropriately; no public signatures changed. |
Rotation Loss Output Handling src/LossFunctions/RotationPredictionLoss.cs |
Return path now converts one-hot rotation labels into class-index Vector<T> when TOutput is Vector<T>; preserved augmentation logic otherwise. |
Cropping Layer (NHWC↔NCHW) src/NeuralNetworks/Layers/CroppingLayer.cs |
Forward/backward convert NHWC→NCHW, call Engine.Crop/CropBackward in NCHW, then convert results back to NHWC; gradient conversions updated accordingly. |
Bias Broadcasting & Patch Projection src/NeuralNetworks/Layers/DenseLayer.cs, src/NeuralNetworks/Layers/GatedLinearUnitLayer.cs, src/NeuralNetworks/Layers/PatchEmbeddingLayer.cs |
Dense/GLU: bias adds replaced with broadcast-aware TensorBroadcastAdd; PatchEmbedding: projection rewritten to flatten [B,N,patchDim]→[B*N,patchDim], matmul, then reshape to [B,N,embedDim]. |
Convolutional Backward Simplification src/NeuralNetworks/Layers/ConvolutionalLayer.cs |
BackwardManual now sets delta directly via ApplyActivationDerivative(_lastOutput, outputGradient) (removes separate multiplication by outputGradient). |
Highway Layer Pre-activation Caching src/NeuralNetworks/Layers/HighwayLayer.cs |
Added caches for transform/gate pre-activations; forward stores them, backward uses them for derivative calculations, and ResetState clears caches. |
Model Statistics Guard src/Statistics/ModelStats.cs |
ModelStats constructor skips stats calculation for empty inputs; added private IsEmptyInput helper for empty XMatrix/Tensor detection. |
Test Adjustments / Expectations tests/AiDotNet.Tests/UnitTests/Interpretability/FairnessEvaluatorTests.cs, tests/AiDotNet.Tests/UnitTests/RetrievalAugmentedGeneration/GraphTransactionTests.cs |
FairnessEvaluatorTests: test data grouping updated (different per-group predictions/actuals). GraphTransactionTests: expected WAL entry count in full-ACID test changed (7 → 4). |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
- Focus review on:
- New loss tests: validate numeric assertions/tolerances and that test inputs match formulas.
- CroppingLayer NHWC↔NCHW conversions and gradient correctness.
- Bias broadcasting changes in Dense/GLU: ensure callers/weight shapes remain compatible.
- PatchEmbedding reshape/matmul/reshape path and any memory/layout impact.
- ConversionsHelper handling of scalar/empty/1D tensors to prevent downstream regressions.
- HighwayLayer pre-activation caching usage and ResetState behavior.
- RotationPredictionLoss return-type branch for Vector<T> consumers.
- CI workflow removal of continue-on-error: confirm test suite stability.
Possibly related PRs
- ooples/AiDotNet#478 — Overlapping changes to neural-network layer forward/backward logic (CroppingLayer, DenseLayer, ConvolutionalLayer, PatchEmbeddingLayer, HighwayLayer).
- (no other strong code-level matches found)
Poem
🐰
I nudged the tests with whiskers bright,
Contrast and Cosine hopped into light,
Dice and Jaccard danced in tidy rows,
Tensors shaped and biases in prose,
A happy rabbit — CI lights aglow.
Pre-merge checks and finishing touches
❌ Failed checks (2 warnings, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check | ⚠️ Warning | The PR includes several changes beyond the linked issue #376 scope: workflow YAML modifications, helper function updates (ConversionsHelper.cs), loss function changes (RotationPredictionLoss.cs), neural network layer modifications (CroppingLayer, DenseLayer, GatedLinearUnitLayer, PatchEmbeddingLayer, ConvolutionalLayer, HighwayLayer), statistical calculations (ModelStats.cs), and test data adjustments (FairnessEvaluatorTests, GraphTransactionTests). | Review and document why these out-of-scope changes are necessary. Consider isolating the loss function tests into a separate PR focused solely on #376, or update the linked issue to reflect the broader scope of changes. |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
| Title check | ❓ Inconclusive | The title 'Fix issue 376 and related bugs' is vague and does not clearly convey that the PR adds comprehensive unit tests for specialized loss function calculators. | Consider revising the title to be more specific, e.g., 'Add unit tests for specialized loss function fitness calculators' to better describe the main change. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description check | ✅ Passed | The description provides comprehensive context about the unit tests being added for the four specialized loss function calculators, covering test scenarios and patterns. |
| Linked Issues check | ✅ Passed | The PR successfully implements comprehensive unit tests for all four specialized loss function calculators (Dice, Jaccard, Contrastive, CosineSimilarity) as required by issue #376, with 20+ test cases per file covering edge cases, mathematical correctness, and integration scenarios. |
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 @coderabbitai help to get the list of available commands and usage tips.