fix: enhance disk space cleanup and remove duplicate build in docs deployment - #740
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughCI workflow optimization adds disk-cleanup steps to the sonarcloud workflow for managing build artifacts and freeing space before deployment phases. Documentation navigation enhanced with a new "Try It" playground entry positioned between Getting Started and Tutorials sections. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Optimizers/ParticleSwarmOptimizer.cs (1)
98-130: Add guard against empty swarm before accessing swarm[0].
The code at line 116 accessesswarm[0]directly to initializeglobalBest, but there is no validation ensuringSwarmSize > 0. Even though line 98 uses a guard pattern (swarm.Count > 0 ? swarm[0].ParameterCount : ...), this protection is absent at the point wherefirstStepDatais created. IfSwarmSize = 0, the initialization loop never executes andswarm[0]will throw anIndexOutOfRangeException. Add a validation check or guard to ensure the swarm is non-empty before line 116.src/Regression/RegressionBase.cs (1)
523-557: Guard against empty parameter vectors when intercept is enabled.With
UseIntercept = trueandparameters.Length == 0,coefficientCountbecomes-1, leading to invalid vector allocation.🛠️ Proposed fix
if (coefficientCount == 0) { + if (Options.UseIntercept && parameters.Length == 0) + { + throw new ArgumentException("Parameters must include an intercept value.", nameof(parameters)); + } // For untrained model, infer coefficient count from parameters // If UseIntercept: coeffCount = parameters.Length - 1 // If not UseIntercept: coeffCount = parameters.Length coefficientCount = Options.UseIntercept ? parameters.Length - 1 : parameters.Length; }
🤖 Fix all issues with AI agents
In @.github/workflows/sonarcloud.yml:
- Around line 658-672: In the "Free up disk space before upload" step the
unconditional line sudo rm -rf /tmp/* is risky; remove that command or replace
it with a scoped, safer cleanup (for example, only remove build-related
temporary files or files owned by the current runner that match a known pattern
or age) to avoid breaking downstream actions; specifically update the step to
stop running sudo rm -rf /tmp/* (or swap it for a targeted cleanup strategy) so
actions/upload-pages-artifact and actions/deploy-pages aren’t impacted.
In `@src/Optimizers/AdaDeltaOptimizer.cs`:
- Around line 321-337: The CPU path in UpdateParameters multiplies
updateUnscaled by CurrentLearningRate but the GPU path (UpdateParametersGpu)
calls backend.AdadeltaUpdate with no LR, causing divergence when LR != 1; modify
either the GPU kernel signature (backend.AdadeltaUpdate) to accept a
learningRate parameter and use CurrentLearningRate inside the native update, or
pre-scale the update/parameters on the managed side before calling
backend.AdadeltaUpdate so the GPU receives the same scaledUpdate as the CPU;
update references to CurrentLearningRate, AdadeltaUpdate, UpdateParameters and
UpdateParametersGpu and ensure _accumulatedSquaredUpdates behavior remains
identical between paths.
In `@src/Optimizers/BayesianOptimizer.cs`:
- Around line 105-125: The code assumes _options.InitialSamples > 0 and writes
to _sampledPoints[0,*]; add a guard at the start of this initialization block to
validate _options.InitialSamples (check <= 0) and either throw a clear
ArgumentException or return/skip the sampling flow early; ensure you validate
before calling InitializeRandomSolution or allocating
_sampledPoints/_sampledValues so functions like InitializeRandomSolution,
EvaluateSolution and UpdateBestSolution are not invoked when InitialSamples is
non-positive.
In `@src/Optimizers/BFGSOptimizer.cs`:
- Around line 277-366: When reinitializing _inverseHessian in UpdateParameters
(the block checking "_inverseHessian is null || _inverseHessian.Rows !=
parameters.Length"), also reset _previousParameters and _previousGradient to
null to avoid size mismatches; locate the reset logic in the UpdateParameters
method and clear those fields immediately after creating the identity inverse
Hessian so future BFGS updates (s = parameters - _previousParameters, y =
clippedGradient - _previousGradient) don't use incompatible-sized vectors.
In `@src/Optimizers/DFPOptimizer.cs`:
- Around line 304-369: The optimizer stores previous state in
_previousParameters used by UpdateParameters but that field is not persisted
across checkpoints; update the DFPOptimizer's serialization and deserialization
logic (e.g., the class's SaveState/LoadState, GetObjectData/constructor for
ISerializable, or whatever checkpointing methods exist) to include
_previousParameters so it is restored on resume, or explicitly clear/reset
_previousParameters on deserialize if you intend to drop curvature; reference
the _previousParameters field and the UpdateParameters method when making the
change so curvature continuity is preserved or intentionally reset.
In `@src/Optimizers/FTRLOptimizer.cs`:
- Around line 130-182: UpdateParameters and UpdateSolution compute the FTRL
denominator differently (UpdateParameters uses lambda2 + (sqrt(n) + beta)/alpha
while UpdateSolution uses lambda2*(1+beta) + sqrt(n)/alpha) causing inconsistent
updates; pick the correct math and make both use the same expression by either
changing the denominator in UpdateParameters to match UpdateSolution (or vice
versa) and extracting the shared logic into a single helper (e.g.,
ComputeFtrlDenominator or GetDenominator) that takes sqrtN, alpha, beta, lambda2
and is called from UpdateParameters and UpdateSolution so both use the identical
formula.
- Around line 103-107: InitializeAdaptiveParameters currently sets _z and _n to
null which breaks Optimize/UpdateSolution that expect pre-allocated arrays;
instead, in InitializeAdaptiveParameters (called by Optimize), preserve existing
_z/_n if already allocated or reinitialize them to zero-sized arrays of the
correct length (or zero their contents) rather than nulling them; ensure
CurrentLearningRate and _t are still reset as shown, but remove the "_z = null;
_n = null;" behavior or replace it with logic that checks for non-null _z/_n and
zeroes or re-allocates them so UpdateSolution's dereferences of _z!/_n! remain
safe.
In `@src/Optimizers/RootMeanSquarePropagationOptimizer.cs`:
- Around line 244-248: UpdateSolution currently assumes _squaredGradient length
matches the incoming parameter/solution vectors which can cause exceptions after
Reset()/Deserialize() or model shape changes; mirror the guard used in
UpdateParameters by checking if (_squaredGradient.Length != parameters.Length)
(or the corresponding solution length) and reinitialize _squaredGradient = new
Vector<T>(parameters.Length) before performing vector ops so sizes are
consistent with UpdateParameters and safe across deserialization/reset flows.
In `@src/Optimizers/TrustRegionOptimizer.cs`:
- Around line 57-66: The checkpointing currently omits the trust‑region state so
resumed runs diverge: include _trustRegionPreviousParameters and
_trustRegionPreviousGradient in the optimizer's Serialize and Deserialize logic
by writing them into the checkpoint payload and restoring them on load (handle
nulls and add backwards-compatible versioning/optional fields so older
checkpoints still load); update the Serialize method to emit the two vectors and
the Deserialize method to read and assign them back to
_trustRegionPreviousParameters and _trustRegionPreviousGradient (ensure type T
and vector shape validation during restore).
In `@src/Regression/RegressionBase.cs`:
- Around line 655-683: In SetParameters, add the same empty-parameters guard to
avoid computing a negative coefficientCount when parameters.Length == 0 and
Options.UseIntercept is true: if parameters.Length == 0, set coefficientCount =
0, resize Coefficients = new Vector<T>(0) (or ensure Coefficients has length 0),
and set Intercept = default(T) only if Options.UseIntercept (or leave it
unchanged if intended), then skip the parameter copy/indexing and the
expectedCount check that assumes parameters exist; reference SetParameters,
Coefficients, Options.UseIntercept, Intercept, and parameters to locate where to
insert this guard.
🧹 Nitpick comments (8)
src/GaussianProcesses/StandardGaussianProcess.cs (1)
113-121: Consider making jitter magnitude configurable or scale-aware.A fixed
1e-6can be too small for large-kernel scales or too large for tightly normalized data. Exposing a configurable jitter (e.g., constructor optional parameter) or deriving it from the diagonal scale would make this more robust across datasets.src/Optimizers/TrustRegionOptimizer.cs (1)
156-186: Consider applying radius adaptation before computing the step.The radius update is based on the previous step’s outcome, but it executes after the new step is computed, so the current step still uses the old radius. If you want the latest success/failure to influence the current step, move this block before the Cauchy step (or document the intentional lag).
♻️ Proposed refactor (move adaptation block before step computation)
- // Cauchy point computation: + // Adapt trust region radius based on gradient change (proxy for step success) + if (_trustRegionPreviousGradient is not null && _trustRegionPreviousParameters is not null) + { + // Compute the gradient difference to estimate curvature + var gradDiff = (Vector<T>)Engine.Subtract(gradient, _trustRegionPreviousGradient); + var paramDiff = (Vector<T>)Engine.Subtract(parameters, _trustRegionPreviousParameters); + var paramDiffNorm = paramDiff.Norm(); + + if (NumOps.GreaterThan(paramDiffNorm, NumOps.FromDouble(1e-10))) + { + // If gradient is decreasing in magnitude, step was likely successful + var prevGradNorm = _trustRegionPreviousGradient.Norm(); + + if (NumOps.LessThan(gradientNorm, prevGradNorm)) + { + // Successful step - expand trust region + _trustRegionRadius = NumOps.Multiply(_trustRegionRadius, NumOps.FromDouble(_options.ExpansionFactor)); + } + else + { + // Unsuccessful step - contract trust region + _trustRegionRadius = NumOps.Multiply(_trustRegionRadius, NumOps.FromDouble(_options.ContractionFactor)); + } + + // Clamp trust region radius + _trustRegionRadius = MathHelper.Clamp( + _trustRegionRadius, + NumOps.FromDouble(_options.MinTrustRegionRadius), + NumOps.FromDouble(_options.MaxTrustRegionRadius)); + } + } + + // Cauchy point computation: // The optimal step along the steepest descent direction is: // alpha = min(trust_radius / ||g||, ||g||^2 / (g^T B g)) // For gradient-only case (assuming B = I), this simplifies to: // alpha = min(trust_radius / ||g||, 1) // Then: step = -alpha * g @@ - // Adapt trust region radius based on gradient change (proxy for step success) - if (_trustRegionPreviousGradient is not null && _trustRegionPreviousParameters is not null) - { - // Compute the gradient difference to estimate curvature - var gradDiff = (Vector<T>)Engine.Subtract(gradient, _trustRegionPreviousGradient); - var paramDiff = (Vector<T>)Engine.Subtract(parameters, _trustRegionPreviousParameters); - var paramDiffNorm = paramDiff.Norm(); - - if (NumOps.GreaterThan(paramDiffNorm, NumOps.FromDouble(1e-10))) - { - // If gradient is decreasing in magnitude, step was likely successful - var prevGradNorm = _trustRegionPreviousGradient.Norm(); - - if (NumOps.LessThan(gradientNorm, prevGradNorm)) - { - // Successful step - expand trust region - _trustRegionRadius = NumOps.Multiply(_trustRegionRadius, NumOps.FromDouble(_options.ExpansionFactor)); - } - else - { - // Unsuccessful step - contract trust region - _trustRegionRadius = NumOps.Multiply(_trustRegionRadius, NumOps.FromDouble(_options.ContractionFactor)); - } - - // Clamp trust region radius - _trustRegionRadius = MathHelper.Clamp( - _trustRegionRadius, - NumOps.FromDouble(_options.MinTrustRegionRadius), - NumOps.FromDouble(_options.MaxTrustRegionRadius)); - } - }src/Optimizers/OptimizerBase.cs (1)
1259-1262: Redundant empty matrix check.This validation is redundant since lines 1226-1229 already throw
ArgumentExceptionifmatrix.Rows == 0. The code at line 1263 (matrix[0, col]) is only reachable whenmatrix.Rows > 0.♻️ Suggested removal
for (int col = 0; col < features; col++) { - // Safe to access matrix[0, col] after validation above - if (matrix.Rows == 0) - { - throw new ArgumentException("Matrix cannot be empty", nameof(matrix)); - } T initialValue = matrix[0, col];src/Models/Options/LBFGSOptimizerOptions.cs (1)
98-101: Outdated documentation: still references "new" keyword.The XML documentation on lines 98-99 states "This property uses the 'new' keyword" but the property now uses
override. Update the documentation to reflect the change:- /// Note: This property uses the "new" keyword because it overrides the base class property with a - /// different default value that's more appropriate for L-BFGS.</para> + /// Note: This property overrides the base class property with a different default value + /// that's more appropriate for L-BFGS.</para>The change from
newtooverrideis correct and enables proper polymorphic behavior.src/Optimizers/AdagradOptimizer.cs (1)
483-499: Deserialization looks correct; minor naming nit.The deserialization logic properly reconstructs the optimizer state. One small inconsistency:
data_arr(line 489) uses snake_case while the codebase appears to use camelCase for local variables.Optional naming fix
- T[] data_arr = new T[length]; + T[] dataArr = new T[length]; for (int i = 0; i < length; i++) { - data_arr[i] = NumOps.FromDouble(reader.ReadDouble()); + dataArr[i] = NumOps.FromDouble(reader.ReadDouble()); } - _accumulatedSquaredGradients = new Vector<T>(data_arr); + _accumulatedSquaredGradients = new Vector<T>(dataArr);src/Optimizers/NadamOptimizer.cs (2)
513-519: Consider serializing_previousTfor complete state restoration.The time step
_tis serialized (line 513), but_previousT(used byReverseUpdate) is not. IfReverseUpdateis called after deserialization without first callingUpdateParameters,_previousTwill have its default value (0), which may not match the restored_previousM/_previousVstate.Suggested fix
writer.Write(_t); + writer.Write(_previousT); + // Serialize state vectors SerializeVector(writer, _m);And in
Deserialize:_t = reader.ReadInt32(); + _previousT = reader.ReadInt32(); + // Deserialize state vectors _m = DeserializeVector(reader);
525-553: Helper methods duplicate code from AMSGradOptimizer.
SerializeVectorandDeserializeVectorare identical to those inAMSGradOptimizer. Consider extracting these to a shared base class or utility class to reduce duplication.tests/AiDotNet.Tests/IntegrationTests/Optimizers/MetaheuristicOptimizerIntegrationTests.cs (1)
10-22: Consider tagging these as integration tests for filtering.There are many stochastic optimizers with non-trivial iteration counts. A category/trait would let CI split or filter if runtime becomes an issue.
![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 fixes "No space left on device" errors during documentation deployment while also adding comprehensive optimizer integration tests and fixing several optimizer-related bugs.
Changes:
- Enhanced disk space cleanup in CI/CD pipeline (~20GB freed through removal of unused tools and runtimes)
- Removed duplicate solution build in docs job (DocFX extracts metadata from source, not binaries)
- Added 5200+ lines of comprehensive integration tests for gradient-based and metaheuristic optimizers
- Implemented missing
UpdateParametersmethods for several optimizers (BFGS, DFP, L-BFGS, TrustRegion, FTRL) - Fixed parameter dimension mismatches in optimizers (CMA-ES, Bayesian, Powell, ParticleSwarm, DifferentialEvolution, AntColony)
- Enhanced serialization/deserialization for optimizer state vectors
- Fixed untrained model parameter handling in RegressionBase
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/sonarcloud.yml |
Enhanced disk cleanup (~20GB), removed duplicate build, added intermediate cleanup |
toc.yml |
Added "Try It" playground navigation entry |
MetaheuristicOptimizerIntegrationTests.cs |
New comprehensive tests for GA, PSO, DE, SA, ACO, Tabu, CMA-ES, Bayesian, Nelder-Mead, Powell, Normal optimizers |
GradientBasedOptimizerIntegrationTests.cs |
New comprehensive tests for SGD, Momentum, Adam, RMSProp, Adagrad, AdaDelta, NAG, AdaMax, AMSGrad, Nadam, Lion, L-BFGS, Newton, AdamW, FTRL, LAMB, LARS, BFGS, DFP, ConjugateGradient, TrustRegion, Levenberg-Marquardt, etc. |
RegressionBase.cs |
Fixed WithParameters/SetParameters to handle untrained models correctly |
| Multiple optimizer files | Implemented missing UpdateParameters methods, fixed serialization, added state management |
| Multiple option files | Changed new to override for InitialLearningRate property |
StandardGeneticAlgorithm.cs |
Fixed parameter initialization for untrained models |
GeneticBase.cs |
Added training input storage for proper dimension inference |
StandardGaussianProcess.cs |
Added jitter term for numerical stability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Enhance disk cleanup to free ~15GB (Android SDK, Haskell, language runtimes, docker images, apt cache) - Remove duplicate full solution build - DocFX extracts metadata from source code, not compiled binaries, so only restore is needed - Add intermediate cleanup before upload to free build artifacts - Add "Try It" link to main navigation for the playground Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
9a00982 to
e678979
Compare
|
Summary
Fixes the "No space left on device" error during documentation deployment by:
Aggressive disk space cleanup (~20GB freed):
Remove duplicate full solution build:
Add intermediate cleanup before upload:
Test plan
🤖 Generated with Claude Code