Remove R2NModels#311
Conversation
There was a problem hiding this comment.
Pull request overview
This PR partially addresses issue #220 by removing the custom R2NModel type and replacing it with QuadraticModel from the QuadraticModels.jl package. The R2NModel was a bespoke NLP model representing the smooth quadratic R2N subproblem; swapping it for QuadraticModel reduces the amount of custom code in the repository and leverages an actively maintained external dependency.
Changes:
- Removes
src/R2NModel.jl(the custom model) and itsincludeinRegularizedOptimization.jl - Replaces
R2NModelconstruction and field access withQuadraticModelanddata.H/data.σinR2N.jlandTR_alg.jl; adds a preallocated work vectorv1to both solver structs - Adds
QuadraticModelsas a dependency inProject.toml
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/R2NModel.jl |
File deleted — custom quadratic subproblem model removed |
src/RegularizedOptimization.jl |
Added QuadraticModels to using list; removed include("R2NModel.jl") |
src/R2N.jl |
Replaces R2NModel with QuadraticModel; updates field accesses to data.H/data.σ; adds v1 work vector; rewrites mk lambda to avoid skip_sigma |
src/TR_alg.jl |
Same R2NModel → QuadraticModel substitution; adds v1 work vector; updates data.H accesses |
Project.toml |
Adds QuadraticModels dependency with compat constraint "0.9.15" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #311 +/- ##
===========================================
+ Coverage 61.53% 83.38% +21.85%
===========================================
Files 11 12 +1
Lines 1292 1643 +351
===========================================
+ Hits 795 1370 +575
+ Misses 497 273 -224 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cd01951
into
JuliaSmoothOptimizers:master
partially solves #220.