feat(neural-networks): implement UpdateParameters for 6 network classes - #240
Conversation
|
Warning Rate limit exceeded@ooples has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 28 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 (2)
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. Summary by CodeRabbit
WalkthroughThree neural network classes—EchoStateNetwork, NEAT, and RestrictedBoltzmannMachine—receive UpdateParameters implementations that validate incoming parameter vector lengths and assign values to internal weights and bias arrays, replacing previous no-op or exception-throwing stubs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/NeuralNetworks/SelfOrganizingMap.cs (3)
624-627: Fix array shape comparison (always true due to reference inequality).Use length and index checks (or SequenceEqual); current code will throw for valid inputs.
- if (input.Shape != new[] { _inputDimension }) + if (input.Shape.Length != 1 || input.Shape[0] != _inputDimension) { throw new ArgumentException($"Input shape must be [{_inputDimension}], but got {string.Join(", ", input.Shape)}"); }Apply the same check in Train.
- if (input.Shape != new[] { _inputDimension }) + if (input.Shape.Length != 1 || input.Shape[0] != _inputDimension) { throw new ArgumentException($"Input shape must be [{_inputDimension}], but got {string.Join(", ", input.Shape)}"); }Also applies to: 667-670
439-440: Integer division bug in learning-rate schedule.Cast to double before division; currentEpoch/totalEpochs truncates to 0 until the end.
- return NumOps.Multiply(initialLearningRate, NumOps.Exp(NumOps.Negate(NumOps.FromDouble(currentEpoch / totalEpochs)))); + return NumOps.Multiply( + initialLearningRate, + NumOps.Exp(NumOps.Negate(NumOps.FromDouble((double)currentEpoch / totalEpochs))) + );
263-264: Store Random as a field instead of creating new instances on each call.The inherited
Randomproperty inNeuralNetworkBasecreates a new instance with each access (protected Random Random => new();). In the tight loop at line 263, this generates many Random instances in rapid succession, each with the same system clock seed, resulting in poor-quality randomness.Store the instance as a field (e.g.,
private readonly Random _random = new();) and use it consistently, matching the pattern found inSamplingHelper.csand other classes in the codebase.src/NeuralNetworks/EchoStateNetwork.cs (1)
1020-1045: Update XML summary to reflect implemented behavior.Comment still says “Always thrown/Not implemented” but method now updates output weights and bias.
src/NeuralNetworks/RestrictedBoltzmannMachine.cs (1)
369-377: Fix weight initialization indexing (row/col swapped).Matrix is HiddenSize×VisibleSize, but loops iterate VisibleSize then HiddenSize and write _weights[i, j]. Use _weights[j, i] or swap loops.
- for (int i = 0; i < VisibleSize; i++) - { - _visibleBiases[i] = NumOps.Zero; - for (int j = 0; j < HiddenSize; j++) - { - _weights[i, j] = NumOps.FromDouble(Random.NextDouble() * 0.1 - 0.05); - } - } + for (int j = 0; j < HiddenSize; j++) + { + for (int i = 0; i < VisibleSize; i++) + { + _weights[j, i] = NumOps.FromDouble(Random.NextDouble() * 0.1 - 0.05); + } + } + for (int i = 0; i < VisibleSize; i++) + { + _visibleBiases[i] = NumOps.Zero; + }
🧹 Nitpick comments (6)
VERIFICATION_US-IF-001.md (2)
79-91: Verification commands appear to be placeholders.Lines 79–91 show bash command examples with "# No matches" as output, but these appear to be illustrative placeholders rather than actual verification output. Consider either:
- Running these commands and embedding the actual results, or
- Removing the bash block if it's not needed for documentation
This is a minor clarity issue.
112-112: Minor: Consider rewording "with respect to" for conciseness.Line 112 uses "with respect to," which is somewhat wordy. Consider a shorter alternative such as "regarding" or restructuring the sentence.
-The codebase is production-ready with respect to these interface implementations. +The codebase is production-ready regarding these interface implementations.src/NeuralNetworks/SelfOrganizingMap.cs (2)
197-198: Avoid Console.WriteLine in library code; use a logger.Replace with injected logger or a central logging abstraction.
144-146: Clean up encoding artifacts in XML docs.Replace “�” with “×” or “x”, and proper symbols (e.g., “·”, “^”). This leaks into generated docs.
Also applies to: 334-336, 541-543
src/NeuralNetworks/ExtremeLearningMachine.cs (1)
147-157: Add length check against output layer ParameterCount.Prevents silent shape mismatches when callers pass vectors of wrong size.
- var outputLayer = Layers[Layers.Count - 1]; - - // Update the output layer parameters - outputLayer.UpdateParameters(parameters); + var outputLayer = Layers[Layers.Count - 1]; + int expected = outputLayer.ParameterCount; + if (parameters.Length != expected) + { + throw new ArgumentException($"Parameter vector length mismatch. Expected {expected} but got {parameters.Length}.", nameof(parameters)); + } + outputLayer.UpdateParameters(parameters);src/NeuralNetworks/RestrictedBoltzmannMachine.cs (1)
113-114: Doc nit: replace “28�28” with “28×28” (or “28x28”).Cosmetic but improves generated docs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
VERIFICATION_US-IF-001.md(1 hunks)src/NeuralNetworks/EchoStateNetwork.cs(1 hunks)src/NeuralNetworks/ExtremeLearningMachine.cs(4 hunks)src/NeuralNetworks/HopfieldNetwork.cs(1 hunks)src/NeuralNetworks/NEAT.cs(2 hunks)src/NeuralNetworks/RestrictedBoltzmannMachine.cs(2 hunks)src/NeuralNetworks/SelfOrganizingMap.cs(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/NeuralNetworks/ExtremeLearningMachine.cs (3)
src/NeuralNetworks/EchoStateNetwork.cs (1)
UpdateParameters(1044-1071)src/NeuralNetworks/GraphNeuralNetwork.cs (1)
UpdateParameters(418-431)src/NeuralNetworks/NeuralNetwork.cs (1)
UpdateParameters(141-154)
🪛 LanguageTool
VERIFICATION_US-IF-001.md
[style] ~112-~112: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...n met. The codebase is production-ready with respect to these interface implementations. No fur...
(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
🔇 Additional comments (5)
VERIFICATION_US-IF-001.md (2)
1-9: Inconsistency: AI summary conflicts with PR objectives.The AI-generated summary states "This pull request implements functional parameter-update logic," but the PR objectives explicitly state "No code changes were necessary" and this file is documentation-only verification. These conflict substantially.
Please clarify which is accurate: Did this PR include code implementations, or is it purely a verification report?
12-75: Corrected: LoadModel line number and method misidentification in PredictionModelResult.cs.The verification document confuses two distinct methods in PredictionModelResult.cs. Lines 482-496 contain the instance method
LoadFromFile(), not the staticLoadModel()method. The staticLoadModel()method begins at line 537. All other line numbers and implementation details have been verified as accurate.### 1. ModelIndividual.cs **Location**: `src/Genetics/ModelIndividual.cs` **Status**: ✅ Complete All 10 interface methods fully implemented: - ✅ `Train(TInput, TOutput)` - Line 217-220 - ✅ `GetModelMetadata()` - Line 222-225 - ✅ `GetActiveFeatureIndices()` - Line 227-230 - ✅ `GetFeatureImportance()` - Line 232-235 - ✅ `SetActiveFeatureIndices()` - Line 237-240 - ✅ `IsFeatureUsed(int)` - Line 242-245 - ✅ `DeepCopy()` - Line 247-264 - ✅ `Clone()` - Line 266-283 - ✅ `SetParameters(Vector<T>)` - Line 285-289 - ✅ `ParameterCount` - Line 291-293 All methods properly delegate to the inner model (`_innerModel`). ### 2. PredictionModelResult.cs **Location**: `src/Models/Results/PredictionModelResult.cs` **Status**: ✅ Complete All interface methods fully implemented: - ✅ `GetModelMetadata()` - Line 259-262 - ✅ `Predict(TInput)` - Line 295-311 - ✅ `Serialize()` - Line 338-357 - ✅ `Deserialize(byte[])` - Line 388-419 - ✅ `SaveModel(string)` - Line 449-452 - ✅ `LoadModel(string)` - Line 537+ Properly wraps and delegates to the underlying model with normalization support. ### 3. ExpressionTree.cs **Location**: `src/LinearAlgebra/ExpressionTree.cs` **Status**: ✅ Complete All IFullModel methods fully implemented: - ✅ `GetFeatureImportance()` - Line 854-908 - Counts feature occurrences recursively - Normalizes importance scores by total occurrences - ✅ `SetActiveFeatureIndices(IEnumerable<int>)` - Line 919-959 - Deactivates features not in the active set by converting them to constant zero - Clears cached feature count - ✅ `SetParameters(Vector<T>)` - Line 1179-1238 - Two-pass implementation for validation - Counts constants, validates parameter count, then assigns - Includes atomicity guarantee ### 4. DecoderLayer.cs **Location**: `src/NeuralNetworks/Layers/DecoderLayer.cs` **Status**: ✅ Complete Forward method fully implemented: - ✅ `Forward(params Tensor<T>[])` - Line 137-156 - Validates 2-3 input tensors (decoder input, encoder output, optional mask) - Delegates to `ForwardInternal()` - ✅ `ForwardInternal()` - Line 171-190 - Implements self-attention with normalization - Implements cross-attention with encoder output - Implements feed-forward network - Includes residual connections and layer normalization - ✅ `Forward(Tensor<T>)` - Line 381-387 - Throws `NotSupportedException` (by design, requires multiple inputs)src/NeuralNetworks/SelfOrganizingMap.cs (1)
579-596: UpdateParameters for SOM looks correct; confirm row-major mapping.Length guard and write order are sensible. Please confirm the expected flattening order is row-major (neuron rows, input-dim columns).
src/NeuralNetworks/RestrictedBoltzmannMachine.cs (1)
451-484: Vector layout confirmed consistent between UpdateParameters and serialization methods.The weights (HiddenSize×VisibleSize loop), visible biases, and hidden biases are serialized and deserialized in the same order as UpdateParameters reads them—no mismatch.
However, note: UpdateParameters only reconstructs weights and biases from the parameter vector, while SerializeNetworkSpecificData and DeserializeNetworkSpecificData also handle learning rate, cdSteps, and activation type. Confirm whether UpdateParameters is intentionally scoped to weights/biases only, or if it should also restore those configuration fields.
src/NeuralNetworks/EchoStateNetwork.cs (1)
1046-1071: Ensure ESN ParameterCount and GetParameters() align with UpdateParameters contract.The UpdateParameters method validates that the input length matches
reservoirSize * outputSize + outputSize, which is correct. However, ESN's_outputWeightsand_outputBiasare standalone fields, not part of the Layers collection. Since ESN does not overrideParameterCountorGetParameters(), the base class implementation sums onlyLayersparameters, creating a contract mismatch:
UpdateParametersexpects:reservoirSize * outputSize + outputSizeparametersGetParameters()(inherited) returns: sum of layer parameters (likely incomplete)- External callers relying on
ParameterCountwill pass incorrect lengthsNeuralNetworkARIMAModel currently works around this by manually calculating slice length (line 393), but the contract should be properly enforced.
Action: Override
ParameterCountto return_reservoirSize * _outputSize + _outputSize, or refactor output parameters into a managed layer to ensure consistency across the interface.
ooples
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Fixed critical bugs identified in code review: 1. SelfOrganizingMap.cs: - Fixed array shape comparison bug (was always true due to reference inequality) - Fixed integer division bug in learning rate calculation - Changed to proper array length and element comparison 2. RestrictedBoltzmannMachine.cs: - Fixed weight initialization indexing bug (rows/columns were swapped) - Matrix is [HiddenSize, VisibleSize] but loops were backwards 3. EchoStateNetwork.cs: - Restored file from master (was empty in branch) - File now contains proper implementation Addresses CodeRabbit CHANGES_REQUESTED feedback 🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/NeuralNetworks/HopfieldNetwork.cs (1)
322-365: Symmetry enforcement successfully implemented.The implementation correctly addresses the past review comment by enforcing symmetric weights. The approach is sound:
- Validates parameter count (_size × _size)
- Populates weight matrix from parameters, explicitly zeroing diagonals
- Enforces symmetry by averaging each off-diagonal pair: W[i,j] = W[j,i] = (W[i,j] + W[j,i]) / 2
The parameter format is clearly documented: callers supply _size² parameters, diagonal entries are consumed but ignored.
Optional: Eliminate redundant diagonal zeroing.
Diagonal elements are explicitly set to zero twice: once in the first pass (line 342) and again in the second pass (line 363). The second assignment (line 363) is redundant since diagonals are already zero after the first pass.
Apply this diff to remove the redundant operation:
// Second pass: enforce symmetry by averaging off-diagonal pairs for (int i = 0; i < _size; i++) { for (int j = i + 1; j < _size; j++) { // Average the (i,j) and (j,i) entries to enforce symmetry T avg = NumOps.Divide(NumOps.Add(_weights[i, j], _weights[j, i]), NumOps.FromDouble(2.0)); _weights[i, j] = avg; _weights[j, i] = avg; } - // Explicitly ensure diagonal is zero - _weights[i, i] = NumOps.Zero; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/NeuralNetworks/HopfieldNetwork.cs(2 hunks)src/NeuralNetworks/SelfOrganizingMap.cs(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/NeuralNetworks/HopfieldNetwork.cs (4)
src/NeuralNetworks/SelfOrganizingMap.cs (6)
UpdateParameters(580-600)Vector(548-557)T(339-349)T(437-443)T(475-478)T(510-516)src/NeuralNetworks/FeedForwardNeuralNetwork.cs (1)
UpdateParameters(226-239)src/NeuralNetworks/Layers/SpikingLayer.cs (7)
UpdateParameters(1124-1143)UpdateParameters(1504-1530)Vector(725-740)Vector(769-805)Vector(833-864)Vector(894-930)Vector(961-1000)src/NeuralNetworks/NeuralNetworkBase.cs (6)
UpdateParameters(774-774)Vector(272-275)Vector(287-309)Vector(701-721)Vector(1852-1869)T(833-842)
🔇 Additional comments (7)
src/NeuralNetworks/HopfieldNetwork.cs (1)
203-203: LGTM: Documentation correction.Fixed character encoding issue in capacity documentation.
src/NeuralNetworks/SelfOrganizingMap.cs (6)
144-144: LGTM: Documentation improvement.Minor formatting improvement to documentation.
334-334: LGTM: More accurate distance calculation description.Documentation now correctly describes Euclidean distance as the square root of squared differences.
439-442: LGTM: Learning rate calculation refactored for clarity.The multi-line format improves readability. The computation remains functionally equivalent:
initialLearningRate × exp(-(currentEpoch / totalEpochs)). The explicit cast todoublebefore division ensures proper floating-point arithmetic.
544-544: LGTM: Formula clarification.Documentation now explicitly shows the weight update formula:
learningRate × influence × (input - weight).
580-600: UpdateParameters correctly implemented.The implementation properly populates the SOM weight matrix from the parameter vector:
- Validates parameter length matches expected size: (mapWidth × mapHeight) × inputDimension
- Uses row-major assignment to update all weight matrix entries
- Clear error messaging for length mismatches
This aligns with the UpdateParameters pattern used in other network classes (e.g., FeedForwardNeuralNetwork, SpikingLayer).
627-627: Improved input validation logic.The validation now explicitly checks:
- Input has exactly 1 dimension (
input.Shape.Length != 1)- That dimension equals
_inputDimension(input.Shape[0] != _inputDimension)This is more explicit and robust than array equality comparison, avoiding potential issues with shape array comparison semantics.
Also applies to: 670-670
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Optimizers/NonGradientBasedOptimizerBase.cs (1)
26-27: Consider interface segregation to avoid LSP violation.The current design overrides
Step()andCalculateUpdate()to throwNotSupportedException, which violates the Liskov Substitution Principle—code expecting anOptimizerBaseinstance cannot safely substitute aNonGradientBasedOptimizerBasewithout runtime exceptions.Consider splitting optimizer contracts into separate interfaces (e.g.,
IGradientBasedOptimizer,INonGradientBasedOptimizer) or using composition instead of inheritance. This would make the distinction between gradient-based and non-gradient-based optimizers type-safe at compile time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Optimizers/NonGradientBasedOptimizerBase.cs(1 hunks)
🔇 Additional comments (2)
src/Optimizers/NonGradientBasedOptimizerBase.cs (2)
28-38: LGTM!The constructor is appropriately protected for an abstract base class, accepts the expected parameters, and correctly delegates to the base constructor. The nullable model parameter is well-documented.
40-72: Optimize() is accessible and base methods are properly virtual—no issues found.Verification confirms the implementation is correct:
- Optimize() is abstract in OptimizerBase and implemented across all concrete optimizers (AdamOptimizer, ParticleSwarmOptimizer, GeneticAlgorithmOptimizer, etc.)
- Step() and CalculateUpdate(Dictionary<string, Vector>) are marked as virtual in OptimizerBase with guidance messages
- NonGradientBasedOptimizerBase correctly overrides these methods and throws NotSupportedException
The documentation is accurate and the override pattern is sound.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new base class NonGradientBasedOptimizerBase for non-gradient-based (derivative-free) optimization algorithms. This provides a common foundation for optimizers like Particle Swarm, Genetic Algorithms, Simulated Annealing, etc., similar to how GradientBasedOptimizerBase serves gradient-based optimizers.
- Adds
NonGradientBasedOptimizerBase<T, TInput, TOutput>as an abstract base class - Overrides
Step()andCalculateUpdate()methods to throwNotSupportedException - Provides comprehensive documentation explaining non-gradient-based optimization
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f4a25c0 to
83e8c4a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/NeuralNetworks/NEAT.cs (1)
618-626: Ensure deterministic, enabled-only parameter mappingIterating
bestGenome.Connectionsdirectly makes the weight assignment order depend on the historical mutation/crossover sequence. After any structural change, the same parameter vector will reshuffle across connections. You also force callers to supply values for disabled connections that inference never reads. That is already triggering length mismatches in practice and silently corrupts weights when it doesn’t. Please rebuild the mapping using only enabled connections ordered by a stable key (e.g.,Innovation) and validate the parameter count against that list before assignment.- if (parameters.Length != bestGenome.Connections.Count) + var enabledConnections = bestGenome.Connections + .Where(c => c.IsEnabled) + .OrderBy(c => c.Innovation) + .ToList(); + + if (parameters.Length != enabledConnections.Count) { - throw new ArgumentException($"Parameter vector length mismatch. Expected {bestGenome.Connections.Count} parameters but got {parameters.Length}.", nameof(parameters)); + throw new ArgumentException($"Parameter vector length mismatch. Expected {enabledConnections.Count} enabled parameters but got {parameters.Length}.", nameof(parameters)); } - for (int i = 0; i < bestGenome.Connections.Count; i++) + for (int i = 0; i < enabledConnections.Count; i++) { - bestGenome.Connections[i].Weight = parameters[i]; + enabledConnections[i].Weight = parameters[i]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Interpretability/BiasDetectorBase.cs(1 hunks)src/NeuralNetworks/EchoStateNetwork.cs(1 hunks)src/NeuralNetworks/NEAT.cs(2 hunks)src/NeuralNetworks/RestrictedBoltzmannMachine.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Interpretability/BiasDetectorBase.cs (1)
src/Helpers/MathHelper.cs (2)
INumericOperations(33-61)MathHelper(16-987)
🪛 GitHub Actions: Build
src/Interpretability/BiasDetectorBase.cs
[error] 24-24: The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
🪛 GitHub Check: Build All Frameworks
src/Interpretability/BiasDetectorBase.cs
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 135-135:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 135-135:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 135-135:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
🪛 GitHub Check: Publish Size Analysis
src/Interpretability/BiasDetectorBase.cs
[failure] 135-135:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
| public abstract class BiasDetectorBase<T> : IBiasDetector<T> | ||
| { | ||
| /// <summary> | ||
| /// Provides mathematical operations for the specific numeric type being used. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// <b>For Beginners:</b> This is a toolkit that helps perform math operations | ||
| /// regardless of whether we're using integers, decimals, doubles, etc. | ||
| /// | ||
| /// It allows the detector to work with different numeric types without | ||
| /// having to rewrite the math operations for each type. | ||
| /// </para> | ||
| /// </remarks> | ||
| protected readonly INumericOperations<T> _numOps; | ||
|
|
||
| /// <summary> | ||
| /// Indicates whether lower bias scores represent better (fairer) models. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// <b>For Beginners:</b> This tells us whether smaller numbers mean fairer models. | ||
| /// | ||
| /// For bias detection: | ||
| /// - Lower bias scores typically indicate fairer models (closer to equal treatment) | ||
| /// - A bias score of 0 would indicate perfect fairness | ||
| /// | ||
| /// This helps the system know how to compare different models for fairness. | ||
| /// </para> | ||
| /// </remarks> | ||
| protected readonly bool _isLowerBiasBetter; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the BiasDetectorBase class. | ||
| /// </summary> | ||
| /// <param name="isLowerBiasBetter">Indicates whether lower bias scores represent better (fairer) models.</param> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// <b>For Beginners:</b> This sets up the basic properties of the bias detector. | ||
| /// | ||
| /// Parameters: | ||
| /// - isLowerBiasBetter: Tells the system whether smaller numbers mean fairer models | ||
| /// (typically true for bias metrics, where 0 represents perfect fairness) | ||
| /// </para> | ||
| /// </remarks> | ||
| protected BiasDetectorBase(bool isLowerBiasBetter) | ||
| { | ||
| _isLowerBiasBetter = isLowerBiasBetter; | ||
| _numOps = MathHelper.GetNumericOperations<T>(); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Detects bias in model predictions by analyzing predictions across different groups. | ||
| /// </summary> | ||
| /// <param name="predictions">The model's predictions.</param> | ||
| /// <param name="sensitiveFeature">The sensitive feature (e.g., race, gender) used to identify groups.</param> | ||
| /// <param name="actualLabels">Optional actual labels for computing additional bias metrics.</param> | ||
| /// <returns>A result object containing bias detection metrics and analysis.</returns> | ||
| /// <exception cref="ArgumentNullException">Thrown when predictions or sensitiveFeature is null.</exception> | ||
| /// <exception cref="ArgumentException">Thrown when predictions and sensitiveFeature have different lengths.</exception> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// <b>For Beginners:</b> This method checks if your model treats different groups fairly. | ||
| /// | ||
| /// It works by: | ||
| /// 1. Validating that all required data is provided and properly formatted | ||
| /// 2. Calling the specific bias detection logic implemented by derived classes | ||
| /// 3. Returning detailed results about any bias found | ||
| /// | ||
| /// The method handles the common validation logic, while the specific bias detection | ||
| /// algorithm is defined in each detector that extends this base class. | ||
| /// </para> | ||
| /// </remarks> | ||
| public BiasDetectionResult<T> DetectBias(Vector<T> predictions, Vector<T> sensitiveFeature, Vector<T>? actualLabels = null) | ||
| { | ||
| if (predictions == null) | ||
| throw new ArgumentNullException(nameof(predictions)); | ||
|
|
||
| if (sensitiveFeature == null) | ||
| throw new ArgumentNullException(nameof(sensitiveFeature)); | ||
|
|
||
| if (predictions.Length != sensitiveFeature.Length) | ||
| throw new ArgumentException($"Predictions and sensitive feature must have the same length. Predictions: {predictions.Length}, Sensitive feature: {sensitiveFeature.Length}"); | ||
|
|
||
| if (actualLabels != null && predictions.Length != actualLabels.Length) | ||
| throw new ArgumentException($"Predictions and actual labels must have the same length. Predictions: {predictions.Length}, Actual labels: {actualLabels.Length}"); | ||
|
|
||
| return GetBiasDetectionResult(predictions, sensitiveFeature, actualLabels); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Abstract method that must be implemented by derived classes to perform specific bias detection logic. | ||
| /// </summary> | ||
| /// <param name="predictions">The model's predictions.</param> | ||
| /// <param name="sensitiveFeature">The sensitive feature used to identify groups.</param> | ||
| /// <param name="actualLabels">Optional actual labels for computing additional bias metrics.</param> | ||
| /// <returns>A result object containing bias detection metrics and analysis.</returns> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// <b>For Beginners:</b> This is a placeholder method that each specific detector must fill in. | ||
| /// | ||
| /// Think of it like a template that says "here's where you put your specific bias detection logic." | ||
| /// Each detector that extends this base class will provide its own implementation of this method, | ||
| /// defining exactly how it detects and measures bias. | ||
| /// | ||
| /// For example: | ||
| /// - A disparate impact detector would check if positive outcomes are equally distributed | ||
| /// - An equal opportunity detector would check if qualified individuals have equal chances | ||
| /// - A demographic parity detector would check for balanced outcomes across groups | ||
| /// </para> | ||
| /// </remarks> | ||
| protected abstract BiasDetectionResult<T> GetBiasDetectionResult( | ||
| Vector<T> predictions, | ||
| Vector<T> sensitiveFeature, | ||
| Vector<T>? actualLabels); | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether lower bias scores represent better (fairer) models. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// <b>For Beginners:</b> This property tells you whether smaller numbers mean fairer models. | ||
| /// | ||
| /// For most bias metrics: | ||
| /// - IsLowerBiasBetter is true (0 bias means perfect fairness) | ||
| /// - Lower values indicate the model treats different groups more equally | ||
| /// | ||
| /// This helps you interpret the scores correctly when comparing different models. | ||
| /// </para> | ||
| /// </remarks> | ||
| public bool IsLowerBiasBetter => _isLowerBiasBetter; | ||
|
|
||
| /// <summary> | ||
| /// Determines whether a new bias score represents better (fairer) performance than the current best score. | ||
| /// </summary> | ||
| /// <param name="currentBias">The current bias score to evaluate.</param> | ||
| /// <param name="bestBias">The best (fairest) bias score found so far.</param> | ||
| /// <returns>True if the current bias score is better (fairer) than the best bias score; otherwise, false.</returns> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// <b>For Beginners:</b> This method compares two bias scores and tells you which model is fairer. | ||
| /// | ||
| /// It takes into account whether higher scores are better or lower scores are better: | ||
| /// - If lower scores are better (typical for bias), it returns true when the new score is lower | ||
| /// - If higher scores are better (less common), it returns true when the new score is higher | ||
| /// | ||
| /// This is particularly useful when: | ||
| /// - Selecting the fairest model from multiple options | ||
| /// - Deciding whether model changes improved fairness | ||
| /// - Tracking fairness improvements during model development | ||
| /// </para> | ||
| /// </remarks> | ||
| public bool IsBetterBiasScore(T currentBias, T bestBias) | ||
| { | ||
| return _isLowerBiasBetter | ||
| ? _numOps.LessThan(currentBias, bestBias) | ||
| : _numOps.GreaterThan(currentBias, bestBias); | ||
| } | ||
| } |
There was a problem hiding this comment.
Restore missing bias-detection contracts so the build succeeds
This file depends on IBiasDetector<T> and BiasDetectionResult<T>, but neither type is present in the solution. Current builds fail with “type or namespace … could not be found.” Please add the corresponding interface/result definitions (or adjust the namespace/imports if they already exist elsewhere) before landing this change; otherwise this class will not compile.
🧰 Tools
🪛 GitHub Actions: Build
[error] 24-24: The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
🪛 GitHub Check: Build All Frameworks
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 135-135:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 135-135:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 135-135:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
🪛 GitHub Check: Publish Size Analysis
[failure] 135-135:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 97-97:
The type or namespace name 'BiasDetectionResult<>' could not be found (are you missing a using directive or an assembly reference?)
[failure] 24-24:
The type or namespace name 'IBiasDetector<>' could not be found (are you missing a using directive or an assembly reference?)
🤖 Prompt for AI Agents
In src/Interpretability/BiasDetectorBase.cs around lines 24 to 182, the build
fails because the referenced types IBiasDetector<T> and BiasDetectionResult<T>
are missing; add definitions for them (public, same namespace as
BiasDetectorBase or import the correct namespace) so the file can compile:
define IBiasDetector<T> with the DetectBias signature and
IsLowerBiasBetter/IsBetterBiasScore members used by implementations as
appropriate, and define BiasDetectionResult<T> as a simple public result
container type (with whatever properties the rest of the codebase expects, e.g.,
numeric score, per-group metrics and optional metadata), ensure both are generic
T, placed in the project’s Interpretability folder or a shared Contracts
namespace, and update any using statements or project files so the new types are
discoverable by BiasDetectorBase.
Implemented UpdateParameters method for six neural network classes: - EchoStateNetwork: updates output weights and bias - ExtremeLearningMachine: updates output layer parameters - HopfieldNetwork: updates weight matrix while preserving diagonal zeros - NEAT: updates weights of best genome connections - RestrictedBoltzmannMachine: updates weights and biases - SelfOrganizingMap: updates weight matrix All implementations include parameter length validation and error handling. Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
b5e91e0 to
c2299ed
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/NeuralNetworks/RestrictedBoltzmannMachine.cs (1)
430-450: Update the XML doc comment to reflect the new implementation.
The summary/remarks still claim this method throwsNotImplementedException, but the body now performs a full parameter update. Please revise the documentation so callers aren't misled about the supported behavior.
♻️ Duplicate comments (1)
src/NeuralNetworks/NEAT.cs (1)
611-626: Update only enabled connections in deterministic order.This implementation still has the critical issues flagged in the previous review:
- Non-deterministic mapping: Connections are not sorted, so parameter-to-connection mapping varies by insertion order.
- Enabled/disabled mismatch: This updates all connections, but
ActivateGenome(line 802) skips disabled connections. The parameter vector should only represent enabled connections.Refer to the previous review comment for the complete solution that filters to enabled connections, sorts by Innovation ID, and validates against the enabled connection count.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/NeuralNetworks/EchoStateNetwork.cs(1 hunks)src/NeuralNetworks/NEAT.cs(2 hunks)src/NeuralNetworks/RestrictedBoltzmannMachine.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/NeuralNetworks/EchoStateNetwork.cs
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Franklin Moormann <cheatcountry@gmail.com>
…tzmannMachine Replace garbled character with proper × (multiplication) symbol in documentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary
This PR implements the UpdateParameters method for 6 neural network classes, enabling proper parameter updates during training. Previously, these methods either threw NotImplementedException or had no-op implementations.
Changes
Neural Network UpdateParameters Implementations
EchoStateNetwork.cs (+25 lines)
ExtremeLearningMachine.cs (+18 lines)
HopfieldNetwork.cs (+27 lines)
NEAT.cs (+19 lines)
RestrictedBoltzmannMachine.cs (+32 lines)
SelfOrganizingMap.cs (+21 lines)
Verification Documentation
Added VERIFICATION_US-IF-001.md documenting the completion status and coding standards adherence for IFullModel implementations.
Coding Standards Compliance
All implementations follow project standards:
requiredkeyword (net462 compatibility)objectstorageTest Plan
Related Issue
Addresses US-IF-001: Complete IFullModel and ICloneable implementations
🤖 Generated with Claude Code