Skip to content

Add lag_hess_structure: canonical COO enumeration for the vector lag_h - #1312

Open
SebastianM-C wants to merge 1 commit into
SciML:masterfrom
SebastianM-C:smc/lag-hess-structure
Open

Add lag_hess_structure: canonical COO enumeration for the vector lag_h#1312
SebastianM-C wants to merge 1 commit into
SciML:masterfrom
SebastianM-C:smc/lag-hess-structure

Conversation

@SebastianM-C

Copy link
Copy Markdown
Member

The bug this fixes

The vector-form lag_h(h, θ, σ, λ) that OptimizationBase generates fills h with the upper-triangle entries of the sparse Lagrangian Hessian in findnz (CSC, column-major) order. That order was an implicit contract that each solver wrapper re-derived independently:

  • OptimizationIpopt derived its declared structure with the same mask and order — correct.
  • OptimizationNLPModels' NLPModelsAdaptor declared the lower triangle in CSC order — a different enumeration for any symmetric pattern with ≥ 3 coupled columns. Every off-diagonal Hessian value landed on a wrong index, and MadNLP silently optimized with garbled curvature (found while benchmarking a 6471-nnz optimal-control NLP: the misplaced entries turned a convergent solve into 300 iterations of wandering).

Why tests never caught it: the existing MadNLP test used a 2×2 pattern, where the two enumerations coincide by symmetry — structurally incapable of detecting the mismatch — and assertions compared converged solutions, never derivative values.

The change

  • OptimizationBase: export lag_hess_structure(prototype::SparseMatrixCSC) -> (rows, cols), the canonical coordinates in exactly the order the vector lag_h writes, documented as frozen public API. Wrappers derive their structure from this instead of re-deriving their own enumeration.
  • OptimizationIpopt: use the helper (behavior identical — pure refactor; the derivation now lives in one place).
  • OptimizationNLPModels: use the helper, mirroring each (i, j), i ≤ j to (j, i) so the declared coordinates satisfy the NLPModels lower-triangle convention while preserving the value order (mirrored upper-CSC ≡ lower-CSR: same entries, same sequence).
  • Tests: replace the 2×2 tutorial problem with an ordering-sensitive 3-variable problem checked against a pure-AD reference; fix the 4×4 test's hand-written lag_h to the canonical order; add an entry-wise structure/value regression test in OptimizationNLPModels against an independent dense-AD Lagrangian Hessian (plus a guard asserting the pattern actually distinguishes the two enumerations).

Verification

23/23 in a dedicated suite: helper unit (exact expected coordinates on a 3×3 pattern), entry-wise value-at-coordinate checks through the AutoSparse DI path, adaptor end-to-end via hess_structure!/hess_coord!, and cross-solver consistency — the same hand-written canonical-order lag_h reaches the same optimum through both IpoptOptimizer and MadNLPOptimizer (previously impossible: the two wrappers demanded contradictory value orders).

Compatibility note

User-supplied sparse vector-form lag_h callbacks that wrote values in the adaptor's old lower-CSC order (as the in-repo 4×4 test did) must switch to the canonical order. Such callbacks could never have been correct on both Ipopt and MadNLP simultaneously; this PR makes one order valid everywhere.

Known separate issue (not addressed here): AD-only constrained problems with SecondOrder(_, AutoZygote) error in OptimizationZygoteExt (closure arity) on the MadNLP path.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Gm49JbHkX5UC15UrQXveFh

@SebastianM-C
SebastianM-C force-pushed the smc/lag-hess-structure branch 2 times, most recently from 949e556 to 90abff2 Compare August 19, 2026 12:27
@SebastianM-C

SebastianM-C commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

docs issue addressed, should be good to go now.

The JET _prep_grad report is pre-existing (from #1283), not introduced here.

The vector-form lag_h(h, θ, σ, λ) fills h with the upper-triangle entries
of the sparse Lagrangian Hessian in findnz (CSC) order. That order was an
implicit contract each wrapper re-derived: OptimizationIpopt matched it,
but OptimizationNLPModels' NLPModelsAdaptor declared the lower triangle
in CSC order — a different enumeration for any pattern with >= 3 coupled
columns — so every off-diagonal Hessian value landed on a wrong index and
MadNLP silently optimized with garbled curvature.

- OptimizationBase: export lag_hess_structure(prototype) returning the
  canonical (rows, cols); document the write order as frozen public API.
- OptimizationIpopt: derive the declared structure through the helper
  (behavior identical; the derivation now lives in one place).
- OptimizationNLPModels: derive the mirrored lower-triangle structure
  through the helper — this fixes the ordering bug while satisfying the
  NLPModels lower-triangle convention (mirrored upper-CSC = lower-CSR;
  same entries, same value order).
- Tests: replace the 2x2 MadNLP tutorial problem (a 2x2 symmetric pattern
  cannot detect ordering mismatches — the enumerations coincide) with an
  ordering-sensitive 3-variable problem checked against a pure-AD
  reference; fix the 4x4 test's user lag_h to the canonical order; add an
  entry-wise structure/value regression test in OptimizationNLPModels
  against an independent dense-AD Lagrangian Hessian.

BREAKING-ish note: user-supplied sparse vector lag_h callbacks that wrote
values in the adaptor's old lower-CSC order (as the in-repo 4x4 test did)
must switch to the canonical order. Previously the two wrappers demanded
contradictory orders, so no such callback could be correct on both Ipopt
and MadNLP; cross-solver consistency for one canonical order is verified.

Known separate issue (not addressed here): AD-only constrained problems
with SecondOrder(_, AutoZygote) error in OptimizationZygoteExt (closure
arity) on the MadNLP path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SebastianM-C
SebastianM-C force-pushed the smc/lag-hess-structure branch from 90abff2 to 248ff60 Compare August 19, 2026 16:26
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.

1 participant