add option to change subsolver in AL#310
add option to change subsolver in AL#310MaxenceGollier wants to merge 1 commit intoJuliaSmoothOptimizers:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for selecting which subproblem solver the Augmented Lagrangian (AL) algorithm uses, and expands tests to cover running AL with different subsolvers.
Changes:
- Add a
subsolverkeyword pathway for equality-constrained AL runs (and update the AL docstring example accordingly). - Update
ALSolverconstruction to accept a configurablesubsolver. - Extend
test_AL.jlto exercise AL withR2SolverandR2NSolver.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/AL_alg.jl |
Introduces configurable AL subsolver selection and updates the public docstring/example. |
test/test_AL.jl |
Adds regression coverage for selecting AL subsolvers (R2Solver, R2NSolver). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| has_bnds = has_bounds(nlp) | ||
| sub_model = AugLagModel(nlp, V(undef, ncon), T(0), x, T(0), cx) | ||
| sub_problem = RegularizedNLPModel(sub_model, reg_nlp.h, reg_nlp.selected) | ||
| sub_solver = R2Solver(reg_nlp; kwargs...) | ||
| sub_solver = subsolver(reg_nlp; kwargs...) | ||
| sub_stats = RegularizedExecutionStats(sub_problem) |
There was a problem hiding this comment.
ALSolver builds sub_problem as an augmented-Lagrangian RegularizedNLPModel, but the sub_solver is constructed using reg_nlp (the original problem). For subsolvers that precompute model-dependent operators/state (e.g. R2NSolver builds a Hessian operator in its constructor), this means the subsolver is initialized for the wrong model and then used to solve sub_problem, which can lead to incorrect steps or failures. Construct the subsolver from sub_problem instead (and forward any subsolver-specific kwargs there) so the solver is consistent with the subproblem it will be asked to solve.
| - `max_eval::Int = -1`: maximum number of evaluation of the objective function (negative number means unlimited); | ||
| - `subsolver::AbstractOptimizationSolver = has_bounds(nlp) ? TR : R2`: the procedure used to compute a step (e.g. `PG`, `R2`, `TR` or `TRDH`); | ||
| - `subsolver::AbstractOptimizationSolver = R2Solver`: the procedure used to compute a step (e.g. `R2Solver`, `R2NSolver`, `R2DHSolver`, `TRSolver` or `TRDHSolver`); | ||
| - `subsolver_logger::AbstractLogger`: a logger to pass to the subproblem solver; |
There was a problem hiding this comment.
The docstring lists subsolver::AbstractOptimizationSolver = R2Solver, but subsolver is used as a constructor/callable (e.g. subsolver(reg_nlp; ...)) rather than an AbstractOptimizationSolver instance. To avoid confusing users (and to match how other solvers document subsolver), update the doc to reflect that this expects a solver type/constructor (e.g. Type{<:AbstractOptimizationSolver} or a callable that returns a solver) and clarify the default accordingly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #310 +/- ##
===========================================
+ Coverage 61.53% 83.91% +22.38%
===========================================
Files 11 13 +2
Lines 1292 1654 +362
===========================================
+ Hits 795 1388 +593
+ Misses 497 266 -231 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.