From 340e256dba8450e9b221f2fb0c58b42dad832564 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Mon, 25 May 2026 22:05:30 -0400 Subject: [PATCH 1/7] feat(madnlp): expose intermediate_callback + fixed_variable_treatment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two pass-through fields to `MadNLPOptions` so users can install `MadNLP.AbstractUserCallback` subtypes for per-iteration hooks (e.g. live plotting of converging pulses, custom stopping criteria) and override MadNLP's handling of fixed bound variables. `fixed_variable_treatment = MadNLP.RelaxBound` is required when a callback needs the full primal vector — the default `MakeParameter` eliminates fixed variables from `solver.x` and breaks the `MadNLP.variable(solver.x)` → `traj.datavec` mapping that trajectory-reconstructing callbacks rely on. Both fields are forwarded by the existing generic `set_options!` loop and default to `nothing` (no behaviour change for existing users). --- ext/MadNLPSolverExt/MadNLPSolverExt.jl | 24 ++++++++++++++++++++++++ src/solvers/madnlp_solver/options.jl | 11 +++++++++++ 2 files changed, 35 insertions(+) diff --git a/ext/MadNLPSolverExt/MadNLPSolverExt.jl b/ext/MadNLPSolverExt/MadNLPSolverExt.jl index 4aceef8..e026776 100644 --- a/ext/MadNLPSolverExt/MadNLPSolverExt.jl +++ b/ext/MadNLPSolverExt/MadNLPSolverExt.jl @@ -29,6 +29,8 @@ include("utils.jl") @test opts.max_iter == 3000 @test opts.print_level == 3 @test opts.hessian_approximation == "exact" + @test opts.intermediate_callback === nothing + @test opts.fixed_variable_treatment === nothing opts2 = DirectTrajOpt.MadNLPOptions(max_iter = 100, tol = 1e-6) @test opts2.max_iter == 100 @@ -36,6 +38,28 @@ include("utils.jl") @test opts isa Solvers.AbstractSolverOptions end +@testitem "MadNLP intermediate_callback fires per iter" setup=[DTOTestHelpers] begin + import MadNLP + + mutable struct _IterCounter <: MadNLP.AbstractUserCallback + count::Base.RefValue{Int} + end + (cb::_IterCounter)(::MadNLP.AbstractMadNLPSolver, _) = (cb.count[] += 1; true) + + cb = _IterCounter(Ref(0)) + prob, _ = make_standard_prob() + solve!( + prob; + options = DirectTrajOpt.MadNLPOptions( + max_iter = 5, + intermediate_callback = cb, + fixed_variable_treatment = MadNLP.RelaxBound, + ), + verbose = false, + ) + @test cb.count[] > 0 +end + @testitem "MadNLP basic solve" setup=[DTOTestHelpers] begin prob, _ = make_standard_prob() traj_before = deepcopy(prob.trajectory.data) diff --git a/src/solvers/madnlp_solver/options.jl b/src/solvers/madnlp_solver/options.jl index 6d3a382..9b2c112 100644 --- a/src/solvers/madnlp_solver/options.jl +++ b/src/solvers/madnlp_solver/options.jl @@ -14,6 +14,17 @@ export MadNLPOptions kkt_system::Any = nothing # e.g. MadNLP.SparseUnreducedKKTSystem cudss_ordering::Any = nothing # e.g. MadNLPGPU.AMD_ORDERING + # Per-iteration user callback. Must be a subtype of `MadNLP.AbstractUserCallback` + # with call signature `(cb)(solver::MadNLP.AbstractMadNLPSolver, mode) -> Bool`. + # Return `false` to stop the solver (yields `USER_REQUESTED_STOP`). + intermediate_callback::Any = nothing + + # Controls how MadNLP handles variables with `lb == ub`. Default (`nothing`) + # uses MadNLP's `MakeParameter`, which eliminates fixed vars from `solver.x`. + # Pass `MadNLP.RelaxBound` if a callback needs the full primal vector (e.g. + # to reconstruct a `NamedTrajectory` from `MadNLP.variable(solver.x)`). + fixed_variable_treatment::Any = nothing + # # Only supported by DirectTrajOpt._solve, as an optional kwarg override of `hessian_approximation`; # # `hessian_approximation = eval_hessian ? "exact" : "compact_lbfgs"` # eval_hessian::Bool = true From bd4ab71d91392cba11b697a65acd20916c0f4427 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Mon, 25 May 2026 22:43:09 -0400 Subject: [PATCH 2/7] feat(solvers): solver-agnostic AbstractIntermediateCallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an abstract callback type in the Solvers module so per-iteration hooks can be written once and reused across every solver backend. Subtypes implement `(cb)(primal::AbstractVector, iter::Integer) -> Bool`; each solver extension wraps an instance in a native-shape adapter at solve time. Wired up in MadNLP via `_MadNLPCallbackAdapter <: MadNLP.AbstractUserCallback`, which translates `solver.x → MadNLP.variable(solver.x)` and forwards `solver.cnt.k` as the iteration index. Raw `MadNLP.AbstractUserCallback` subtypes still pass through unwrapped for users wanting full IPM access. This unblocks downstream packages (e.g. Piccolo) from shipping plotting or analysis callbacks without taking a weak dependency on each solver. --- ext/MadNLPSolverExt/MadNLPSolverExt.jl | 31 +++++++++++++++++++++++++- ext/MadNLPSolverExt/solver.jl | 21 +++++++++++++++++ src/solvers/_solvers.jl | 19 ++++++++++++++++ src/solvers/madnlp_solver/options.jl | 12 ++++++++-- 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/ext/MadNLPSolverExt/MadNLPSolverExt.jl b/ext/MadNLPSolverExt/MadNLPSolverExt.jl index e026776..40e4207 100644 --- a/ext/MadNLPSolverExt/MadNLPSolverExt.jl +++ b/ext/MadNLPSolverExt/MadNLPSolverExt.jl @@ -38,7 +38,7 @@ include("utils.jl") @test opts isa Solvers.AbstractSolverOptions end -@testitem "MadNLP intermediate_callback fires per iter" setup=[DTOTestHelpers] begin +@testitem "MadNLP intermediate_callback (raw MadNLP callback) fires per iter" setup=[DTOTestHelpers] begin import MadNLP mutable struct _IterCounter <: MadNLP.AbstractUserCallback @@ -60,6 +60,35 @@ end @test cb.count[] > 0 end +@testitem "MadNLP intermediate_callback (AbstractIntermediateCallback) fires per iter" setup=[DTOTestHelpers] begin + import MadNLP + + mutable struct _AgnosticCounter <: DirectTrajOpt.AbstractIntermediateCallback + count::Base.RefValue{Int} + last_primal_len::Base.RefValue{Int} + end + function (cb::_AgnosticCounter)(primal::AbstractVector, iter::Integer) + cb.count[] += 1 + cb.last_primal_len[] = length(primal) + return true + end + + cb = _AgnosticCounter(Ref(0), Ref(0)) + prob, _ = make_standard_prob() + solve!( + prob; + options = DirectTrajOpt.MadNLPOptions( + max_iter = 5, + intermediate_callback = cb, + fixed_variable_treatment = MadNLP.RelaxBound, + ), + verbose = false, + ) + @test cb.count[] > 0 + # With RelaxBound, the primal vector matches the full NLP variable count. + @test cb.last_primal_len[] == length(prob.trajectory.datavec) + prob.trajectory.global_dim +end + @testitem "MadNLP basic solve" setup=[DTOTestHelpers] begin prob, _ = make_standard_prob() traj_before = deepcopy(prob.trajectory.data) diff --git a/ext/MadNLPSolverExt/solver.jl b/ext/MadNLPSolverExt/solver.jl index ca60e53..f10ccb5 100644 --- a/ext/MadNLPSolverExt/solver.jl +++ b/ext/MadNLPSolverExt/solver.jl @@ -200,6 +200,22 @@ end # ---------------------------------------------------------------------------- +""" + _MadNLPCallbackAdapter(inner) + +Wrap a `DirectTrajOpt.AbstractIntermediateCallback` so MadNLP can call it +with its native `(solver, mode)` signature. Translates `solver.x` (with +slacks) into just the NLP primal via `MadNLP.variable`, and forwards +`solver.cnt.k` as the iteration index. +""" +struct _MadNLPCallbackAdapter <: MadNLP.AbstractUserCallback + inner::DirectTrajOpt.AbstractIntermediateCallback +end + +function (a::_MadNLPCallbackAdapter)(solver::MadNLP.AbstractMadNLPSolver, _mode) + return a.inner(MadNLP.variable(solver.x), solver.cnt.k) +end + function DirectTrajOpt.set_options!(optimizer::AbstractOptimizer, options::MadNLPOptions) ignored_options = [:eval_hessian] @@ -221,6 +237,11 @@ function DirectTrajOpt.set_options!(optimizer::AbstractOptimizer, options::MadNL hessian_approximation = ((value == "compact_lbfgs") ? MadNLP.CompactLBFGS : hessian_approximation) optimizer.options[name] = hessian_approximation + elseif name == :intermediate_callback && + value isa DirectTrajOpt.AbstractIntermediateCallback + # Wrap solver-agnostic callbacks in the MadNLP-shaped adapter. + # Raw `MadNLP.AbstractUserCallback` subtypes fall through unwrapped. + optimizer.options[name] = _MadNLPCallbackAdapter(value) else optimizer.options[name] = value end diff --git a/src/solvers/_solvers.jl b/src/solvers/_solvers.jl index e81bb2d..3e5a599 100644 --- a/src/solvers/_solvers.jl +++ b/src/solvers/_solvers.jl @@ -2,6 +2,7 @@ module Solvers export AbstractOptimizer export AbstractSolverOptions, DefaultSolverOptions, _DefaultSolverOptions +export AbstractIntermediateCallback export _solve export _solve_with_kwargs export solve! @@ -17,6 +18,24 @@ using TestItemRunner const AbstractOptimizer = MOI.AbstractOptimizer abstract type AbstractSolverOptions end +""" + AbstractIntermediateCallback + +Solver-agnostic per-iteration callback for trajectory optimization. + +Subtypes implement a callable with signature + + (cb::SubType)(primal::AbstractVector, iter::Integer) -> Bool + +where `primal` is the current full NLP primal vector and `iter` is the +iteration index. Return `true` to continue solving, `false` to stop early. + +Each solver extension wraps an `AbstractIntermediateCallback` instance in a +solver-specific adapter at solve time, so the same callback object works +with every backend (MadNLP, Ipopt, …). +""" +abstract type AbstractIntermediateCallback end + struct DefaultSolverOptions <: AbstractSolverOptions end const _DefaultSolverOptions::Ref{Type{<:AbstractSolverOptions}} = Ref{Type{<:AbstractSolverOptions}}(DefaultSolverOptions) diff --git a/src/solvers/madnlp_solver/options.jl b/src/solvers/madnlp_solver/options.jl index 9b2c112..2d8f77c 100644 --- a/src/solvers/madnlp_solver/options.jl +++ b/src/solvers/madnlp_solver/options.jl @@ -14,8 +14,16 @@ export MadNLPOptions kkt_system::Any = nothing # e.g. MadNLP.SparseUnreducedKKTSystem cudss_ordering::Any = nothing # e.g. MadNLPGPU.AMD_ORDERING - # Per-iteration user callback. Must be a subtype of `MadNLP.AbstractUserCallback` - # with call signature `(cb)(solver::MadNLP.AbstractMadNLPSolver, mode) -> Bool`. + # Per-iteration user callback. Two accepted forms: + # + # 1. A subtype of `DirectTrajOpt.AbstractIntermediateCallback` (solver-agnostic). + # Signature: `(cb)(primal::AbstractVector, iter::Integer) -> Bool`. + # The MadNLP extension wraps it in an internal adapter at solve time. + # + # 2. A raw `MadNLP.AbstractUserCallback` subtype with native MadNLP signature + # `(cb)(solver::MadNLP.AbstractMadNLPSolver, mode) -> Bool` — passed through + # unwrapped for users who want full access to the IPM state. + # # Return `false` to stop the solver (yields `USER_REQUESTED_STOP`). intermediate_callback::Any = nothing From 27cac8a2e0bc27cfc5a80546d4dc1166423536d2 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Mon, 25 May 2026 23:57:58 -0400 Subject: [PATCH 3/7] review: tighten intermediate_callback wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on #98: - Filter mode to `UserCallbackRegular` in `_MadNLPCallbackAdapter`. MadNLP also invokes user callbacks during feasibility-restoration and robust phases, which would surface intermediate IPM state that's not meaningful to trajectory-level callbacks. Silently skipping those phases keeps `solver.cnt.k` monotonic from the callback's view. - Auto-couple `fixed_variable_treatment = MadNLP.RelaxBound` when an `AbstractIntermediateCallback` is installed without an explicit value. Emits an `@info` log so the override is visible. Raw MadNLP callbacks are untouched (they manage their own primal access). - Exhaustive type check on `intermediate_callback` — explicit `ArgumentError` for non-callback inputs instead of failing deep inside MadNLP's option parser. - `AbstractIntermediateCallback` docstring now documents the primal-aliasing contract (copy if retaining), iter monotonicity, and the MadNLP RelaxBound auto-coupling. - New testitems: - `auto-couples RelaxBound` — agnostic callback without explicit `fixed_variable_treatment` still receives the full primal. - `early termination via return false` — callback returning `false` after N iters halts the solve before `max_iter`. - `rejects invalid type` — bare `Function` raises `ArgumentError`. --- ext/MadNLPSolverExt/MadNLPSolverExt.jl | 65 ++++++++++++++++++++++++++ ext/MadNLPSolverExt/solver.jl | 51 ++++++++++++++++---- src/solvers/_solvers.jl | 24 +++++++++- 3 files changed, 130 insertions(+), 10 deletions(-) diff --git a/ext/MadNLPSolverExt/MadNLPSolverExt.jl b/ext/MadNLPSolverExt/MadNLPSolverExt.jl index 40e4207..d5dbf19 100644 --- a/ext/MadNLPSolverExt/MadNLPSolverExt.jl +++ b/ext/MadNLPSolverExt/MadNLPSolverExt.jl @@ -89,6 +89,71 @@ end @test cb.last_primal_len[] == length(prob.trajectory.datavec) + prob.trajectory.global_dim end +@testitem "MadNLP intermediate_callback auto-couples RelaxBound" setup=[DTOTestHelpers] begin + import MadNLP + + mutable struct _AutoCoupleProbe <: DirectTrajOpt.AbstractIntermediateCallback + last_primal_len::Base.RefValue{Int} + end + function (cb::_AutoCoupleProbe)(primal::AbstractVector, _) + cb.last_primal_len[] = length(primal) + return true + end + + cb = _AutoCoupleProbe(Ref(0)) + prob, _ = make_standard_prob() + # Note: NOT passing fixed_variable_treatment. set_options! should auto-set it. + solve!( + prob; + options = DirectTrajOpt.MadNLPOptions( + max_iter = 5, + intermediate_callback = cb, + ), + verbose = false, + ) + # If RelaxBound auto-coupled correctly, the primal includes fixed variables. + @test cb.last_primal_len[] == length(prob.trajectory.datavec) + prob.trajectory.global_dim +end + +@testitem "MadNLP intermediate_callback early termination via return false" setup=[DTOTestHelpers] begin + import MadNLP + + mutable struct _Stopper <: DirectTrajOpt.AbstractIntermediateCallback + max_iters::Int + count::Base.RefValue{Int} + end + function (cb::_Stopper)(_, _) + cb.count[] += 1 + return cb.count[] < cb.max_iters + end + + cb = _Stopper(3, Ref(0)) + prob, _ = make_standard_prob() + solve!( + prob; + options = DirectTrajOpt.MadNLPOptions( + max_iter = 100, + intermediate_callback = cb, + ), + verbose = false, + ) + # Callback stopped the solve well before max_iter=100. + @test cb.count[] <= 5 +end + +@testitem "MadNLP intermediate_callback rejects invalid type" setup=[DTOTestHelpers] begin + prob, _ = make_standard_prob() + bogus_cb(args...) = true # bare Function — neither abstract nor MadNLP subtype + @test_throws ArgumentError solve!( + prob; + options = DirectTrajOpt.MadNLPOptions( + max_iter = 5, + intermediate_callback = bogus_cb, + ), + verbose = false, + ) +end + @testitem "MadNLP basic solve" setup=[DTOTestHelpers] begin prob, _ = make_standard_prob() traj_before = deepcopy(prob.trajectory.data) diff --git a/ext/MadNLPSolverExt/solver.jl b/ext/MadNLPSolverExt/solver.jl index f10ccb5..af8f255 100644 --- a/ext/MadNLPSolverExt/solver.jl +++ b/ext/MadNLPSolverExt/solver.jl @@ -204,21 +204,43 @@ end _MadNLPCallbackAdapter(inner) Wrap a `DirectTrajOpt.AbstractIntermediateCallback` so MadNLP can call it -with its native `(solver, mode)` signature. Translates `solver.x` (with -slacks) into just the NLP primal via `MadNLP.variable`, and forwards -`solver.cnt.k` as the iteration index. +with its native `(solver, mode)` signature. + +The adapter: +- **Filters mode to `UserCallbackRegular`.** MadNLP also invokes user + callbacks during feasibility restoration and robust mode; those phases + surface intermediate IPM state that's typically not meaningful to a + trajectory-level callback, so they're silently skipped (return `true`). + This makes `solver.cnt.k` monotonic from the callback's point of view. +- **Translates `solver.x` → `MadNLP.variable(solver.x)`** to strip the + slack tail and hand back just the NLP primal. +- **Forwards `solver.cnt.k`** as the iteration index. """ struct _MadNLPCallbackAdapter <: MadNLP.AbstractUserCallback inner::DirectTrajOpt.AbstractIntermediateCallback end -function (a::_MadNLPCallbackAdapter)(solver::MadNLP.AbstractMadNLPSolver, _mode) +function (a::_MadNLPCallbackAdapter)( + solver::MadNLP.AbstractMadNLPSolver, + mode::MadNLP.AbstractUserCallbackStatus, +) + mode isa MadNLP.UserCallbackRegular || return true return a.inner(MadNLP.variable(solver.x), solver.cnt.k) end function DirectTrajOpt.set_options!(optimizer::AbstractOptimizer, options::MadNLPOptions) ignored_options = [:eval_hessian] + # Auto-couple: an AbstractIntermediateCallback needs the full primal vector, + # which requires fixed_variable_treatment = RelaxBound. If the user installed + # an agnostic callback without setting fixed_variable_treatment, apply it for + # them. Raw MadNLP callbacks are presumed to manage this themselves. + if options.intermediate_callback isa DirectTrajOpt.AbstractIntermediateCallback && + options.fixed_variable_treatment === nothing + @info "Setting fixed_variable_treatment = MadNLP.RelaxBound for AbstractIntermediateCallback (required for full-primal access)" + optimizer.options[:fixed_variable_treatment] = MadNLP.RelaxBound + end + for name in fieldnames(typeof(options)) value = getfield(options, name) if name in ignored_options @@ -237,11 +259,22 @@ function DirectTrajOpt.set_options!(optimizer::AbstractOptimizer, options::MadNL hessian_approximation = ((value == "compact_lbfgs") ? MadNLP.CompactLBFGS : hessian_approximation) optimizer.options[name] = hessian_approximation - elseif name == :intermediate_callback && - value isa DirectTrajOpt.AbstractIntermediateCallback - # Wrap solver-agnostic callbacks in the MadNLP-shaped adapter. - # Raw `MadNLP.AbstractUserCallback` subtypes fall through unwrapped. - optimizer.options[name] = _MadNLPCallbackAdapter(value) + elseif name == :intermediate_callback + if value isa DirectTrajOpt.AbstractIntermediateCallback + # Wrap solver-agnostic callbacks in the MadNLP-shaped adapter. + optimizer.options[name] = _MadNLPCallbackAdapter(value) + elseif value isa MadNLP.AbstractUserCallback + # Raw MadNLP callbacks pass through unwrapped. + optimizer.options[name] = value + else + throw( + ArgumentError( + "intermediate_callback must be a subtype of " * + "`DirectTrajOpt.AbstractIntermediateCallback` or " * + "`MadNLP.AbstractUserCallback`, got $(typeof(value))", + ), + ) + end else optimizer.options[name] = value end diff --git a/src/solvers/_solvers.jl b/src/solvers/_solvers.jl index 3e5a599..ee212f2 100644 --- a/src/solvers/_solvers.jl +++ b/src/solvers/_solvers.jl @@ -28,11 +28,33 @@ Subtypes implement a callable with signature (cb::SubType)(primal::AbstractVector, iter::Integer) -> Bool where `primal` is the current full NLP primal vector and `iter` is the -iteration index. Return `true` to continue solving, `false` to stop early. +iteration index from the solver's main optimization loop. Return `true` to +continue solving, `false` to stop early (the solver will report a +user-requested termination). Each solver extension wraps an `AbstractIntermediateCallback` instance in a solver-specific adapter at solve time, so the same callback object works with every backend (MadNLP, Ipopt, …). + +# Contract + +- **`primal` may alias the solver's internal vector.** Copy it (e.g. + `collect(primal)`) if you need to retain the data past the callback + invocation — its contents may shift on the next iteration. +- **`iter` is monotonic.** The callback is invoked only from the solver's + main IPM loop; auxiliary phases (e.g. MadNLP's feasibility restoration + or robust modes) do not fire it. + +# Required MadNLP setup + +When using MadNLP, the callback must receive the **full** primal vector +to reconstruct trajectories correctly. MadNLP's default +`fixed_variable_treatment = MakeParameter` eliminates variables with +`lb == ub` from the working primal, so any subtype that maps `primal` +back onto a `NamedTrajectory` needs `fixed_variable_treatment = +MadNLP.RelaxBound`. When an `AbstractIntermediateCallback` is installed +via `MadNLPOptions.intermediate_callback`, DTO sets this automatically +(with an `@info` log) unless the user has provided a value. """ abstract type AbstractIntermediateCallback end From d072284a221035bb2f78ca99ea039dc1655cbaef Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Tue, 26 May 2026 00:50:24 -0400 Subject: [PATCH 4/7] chore: autoformat --- ext/MadNLPSolverExt/MadNLPSolverExt.jl | 28 ++++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/ext/MadNLPSolverExt/MadNLPSolverExt.jl b/ext/MadNLPSolverExt/MadNLPSolverExt.jl index d5dbf19..855e517 100644 --- a/ext/MadNLPSolverExt/MadNLPSolverExt.jl +++ b/ext/MadNLPSolverExt/MadNLPSolverExt.jl @@ -38,7 +38,9 @@ include("utils.jl") @test opts isa Solvers.AbstractSolverOptions end -@testitem "MadNLP intermediate_callback (raw MadNLP callback) fires per iter" setup=[DTOTestHelpers] begin +@testitem "MadNLP intermediate_callback (raw MadNLP callback) fires per iter" setup=[ + DTOTestHelpers, +] begin import MadNLP mutable struct _IterCounter <: MadNLP.AbstractUserCallback @@ -60,7 +62,9 @@ end @test cb.count[] > 0 end -@testitem "MadNLP intermediate_callback (AbstractIntermediateCallback) fires per iter" setup=[DTOTestHelpers] begin +@testitem "MadNLP intermediate_callback (AbstractIntermediateCallback) fires per iter" setup=[ + DTOTestHelpers, +] begin import MadNLP mutable struct _AgnosticCounter <: DirectTrajOpt.AbstractIntermediateCallback @@ -86,7 +90,8 @@ end ) @test cb.count[] > 0 # With RelaxBound, the primal vector matches the full NLP variable count. - @test cb.last_primal_len[] == length(prob.trajectory.datavec) + prob.trajectory.global_dim + @test cb.last_primal_len[] == + length(prob.trajectory.datavec) + prob.trajectory.global_dim end @testitem "MadNLP intermediate_callback auto-couples RelaxBound" setup=[DTOTestHelpers] begin @@ -105,17 +110,17 @@ end # Note: NOT passing fixed_variable_treatment. set_options! should auto-set it. solve!( prob; - options = DirectTrajOpt.MadNLPOptions( - max_iter = 5, - intermediate_callback = cb, - ), + options = DirectTrajOpt.MadNLPOptions(max_iter = 5, intermediate_callback = cb), verbose = false, ) # If RelaxBound auto-coupled correctly, the primal includes fixed variables. - @test cb.last_primal_len[] == length(prob.trajectory.datavec) + prob.trajectory.global_dim + @test cb.last_primal_len[] == + length(prob.trajectory.datavec) + prob.trajectory.global_dim end -@testitem "MadNLP intermediate_callback early termination via return false" setup=[DTOTestHelpers] begin +@testitem "MadNLP intermediate_callback early termination via return false" setup=[ + DTOTestHelpers, +] begin import MadNLP mutable struct _Stopper <: DirectTrajOpt.AbstractIntermediateCallback @@ -131,10 +136,7 @@ end prob, _ = make_standard_prob() solve!( prob; - options = DirectTrajOpt.MadNLPOptions( - max_iter = 100, - intermediate_callback = cb, - ), + options = DirectTrajOpt.MadNLPOptions(max_iter = 100, intermediate_callback = cb), verbose = false, ) # Callback stopped the solve well before max_iter=100. From 94bd8a502d156924485b77646cd36fda90003394 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Tue, 26 May 2026 01:22:22 -0400 Subject: [PATCH 5/7] review: tighten fixed_variable_treatment to Union{Type, Nothing} Mirror MadNLP's own field signature (`fixed_variable_treatment::Type`) instead of leaving it as `::Any = nothing`. Catches bad inputs at the `MadNLPOptions` constructor (Strings, Symbols) rather than deep in MadNLP's option parser, while preserving `nothing` as the "defer to MadNLP's kkt_system-aware default" sentinel. The `set_options!` auto-couple branch is unchanged behaviourally (`=== nothing` still matches the default; `MadNLP.RelaxBound` is a `Type`, so the assignment is type-consistent). Explicit overrides (both `MadNLP.RelaxBound` and `MadNLP.MakeParameter`) still flow through correctly. --- src/solvers/madnlp_solver/options.jl | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/solvers/madnlp_solver/options.jl b/src/solvers/madnlp_solver/options.jl index 2d8f77c..db94878 100644 --- a/src/solvers/madnlp_solver/options.jl +++ b/src/solvers/madnlp_solver/options.jl @@ -27,11 +27,16 @@ export MadNLPOptions # Return `false` to stop the solver (yields `USER_REQUESTED_STOP`). intermediate_callback::Any = nothing - # Controls how MadNLP handles variables with `lb == ub`. Default (`nothing`) - # uses MadNLP's `MakeParameter`, which eliminates fixed vars from `solver.x`. - # Pass `MadNLP.RelaxBound` if a callback needs the full primal vector (e.g. - # to reconstruct a `NamedTrajectory` from `MadNLP.variable(solver.x)`). - fixed_variable_treatment::Any = nothing + # Controls how MadNLP handles variables with `lb == ub`. Mirrors MadNLP's + # own `fixed_variable_treatment::Type` field — must be a `Type` subtype of + # `MadNLP.AbstractFixedVariableTreatment` (e.g. `MadNLP.MakeParameter` or + # `MadNLP.RelaxBound`). Default (`nothing`) defers to MadNLP's kkt_system- + # aware default: `RelaxBound` for `SparseCondensedKKTSystem`, otherwise + # `MakeParameter`. Pass `MadNLP.RelaxBound` explicitly if a callback needs + # the full primal vector (auto-coupled by `set_options!` when an + # `AbstractIntermediateCallback` is installed and this field is left at + # the default). + fixed_variable_treatment::Union{Type,Nothing} = nothing # # Only supported by DirectTrajOpt._solve, as an optional kwarg override of `hessian_approximation`; # # `hessian_approximation = eval_hessian ? "exact" : "compact_lbfgs"` From f53432459bf5b28a0e21314a668487b79d77cc72 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Tue, 26 May 2026 01:27:04 -0400 Subject: [PATCH 6/7] review: respect MadNLP's conditional fixed_variable_treatment default When an `AbstractIntermediateCallback` is installed without explicit `fixed_variable_treatment`, only override to `RelaxBound` if MadNLP's own conditional default would otherwise pick `MakeParameter` and break the callback. For `kkt_system <: SparseCondensedKKTSystem`, MadNLP's default is already `RelaxBound`, so leave its if-one-liner alone. Updated docstring on the field; added a testitem asserting that `kkt_system = SparseCondensedKKTSystem` bypasses the auto-couple (captured `@info` stream is empty for our message + primal length still matches the full NLP variable count). --- ext/MadNLPSolverExt/MadNLPSolverExt.jl | 31 ++++++++++++++++++++++++++ ext/MadNLPSolverExt/solver.jl | 18 ++++++++++----- src/solvers/madnlp_solver/options.jl | 20 ++++++++++------- 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/ext/MadNLPSolverExt/MadNLPSolverExt.jl b/ext/MadNLPSolverExt/MadNLPSolverExt.jl index 855e517..1f5a422 100644 --- a/ext/MadNLPSolverExt/MadNLPSolverExt.jl +++ b/ext/MadNLPSolverExt/MadNLPSolverExt.jl @@ -118,6 +118,37 @@ end length(prob.trajectory.datavec) + prob.trajectory.global_dim end +@testitem "MadNLP auto-couple respects MadNLP's conditional default" setup=[DTOTestHelpers] begin + import MadNLP + using Logging + + mutable struct _PassthroughProbe <: DirectTrajOpt.AbstractIntermediateCallback + len::Base.RefValue{Int} + end + (cb::_PassthroughProbe)(primal, _) = (cb.len[] = length(primal); true) + + cb = _PassthroughProbe(Ref(0)) + prob, _ = make_standard_prob() + # With `kkt_system = SparseCondensedKKTSystem`, MadNLP's own conditional + # default for `fixed_variable_treatment` is already `RelaxBound`, so the + # auto-couple should not fire. Capture @info logs and assert ours is absent. + buf = IOBuffer() + with_logger(SimpleLogger(buf, Logging.Info)) do + solve!( + prob; + options = DirectTrajOpt.MadNLPOptions( + max_iter = 5, + intermediate_callback = cb, + kkt_system = MadNLP.SparseCondensedKKTSystem, + ), + verbose = false, + ) + end + @test !occursin("Setting fixed_variable_treatment", String(take!(buf))) + # MadNLP's untouched conditional default still yields the full primal. + @test cb.len[] == length(prob.trajectory.datavec) + prob.trajectory.global_dim +end + @testitem "MadNLP intermediate_callback early termination via return false" setup=[ DTOTestHelpers, ] begin diff --git a/ext/MadNLPSolverExt/solver.jl b/ext/MadNLPSolverExt/solver.jl index af8f255..573696a 100644 --- a/ext/MadNLPSolverExt/solver.jl +++ b/ext/MadNLPSolverExt/solver.jl @@ -232,13 +232,21 @@ function DirectTrajOpt.set_options!(optimizer::AbstractOptimizer, options::MadNL ignored_options = [:eval_hessian] # Auto-couple: an AbstractIntermediateCallback needs the full primal vector, - # which requires fixed_variable_treatment = RelaxBound. If the user installed - # an agnostic callback without setting fixed_variable_treatment, apply it for - # them. Raw MadNLP callbacks are presumed to manage this themselves. + # which requires fixed_variable_treatment = RelaxBound. We only override when + # MadNLP's own conditional default (`kkt_system <: SparseCondensedKKTSystem ? + # RelaxBound : MakeParameter`) would otherwise pick `MakeParameter` and break + # the callback. When the user has selected a kkt_system whose default is + # already `RelaxBound`, MadNLP's if-one-liner gets to do its job untouched. + # Raw MadNLP callbacks are presumed to manage this themselves. if options.intermediate_callback isa DirectTrajOpt.AbstractIntermediateCallback && options.fixed_variable_treatment === nothing - @info "Setting fixed_variable_treatment = MadNLP.RelaxBound for AbstractIntermediateCallback (required for full-primal access)" - optimizer.options[:fixed_variable_treatment] = MadNLP.RelaxBound + madnlp_default_is_relax_bound = + options.kkt_system isa Type && + options.kkt_system <: MadNLP.SparseCondensedKKTSystem + if !madnlp_default_is_relax_bound + @info "Setting fixed_variable_treatment = MadNLP.RelaxBound for AbstractIntermediateCallback (MadNLP's kkt_system default would otherwise eliminate fixed vars from solver.x)" + optimizer.options[:fixed_variable_treatment] = MadNLP.RelaxBound + end end for name in fieldnames(typeof(options)) diff --git a/src/solvers/madnlp_solver/options.jl b/src/solvers/madnlp_solver/options.jl index db94878..5f8a6d8 100644 --- a/src/solvers/madnlp_solver/options.jl +++ b/src/solvers/madnlp_solver/options.jl @@ -28,14 +28,18 @@ export MadNLPOptions intermediate_callback::Any = nothing # Controls how MadNLP handles variables with `lb == ub`. Mirrors MadNLP's - # own `fixed_variable_treatment::Type` field — must be a `Type` subtype of - # `MadNLP.AbstractFixedVariableTreatment` (e.g. `MadNLP.MakeParameter` or - # `MadNLP.RelaxBound`). Default (`nothing`) defers to MadNLP's kkt_system- - # aware default: `RelaxBound` for `SparseCondensedKKTSystem`, otherwise - # `MakeParameter`. Pass `MadNLP.RelaxBound` explicitly if a callback needs - # the full primal vector (auto-coupled by `set_options!` when an - # `AbstractIntermediateCallback` is installed and this field is left at - # the default). + # own `fixed_variable_treatment::Type` field — must be a `Type` (typically + # `MadNLP.MakeParameter` or `MadNLP.RelaxBound`). Default (`nothing`) defers + # to MadNLP's kkt_system-aware conditional default: + # + # kkt_system <: SparseCondensedKKTSystem ? RelaxBound : MakeParameter + # + # When an `AbstractIntermediateCallback` is installed and this field is + # left at `nothing`, `set_options!` only overrides to `RelaxBound` if + # MadNLP's conditional default would otherwise be `MakeParameter` (which + # eliminates fixed boundary vars from `solver.x` and breaks trajectory + # reconstruction). The conditional default's `RelaxBound` branch is left + # untouched. fixed_variable_treatment::Union{Type,Nothing} = nothing # # Only supported by DirectTrajOpt._solve, as an optional kwarg override of `hessian_approximation`; From e33bb9904a4d1dabc4387693bd16822cf9d25649 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Tue, 26 May 2026 11:34:53 -0400 Subject: [PATCH 7/7] fix(test): use Test.collect_test_logs instead of Logging stdlib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new "MadNLP auto-couple respects MadNLP's conditional default" testitem used `with_logger(SimpleLogger(...))` from the `Logging` stdlib, which isn't in DTO's test extras and failed to resolve in CI. Switch to `Test.collect_test_logs` — available via the testitem's auto-imported Test stdlib, no extra dep needed. --- ext/MadNLPSolverExt/MadNLPSolverExt.jl | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ext/MadNLPSolverExt/MadNLPSolverExt.jl b/ext/MadNLPSolverExt/MadNLPSolverExt.jl index 1f5a422..0774279 100644 --- a/ext/MadNLPSolverExt/MadNLPSolverExt.jl +++ b/ext/MadNLPSolverExt/MadNLPSolverExt.jl @@ -120,7 +120,6 @@ end @testitem "MadNLP auto-couple respects MadNLP's conditional default" setup=[DTOTestHelpers] begin import MadNLP - using Logging mutable struct _PassthroughProbe <: DirectTrajOpt.AbstractIntermediateCallback len::Base.RefValue{Int} @@ -131,9 +130,8 @@ end prob, _ = make_standard_prob() # With `kkt_system = SparseCondensedKKTSystem`, MadNLP's own conditional # default for `fixed_variable_treatment` is already `RelaxBound`, so the - # auto-couple should not fire. Capture @info logs and assert ours is absent. - buf = IOBuffer() - with_logger(SimpleLogger(buf, Logging.Info)) do + # auto-couple should not fire. Capture logs and assert our @info is absent. + logs, _ = Test.collect_test_logs() do solve!( prob; options = DirectTrajOpt.MadNLPOptions( @@ -144,7 +142,7 @@ end verbose = false, ) end - @test !occursin("Setting fixed_variable_treatment", String(take!(buf))) + @test !any(l -> occursin("Setting fixed_variable_treatment", l.message), logs) # MadNLP's untouched conditional default still yields the full primal. @test cb.len[] == length(prob.trajectory.datavec) + prob.trajectory.global_dim end