Skip to content

Use live parameters in sparse constraint Jacobians - #1270

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:agent/fix-sparse-cons-j-live-parameters
Aug 2, 2026
Merged

Use live parameters in sparse constraint Jacobians#1270
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:agent/fix-sparse-cons-j-live-parameters

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

Important

Ignore this PR until it has been reviewed by @ChrisRackauckas.

Summary

  • accept the live parameter argument in generated and analytic sparse constraint-Jacobian callbacks
  • rebuild DifferentiationInterface preparation when the live state or parameter type changes
  • cover in-place/out-of-place and generated/analytic callbacks directly in OptimizationBase
  • exercise generated and analytic sparse callbacks through OptimizationIpopt

Root cause

OptimizationIpopt switched to the generic OptimizationCache in #1257. That cache invokes constraint Jacobians with the current parameter value. The sparse instantiation path still produced two-argument callbacks that captured the initial parameter, causing a MethodError when called as cons_j(J, x, p). The out-of-place path had the equivalent missing live-parameter behavior.

This patch preserves the old two-argument convenience through default arguments while making the current parameter part of differentiation and analytic callback evaluation.

Investigation

  • reproduced the failure from current master at be6fbd279
  • traced the behavior change to Use the default OptimizationCache in OptimizationIpopt #1257; the same BoundaryValueDiffEq case passed with OptimizationIpopt 1.2.3
  • reduced it to sparse generated and analytic constraint Jacobians
  • added direct four-way coverage in OptimizationBase and an in-place solver integration test in OptimizationIpopt

Local validation

  • OptimizationBase AD: 729 passed
  • OptimizationIpopt Core: 129 passed, 1 pre-existing broken
  • Runic check passed for all three changed Julia files
  • git diff --check passed

@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the agent/fix-sparse-cons-j-live-parameters branch from 16265b0 to d1e327f Compare July 16, 2026 19:02

Copy link
Copy Markdown
Member Author

Downstream confirmation from clean current SciML/BoundaryValueDiffEq.jl master (5e9eb4eff5e493c5ae56d5f4fc2027b446c55610).

I reduced lib/BoundaryValueDiffEqMIRK/test/Core/mirk_basic_tests.jl:597 to the vector-parameter TwoPointBVProblem solve with MIRK4(optimize = IpoptOptimizer()), then ran it in an isolated Julia 1.12 environment/depot.

Version boundary:

  • OptimizationBase 5.2.1 + OptimizationIpopt 1.2.3: passes, p = [17.096586859834332]
  • OptimizationBase 5.2.1 + OptimizationIpopt 1.3.0: fails in OptimizationIpopt.eval_constraint_jacobian because the generated sparse callback has only cons_j(J, x) but is called as cons_j(J, x, p)
  • this PR's OptimizationBase at d1e327f4f + released OptimizationIpopt 1.3.0: passes, p = [17.096586859834332]

The 1.3.0 boundary maps to be6fbd279 / #1257. This PR is the correct upstream fix for the clean-main BoundaryValueDiffEq failure; no BVP-side test suppression or compatibility cap is needed.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebased onto current clean upstream/master (4d25faca8); new head 3961edfd6. Before the lease-protected push I audited the remote branch: it contained only the original ChrisRackauckas-Claude commit and no intervening contributor work. The rebased tree is byte-for-byte identical to the freshly validated tree. Current local verification: OptimizationBase AD 785/785; OptimizationBase Core 52/52; OptimizationIpopt 129 pass + 1 pre-existing broken; vector and immutable custom-SciMLStructures BoundaryValueDiffEqMIRK parameter-tuning solves both succeed with p = 17.096586859834332; Runic clean. Versioning remains intentionally untouched pending the merge/release ordering with #1287 and #1269.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

CI triage after the clean-master rebase:

  • The only non-systemic failure observed so far is downgrade OptimizationSophia. Its unseeded Lux/Sophia training assertion ended at 10 × objective = 4.4186e6 versus l1 = 3.5403e6.
  • The simultaneous Recognize explicit second-order AutoSparse AD #1269 job on the same base passed OptimizationSophia 3/3, and the current master downgrade job also passes it.
  • This PR only changes sparse DifferentiationInterface constraint-Jacobian construction; the Sophia test uses dense AutoZygote, so it does not execute the changed path.
  • The remaining QA reexport failures reproduce on master and are already addressed by [AI] Curate the public API instead of blanket-reexporting dependencies #1286.

The available Actions credentials return 403 for a job rerun, so I could not request a clean retry.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Additional CI triage: the documentation job failed only because Documenter linkcheck received HTTP 429 from https://nlopt.readthedocs.io/en/latest/ and https://nlopt.readthedocs.io/en/latest/NLopt_Algorithms/. The branch does not modify documentation or those links, so this is an external rate-limit failure rather than a failure in the sparse constraint-Jacobian change.

Relevant OptimizationBase AD/Core, OptimizationIpopt Core, downgrade, Runic, and spell checks pass; the separately documented unseeded Sophia downgrade and current-master QA reexport failures remain the other red checks.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the agent/fix-sparse-cons-j-live-parameters branch from 3961edf to c4b93f1 Compare August 1, 2026 21:09
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebased onto current master (06a10aa9)

Force-pushed 3961edfdc4b93f1b. The branch was CONFLICTING; it is now MERGEABLE.

Conflict and resolution

One conflict, in lib/OptimizationIpopt/test/core_tests.jl.

The cause is worth calling out: master gained #1291 ("Fix parameterized sparse constraint Jacobians in Ipopt") on 2026-07-31, which targets the same bug as this PR via a different fix, and added a testset in the same location.

Resolved by keeping both testsets, as they are not duplicates:

  • master's "sparse explicit constraint Jacobian with parameters" — analytic cons_j only
  • this PR's "sparse constraint Jacobian with parameters" — generated and analytic, in-place and out-of-place

Also note OptimizationDISparseExt.jl moved ext/src/ on master; git rename detection carried the change over cleanly.

Relationship to #1291 — complementary, not redundant

  • Fix parameterized sparse constraint Jacobians in Ipopt #1291 patches OptimizationIpopt/src/evaluator.jl defensively: applicable(cons_j, J, x, p) guards a fallback to the two-argument call.
  • This PR fixes the root cause in OptimizationBase/src/OptimizationDISparseExt.jl: sparse instantiation now produces callbacks that accept the live p, and rebuilds DifferentiationInterface preparation when the x/p types change.

That distinction is load-bearing. Under #1291 alone, the AD-generated sparse path still falls back to the two-argument closure that captured the initial p, so it silently differentiates at a stale parameter rather than erroring. The MethodError is gone, but the wrong-value case is not.

The test added here pins exactly that: instantiate with p = [2.0], call with [3.0], require 3.0. Master's testset covers only the analytic path, so this gap is currently untested on master.

The change follows the pattern already established on master for the dense path — _prep_valid / _cons_out_eltype in OptimizationDIExt.jl — rather than introducing a new convention.

Local validation on the rebased branch

Both suites run against the rebased tree, with [sources] resolving OptimizationBase to the local checkout:

  • OptimizationBase AD: 785 passed, 0 failed (6m44s)
  • OptimizationIpopt Core: 131 passed, 1 broken, 0 failed (7m17s)

The 1 broken is the pre-existing @test_broken at core_tests.jl:196, present on master unchanged — not introduced here.

Runic --check passes on all three changed files.

Co-Authored-By: Chris Rackauckas accounts@chrisrackauckas.com

@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review August 2, 2026 00:45
@ChrisRackauckas
ChrisRackauckas merged commit 7ba3fc0 into SciML:master Aug 2, 2026
74 of 107 checks passed
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