Skip to content

Use the default OptimizationCache in OptimizationIpopt - #1257

Merged
ChrisRackauckas merged 2 commits into
masterfrom
ipopt-default-cache
Jul 13, 2026
Merged

Use the default OptimizationCache in OptimizationIpopt#1257
ChrisRackauckas merged 2 commits into
masterfrom
ipopt-default-cache

Conversation

@SebastianM-C

@SebastianM-C SebastianM-C commented Jul 11, 2026

Copy link
Copy Markdown
Member

Summary

Cleans up OptimizationIpopt, which had copied OptimizationMOI's structure when it was created. The bespoke IpoptCache (a merge of MOI's MOIOptimizationNLPEvaluator + MOIOptimizationNLPCache) is replaced with OptimizationBase's default OptimizationCache, following the structure of OptimizationMadNLP: the package now only defines IpoptOptimizer, the SciMLBase traits, and __solve(::OptimizationCache{<:IpoptOptimizer}). Solver-specific state (derivative buffers, sparsity, call counters) lives in a transient IpoptEvaluator built inside __solve.

Deleted along with the custom cache: the hand-rolled __init, the ReInitCache getproperty delegation, the four solution accessors, the AutoSymbolics instantiation special-case (traits + the evaluator's lag_h → hess + cons_h fallback cover it), and dead MOI leftovers (int, obj_expr/cons_expr, a discarded sys, stale evaluator.J/evaluator.H comments).

Behavior fixes

  • MaxSense sign bug: the evaluators negated f/grad/hess but __solve reported obj_val without flipping it back, so sol.objective (and the objective passed to user callbacks) had the wrong sign for maximization. Now handled with Ipopt's obj_scaling_factor = -1 (negative ⇒ maximize, unscaled objective reported), deleting all manual sign-flipping.
  • reinit!(cache; p = ...) silently converged to the old optimum: the DI closures capture p at init time, so the objective saw the new parameters while every derivative saw the old ones. The evaluator now forwards the live cache.p (guarded for AutoSymbolics, NoAD without parameters, and p === nothing).
  • Conditional second-order traits: requireshessian/requiresconshess/requireslagh (the latter was previously missing) now depend on hessian_approximation, so limited-memory runs skip Hessian AD generation entirely and pass eval_h = nothing to Ipopt. The unused cons_vjp is no longer generated — Ipopt's C API has no vjp/jvp callbacks.

Test suite

The three "automatically translated from the Ipopt tests" files were audited against the Ipopt repo; only two testsets actually traced back to it, both flawed:

  • MyNLP was a different problem entirely — replaced with the real one (min −(x₂−2)² s.t. x₁² + x₂ − 1 = 0, optimum (±1, 0) with objective −4).
  • LuksanVlcek1 had invented constraint bounds ([4,6] vs the registered [0,0]/[−1,0]) and a changed start — now the equality form with the original alternating start, pinned to its optimum.
  • The restoration phase test's two equality constraints are mutually infeasible (on x₁²+x₂² = 0.5 the max of x₁³+x₂³ is ≈0.354 < 1), which is why its guarded assertion never ran; it now asserts ReturnCode.Infeasible.
  • The warm-start test actually warm-starts via reinit!; print levels are verified through Ipopt's output_file; the option-priority test is now discriminating; the derivative test checks the derivative checker's verdict; stress-test RNGs are seeded.
  • New tests: MaxSense objective/callback sign, reinit! with new parameters, limited-memory trait behavior.
  • Pruned: redundant NLS/mixed-integer testsets, a non-smooth robust-optimization problem, a dead commented-out block, and a duplicate verbose loop. The inaccurate provenance headers are gone; EPL attribution stays on the genuinely Ipopt-derived problems (HS071, MyNLP, LuksanVlcek1).

MTK reinit! with new parameters remains @test_broken: the plain-vector form can't convert into the concretely typed ReInitCache{_, MTKParameters} field, and the symbolic-map form hits SciMLBase's generic process_p_u0_symbolic fallback ("Symbolic remake … is currently not supported"). Both need upstream fixes.

Test results (Julia 1.12.6, Pkg.test)

  • Core: 125 passed, 0 failed, 1 broken (the documented MTK case)
  • QA: 17 passed, 1 broken (pre-existing no_implicit_imports)

Version bumped to 1.3.0.

🤖 Generated with Claude Code

SebastianM-C and others added 2 commits July 11, 2026 05:04
Replace the bespoke IpoptCache (copied from OptimizationMOI's
evaluator/cache split) with OptimizationBase's default OptimizationCache,
mirroring the structure of OptimizationMadNLP: the package now only
defines the optimizer struct, the SciMLBase traits, and
__solve(::OptimizationCache{<:IpoptOptimizer}), with the solver-specific
state (derivative buffers, sparsity, call counters) held by a transient
IpoptEvaluator built inside __solve. This removes the hand-rolled
__init, the ReInitCache delegation, the solution accessors, and the
dead MOI leftovers (int, obj_expr/cons_expr, discarded sys).

Behavior fixes that fell out of the refactor:
- MaxSense is now handled with Ipopt's obj_scaling_factor = -1 instead
  of manually negating in the evaluators, fixing the sign of
  sol.objective and of the value passed to user callbacks.
- The live cache.p is forwarded to the instantiated derivative closures
  (guarded for AutoSymbolics/NoAD/parameterless paths), so
  reinit!(cache; p = ...) no longer silently converges to the optimum
  of the old parameters.
- requireshessian/requiresconshess/requireslagh are conditional on
  hessian_approximation, so limited-memory runs skip second-order AD
  entirely and pass eval_h = nothing; requireslagh was previously
  missing and the unused cons_vjp is no longer generated (Ipopt's C API
  has no vjp/jvp callbacks).

Test suite cleanup, cross-checked against the Ipopt repo:
- Fix the mistranslated MyNLP problem and the invented LuksanVlcek1
  constraint bounds/starting point; pin their optima.
- The restoration phase test's constraints are mutually infeasible, so
  assert ReturnCode.Infeasible instead of a vacuous guarded check.
- Make the warm-start test actually warm-start via reinit!, verify
  print levels through Ipopt's output_file, make the option-priority
  test discriminating, check the derivative checker's verdict, and
  seed the stress-test RNGs.
- Add tests for the MaxSense objective sign, reinit! with new
  parameters, and the limited-memory trait behavior.
- Prune redundant testsets (NLS, mixed-integer, non-smooth robust
  optimization, dead complementarity block, duplicate verbose loop)
  and drop the inaccurate "automatically translated" headers, keeping
  EPL attribution on the genuinely Ipopt-derived problems.

MTK reinit! stays @test_broken: both the plain-vector and symbolic-map
forms are blocked upstream (ReInitCache field conversion and SciMLBase's
process_p_u0_symbolic fallback).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTyNNDwaNrJFyNzN6XTqcn
@SebastianM-C
SebastianM-C marked this pull request as ready for review July 13, 2026 17:09
@ChrisRackauckas
ChrisRackauckas merged commit be6fbd2 into master Jul 13, 2026
48 of 51 checks passed
@SebastianM-C
SebastianM-C deleted the ipopt-default-cache branch July 13, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants