Add lag_hess_structure: canonical COO enumeration for the vector lag_h - #1312
Open
SebastianM-C wants to merge 1 commit into
Open
Add lag_hess_structure: canonical COO enumeration for the vector lag_h#1312SebastianM-C wants to merge 1 commit into
SebastianM-C wants to merge 1 commit into
Conversation
SebastianM-C
force-pushed
the
smc/lag-hess-structure
branch
2 times, most recently
from
August 19, 2026 12:27
949e556 to
90abff2
Compare
Member
Author
|
docs issue addressed, should be good to go now. The JET |
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
force-pushed
the
smc/lag-hess-structure
branch
from
August 19, 2026 16:26
90abff2 to
248ff60
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug this fixes
The vector-form
lag_h(h, θ, σ, λ)that OptimizationBase generates fillshwith the upper-triangle entries of the sparse Lagrangian Hessian infindnz(CSC, column-major) order. That order was an implicit contract that each solver wrapper re-derived independently:OptimizationIpoptderived its declared structure with the same mask and order — correct.OptimizationNLPModels'NLPModelsAdaptordeclared 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
lag_hess_structure(prototype::SparseMatrixCSC) -> (rows, cols), the canonical coordinates in exactly the order the vectorlag_hwrites, documented as frozen public API. Wrappers derive their structure from this instead of re-deriving their own enumeration.(i, j), i ≤ jto(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).lag_hto 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-orderlag_hreaches the same optimum through bothIpoptOptimizerandMadNLPOptimizer(previously impossible: the two wrappers demanded contradictory value orders).Compatibility note
User-supplied sparse vector-form
lag_hcallbacks 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 inOptimizationZygoteExt(closure arity) on the MadNLP path.🤖 Generated with Claude Code
https://claude.ai/code/session_01Gm49JbHkX5UC15UrQXveFh