perform QN updates in callback#358
Conversation
Performing QN updates in a callback allows for customized updates instead of generic updates. A default callback with the expected update is provided.
dpo
requested review from
Copilot and
tmigot
and removed request for
Copilot
There was a problem hiding this comment.
Pull request overview
This PR moves quasi-Newton Hessian-approximation updates out of the TRUNK/TRON solver loops and into a dedicated per-iteration callback, enabling users to customize the update logic while providing a default implementation.
Changes:
- Added a default
callback_quasi_newton(model, solver, stats)implementation to perform quasi-Newton updates. - Updated
trunkandtronto accept and invokecallback_quasi_newton, and to set step acceptance/rejection status instats. - Updated documentation to describe the new callback and renamed the callback section to “Callbacks”.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
src/trunk.jl |
Adds callback_quasi_newton plumbing, introduces solver.s, and sets step status for accepted/rejected steps. |
src/tron.jl |
Adds callback_quasi_newton plumbing and sets step status for accepted/rejected steps. |
src/JSOSolvers.jl |
Introduces the default callback_quasi_newton implementation for quasi-Newton models. |
docs/src/index.md |
Documents the new callback and updates callback-related wording/section naming. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @testset "Test quasi-Newton callback" begin | ||
| f(x) = (x[1] - 1)^2 + 4 * (x[2] - x[1]^2)^2 | ||
| nlp = LBFGSModel(ADNLPModel(f, [-1.2; 1.0])) |
There was a problem hiding this comment.
The test uses LBFGSModel which is not imported in this file. While it works when included from runtests.jl, the test would fail if run independently. Consider adding using NLPModelsModifiers to make the test file self-contained.
| @testset "Test quasi-Newton callback" begin | ||
| f(x) = (x[1] - 1)^2 + 4 * (x[2] - x[1]^2)^2 | ||
| nlp = LBFGSModel(ADNLPModel(f, [-1.2; 1.0])) | ||
| B0 = Matrix(hess_op(nlp, nlp.meta.x0)) | ||
| nb_callback_calls = 0 | ||
| function qn_cb(nlp, solver, stats) | ||
| nb_callback_calls += 1 | ||
| default_callback_quasi_newton(nlp, solver, stats) | ||
| end | ||
| stats = with_logger(NullLogger()) do | ||
| trunk(nlp, callback_quasi_newton = qn_cb) | ||
| end | ||
| @test nb_callback_calls > 0 | ||
| Bn = Matrix(hess_op(nlp, stats.solution)) | ||
| @test !(all(B0 .== Bn)) | ||
| end |
There was a problem hiding this comment.
The quasi-Newton callback test only covers the trunk solver, but according to the documentation (line 119 in docs/src/index.md) and the code changes, tron also supports callback_quasi_newton. Consider adding a test for tron with a quasi-Newton model to ensure the callback works correctly for both solvers.
MaxenceGollier
left a comment
MaxenceGollier
left a comment
There was a problem hiding this comment.
Very nice @dpo, I only have one suggestion/comment.
|
tmigot
left a comment
tmigot
left a comment
There was a problem hiding this comment.
Looks good, thanks. I created a version 0.3.10 for SolverCore, so hopefully the tests should pass.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Maxence Gollier <134112149+MaxenceGollier@users.noreply.github.com>
|
Thank you both. I applied more comments from Copilot's review and submitted a release of NLPModelsModifiers. It should be green lights soon. |
|
|
|
Performing QN updates in a callback allows for customized updates instead of generic updates.
A default callback with the expected update is provided.