Fix issue 398 in AiDotNet #445
Conversation
This commit implements a comprehensive Federated Learning framework for AiDotNet, addressing all requirements from Issue #398. Core Algorithms Implemented: - FedAvg (Federated Averaging): Weighted averaging of client updates - FedProx (Federated Proximal): Handles system heterogeneity with proximal terms - FedBN (Federated Batch Normalization): Special handling for BN layers Privacy Features: - Gaussian Differential Privacy: (ε, δ)-DP with gradient clipping - Secure Aggregation: Cryptographic protocol for private aggregation Personalization: - Personalized Federated Learning: Client-specific layers + global layers - Flexible layer selection strategies - Model split statistics Interfaces & Configuration: - IFederatedTrainer: Main federated learning orchestration - IAggregationStrategy: Pluggable aggregation algorithms - IPrivacyMechanism: Privacy-preserving techniques - IClientModel: Client-side model operations - FederatedLearningOptions: Comprehensive configuration - FederatedLearningMetadata: Training metrics and statistics Testing: - Unit tests for FedAvg aggregation - Unit tests for Differential Privacy Documentation: - Extensive XML documentation with beginner-friendly explanations - Comprehensive README with usage examples and references - Mathematical formulations and research references All success criteria from Issue #398 have been met: ✓ Core algorithms (FedAvg, FedProx, FedBN) ✓ Privacy features (Differential Privacy, Secure Aggregation) ✓ Personalization (PFL with layer-wise strategies) ✓ Clean architecture with interfaces ✓ Comprehensive configuration and metadata ✓ Unit tests for core functionality
|
Warning Rate limit exceeded@ooples has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 42 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 (15)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive Federated Learning framework for AiDotNet, introducing core algorithms (FedAvg, FedProx, FedBN), privacy mechanisms (Gaussian Differential Privacy, Secure Aggregation), and personalization features.
Key changes:
- Core aggregation strategies: FedAvg, FedProx, and FedBN with weighted averaging and specialized handling for heterogeneous data
- Privacy-preserving mechanisms: Gaussian differential privacy with gradient clipping and secure aggregation using cryptographic masking
- Personalization support: Layer-wise personalization enabling client-specific model components
- Configuration and metadata tracking: Comprehensive options for training parameters and detailed metrics collection
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/FederatedLearning/Aggregators/FedAvgAggregationStrategy.cs | Implements weighted averaging aggregation for standard federated learning |
| src/FederatedLearning/Aggregators/FedProxAggregationStrategy.cs | Adds proximal term constraint for handling heterogeneous client capabilities |
| src/FederatedLearning/Aggregators/FedBNAggregationStrategy.cs | Implements selective aggregation keeping batch normalization layers local |
| src/FederatedLearning/Privacy/GaussianDifferentialPrivacy.cs | Implements (ε, δ)-differential privacy with gradient clipping and Gaussian noise |
| src/FederatedLearning/Privacy/SecureAggregation.cs | Implements cryptographic secure aggregation using pairwise secret masking |
| src/FederatedLearning/Personalization/PersonalizedFederatedLearning.cs | Enables client-specific layers while maintaining global shared parameters |
| src/Interfaces/IAggregationStrategy.cs | Defines interface for combining client model updates |
| src/Interfaces/IPrivacyMechanism.cs | Defines interface for privacy-preserving techniques |
| src/Interfaces/IFederatedTrainer.cs | Defines core federated learning training orchestration |
| src/Interfaces/IClientModel.cs | Defines client-side model operations and update management |
| src/Models/Options/FederatedLearningOptions.cs | Configuration options for all federated learning parameters |
| src/Models/FederatedLearningMetadata.cs | Metadata tracking for training progress and metrics |
| src/FederatedLearning/README.md | Documentation covering architecture, usage examples, and references |
| tests/AiDotNet.Tests/FederatedLearning/GaussianDifferentialPrivacyTests.cs | Unit tests for differential privacy mechanism |
| tests/AiDotNet.Tests/FederatedLearning/FedAvgAggregationStrategyTests.cs | Unit tests for FedAvg aggregation strategy |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Client 1 masked: [0.4, 0.0, 0.85] = [0.5, -0.3, 0.8] + [-0.1, 0.3, 0.05] | ||
| /// Client 2 masked: [0.7, 0.7, 1.05] = [0.6, 0.4, 1.1] + [0.1, -0.3, -0.05] | ||
| /// | ||
| /// Sum of masked: [1.1, 0.7, 1.9] |
There was a problem hiding this comment.
The calculation in the comment has an arithmetic error. The middle value should be -0.3 + 0.4 = 0.1, not 0.7 as suggested by the sum [1.1, 0.7, 1.9] shown on line 262. The line should show 'True sum: [0.5, -0.3, 0.8] + [0.6, 0.4, 1.1] = [1.1, 0.1, 1.9]' which is already correct, but conflicts with the 'Sum of masked' value on line 262.
| /// Sum of masked: [1.1, 0.7, 1.9] | |
| /// Sum of masked: [1.1, 0.1, 1.9] |
| /// | ||
| /// For example: | ||
| /// - Client has 1000 samples, BatchSize = 32 | ||
| /// - Data is split into 32 batches of ~31 samples each |
There was a problem hiding this comment.
The calculation in the comment is incorrect. With 1000 samples and batch size 32, there would be 1000/32 = 31.25 batches, which rounds to approximately 32 batches of 31-32 samples each, not '32 batches of ~31 samples'. The comment should say 'Data is split into approximately 32 batches of 31-32 samples each' or 'Data is processed in batches of 32 samples (approximately 31 batches total)'.
| /// - Data is split into 32 batches of ~31 samples each | |
| /// - Data is split into approximately 32 batches of 31-32 samples each |
| foreach (var pattern in _batchNormLayerPatterns) | ||
| { | ||
| if (lowerLayerName.Contains(pattern.ToLowerInvariant())) | ||
| { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var pattern in customPatterns) | ||
| { | ||
| if (layerName.Contains(pattern, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| _personalizedLayers.Add(layerName); | ||
| break; | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var pattern in customPatterns) | |
| { | |
| if (layerName.Contains(pattern, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| _personalizedLayers.Add(layerName); | |
| break; | |
| } | |
| if (customPatterns.Any(pattern => layerName.Contains(pattern, StringComparison.OrdinalIgnoreCase))) | |
| { | |
| _personalizedLayers.Add(layerName); |
| foreach (var clientId in maskedUpdates.Keys) | ||
| { | ||
| var maskedUpdate = maskedUpdates[clientId]; | ||
|
|
||
| foreach (var layerName in maskedUpdate.Keys) | ||
| { | ||
| var maskedParams = maskedUpdate[layerName]; | ||
| var aggregatedParams = aggregatedUpdate[layerName]; | ||
|
|
||
| for (int i = 0; i < maskedParams.Length; i++) | ||
| { | ||
| double currentValue = Convert.ToDouble(aggregatedParams[i]); | ||
| double maskedValue = Convert.ToDouble(maskedParams[i]); | ||
| aggregatedParams[i] = (T)Convert.ChangeType(currentValue + maskedValue, typeof(T)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var layerName in aggregatedUpdate.Keys) | ||
| { | ||
| var aggregatedParams = aggregatedUpdate[layerName]; | ||
|
|
||
| for (int i = 0; i < aggregatedParams.Length; i++) | ||
| { | ||
| double value = Convert.ToDouble(aggregatedParams[i]); | ||
| aggregatedParams[i] = (T)Convert.ChangeType(value / totalWeight, typeof(T)); | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var layerName in noisyModel.Keys) | ||
| { | ||
| var parameters = noisyModel[layerName]; | ||
| for (int i = 0; i < parameters.Length; i++) | ||
| { | ||
| double value = Convert.ToDouble(parameters[i]); | ||
| parameters[i] = (T)Convert.ChangeType(value * scaleFactor, typeof(T)); | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var layerName in noisyModel.Keys) | ||
| { | ||
| var parameters = noisyModel[layerName]; | ||
| for (int i = 0; i < parameters.Length; i++) | ||
| { | ||
| double value = Convert.ToDouble(parameters[i]); | ||
| double noise = GenerateGaussianNoise(0.0, noiseSigma); | ||
| parameters[i] = (T)Convert.ChangeType(value + noise, typeof(T)); | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var param in layer) | ||
| { | ||
| double value = Convert.ToDouble(param); | ||
| sumOfSquares += value * value; | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
This commit implements a comprehensive Federated Learning framework for AiDotNet, addressing all requirements from Issue #398.
Core Algorithms Implemented:
Privacy Features:
Personalization:
Interfaces & Configuration:
Testing:
Documentation:
All success criteria from Issue #398 have been met: ✓ Core algorithms (FedAvg, FedProx, FedBN)
✓ Privacy features (Differential Privacy, Secure Aggregation) ✓ Personalization (PFL with layer-wise strategies) ✓ Clean architecture with interfaces
✓ Comprehensive configuration and metadata
✓ Unit tests for core functionality
User Story / Context
merge-dev2-to-masterSummary
Verification
Copilot Review Loop (Outcome-Based)
Record counts before/after your last push:
Files Modified
Notes