Skip to content

Preserve triangular solves for Broyden Jacobian inversion - #1057

Closed
ChrisRackauckas-Claude wants to merge 2 commits into
SciML:masterfrom
ChrisRackauckas-Claude:agent/fix-broyden-true-jacobian
Closed

Preserve triangular solves for Broyden Jacobian inversion#1057
ChrisRackauckas-Claude wants to merge 2 commits into
SciML:masterfrom
ChrisRackauckas-Claude:agent/fix-broyden-true-jacobian

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Jul 13, 2026

Copy link
Copy Markdown
Member

Important

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

Summary

  • Preserve triangular solve semantics when initializing an inverse Jacobian from a nonsingular CPU strided triangular matrix.
  • Preserve an aliased prior inverse before refilling the reusable identity RHS, so direct resets and nested reinversion remain correct.
  • Keep general matrices and singular triangular matrices on LinearSolve's existing default path, including its pivoted-QR rescue.
  • Add direct workspace/reset coverage and the exact generalized-Rosenbrock bad-Broyden regression.
  • Preserve all existing expected-broken coverage; this does not adopt Update bad Broyden robustness expectations #1069's broken-test change.

Root cause

The current master Core suite at 247a8bfcb fails generalized Rosenbrock with:

Broyden(init_jacobian = Val(:true_jacobian), update_rule = Val(:bad_broyden))

A clean adjacent-commit reproduction and git bisect identified 6a7687f542cde52b4bfeef5b473c6156c8d453a1 (#1039) as the first bad commit:

  • d616cd994 (its parent): Success, 17 iterations, residual 0.0
  • 6a7687f542 and affected master: Unstable, 24 iterations, residual 4.4

The persistent workspace itself was not the cause. The true Jacobian is dense lower triangular, and the generic pivoted-LU solve introduced different roundoff than the previous structure-preserving triangular inverse. Bad Broyden is sensitive to that perturbation. Solving the identity RHS through LowerTriangular restores the prior trajectory without reinstating matrix-sized allocations.

The fast path is restricted to StridedMatrix inputs with a nonzero diagonal. GPU/non-strided inputs, general matrices, and singular triangular matrices retain the existing LinearSolve route. The workspace-owned coefficient buffer handles the case where a prior triangular result aliases the identity RHS during reset.

This implementation uses exported LinearAlgebra API only: diagind, istriu, istril, UpperTriangular, LowerTriangular, and ldiv!. It does not add, remove, or change any public NonlinearSolve API.

Verification

The branch was safely rebased onto current master at 247a8bfcb, after re-fetching the remote branch and verifying that its two commits still contained no intervening contributor work. The following commands were then run locally on Julia 1.10.11 with current compatible dependencies, including LinearSolve 5.0.0:

  • Clean current-master root Core reproduction: generalized Rosenbrock algorithm Make iip a compile-time constant #4 failed with residual 4.4 and ReturnCode.Unstable; the Broyden block reported 94 passed, 1 failed, 1 errored, and 19 broken.
  • Exact patched target: generalized Rosenbrock algorithm Make iip a compile-time constant #4 returned residual 0.0, ReturnCode.Success, and the original 17-iteration trajectory.
  • GROUP=Core for NonlinearSolveBase passed. The expanded linsolve_identity!! workspace/reset/alias block passed 53/53, routing 12/12, operator-Jacobian 1/1, and dense-LU 14/14.
  • GROUP=Core for NonlinearSolveQuasiNewton passed in full: Broyden 810/810, iterator 2/2, termination 27/27; Klement 297/297, iterator 2/2, termination 27/27; LimitedMemoryBroyden 135/135, iterator 2/2, termination 27/27; and the new triangular true-Jacobian regression 2/2.
  • Patched root Core reached the independently reproduced clean-master Brown expected-broken unexpected pass tracked in Brown bad-Broyden broken test is environment-sensitive #1056. Before that stop, generalized Rosenbrock algorithm Make iip a compile-time constant #4 passed and the Broyden block reported 95 passed, 1 errored, and 19 broken. The sole error was Brown algorithm Make iip a compile-time constant #4; no broken-test bookkeeping was changed.
  • Full-repository Runic check passed all 374 Julia files.
  • git diff --check passed and the tracked worktree was clean.

No assertions were loosened, and no tests were skipped, disabled, silenced, deleted, or converted to broken.

Investigation/process log

I reproduced the failure on a clean detached checkout of current master with the latest compatible graph, reproduced its immediate good and bad revisions on a common historical dependency graph, and ran an actual git bisect. I compared the old structure-preserving inversion with the reusable LinearSolve workspace, checked the first step and inverse numerics, and tested generic LU, SVD, pseudoinverse, and triangular identity solves. That reduced the fix to the CPU-strided nonsingular triangular path. I then adopted the existing focused draft rather than opening a duplicate, rebased it onto current master, reran the exact target and both affected sublibrary Core groups, ran the root Core group through the known unrelated Brown stop, checked the LinearAlgebra exports, and ran Runic.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Commit 17d95c4 completes the focused fix.

Local evidence recorded for this commit:

  • NonlinearSolveBase Core passed on Julia 1.10 and 1.12; linsolve_identity!! workspace was 52/52 on both.
  • NonlinearSolveQuasiNewton Core passed on Julia 1.10 and 1.12; the new bad-Broyden regression was 2/2 on both.
  • Exact SciMLBase e3293b9b7 / SciMLOperators 1.24.1 reproduction changed the original case from Unstable, residual=4.4 to Success, residual=0.0 in 17 iterations.
  • Runic and git diff --check passed.

The separate root-Core Brown unexpected-pass result remains tracked in #1056; this commit does not alter broken-test bookkeeping.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

The red root Downgrade / Downgrade Tests - Core check is an exact clean-master resolver failure and does not exercise this PR. Clean master b491a4e8d failed identically in run 29217402054: the downgrade action explicitly pins root LinearSolve = 3.75.0, while the in-tree NonlinearSolveBase 2.34.1 now requires LinearSolve >= 4.3, leaving no version. This branch changes only Base solver workspace logic/tests and QuasiNewton regression coverage. The clean-base compat-floor mismatch is queued for its own audit/fix rather than being folded into #1057.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

The three failed downgrade-sublibrary jobs are the same clean-base compatibility cluster, also introduced before this branch. Clean master run 29217402006 fails in exactly FirstOrder, QuasiNewton, and HomotopyContinuation: their downgraded stacks select/constrain LinearSolve below 4.3 while the in-tree NonlinearSolveBase 2.34.1 requires 4.3+. Clean run 29179952280 at parent d616cd994 passed, and 29180620483 at introducing commit 6a7687f54 first failed. The queued clean-base audit will cover the root and all three sublibrary floors as one dependency-boundary repair; #1057 remains unchanged.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

CI audit update for the red root Core / Julia 1 / Ubuntu check:

  • PR job 86820153277 fails test/Core/homotopy_alloc_tests__item1.jl:83-84 with sweep_per_step(prob_big) = 154.4 and arclength_per_step(prob_big) = 80.0, both against < 48.
  • I checked detached current clean master b491a4e8d locally with Julia 1.12.6 and the current resolved root environment. A focused execution of the same test exits 1 at line 83 with the exact same sweep_per_step(prob_big) = 154.4 result.
  • The hosted clean-master Core job 86752744822 also fails, but earlier in the ordered group before reaching this allocation test.

This is therefore a reproducible clean-master Core regression, independent of the Broyden triangular-solve patch in #1057. Per the repository audit rules, I am assigning a separate reproduce/bisect/fix investigation. PR job: https://github.com/SciML/NonlinearSolve.jl/actions/runs/29250644943/job/86820153277

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

CI audit update for the red NonlinearSolveBase QA check:

PR job 86820096795 is an exact clean-master failure. Clean master b491a4e8d, run 29217402091, job 86757242627 reports the same two QA findings:

  1. An ambiguity between (cache::JacobianCache)(::Nothing) and (cache::JacobianCache{<:AbstractSciMLOperator})(u).
  2. ExplicitImports flags SingleCacheStorage from FunctionWrappersWrappers as non-public at lib/NonlinearSolveBase/ext/NonlinearSolveBaseForwardDiffExt.jl:85.

Neither site is changed by #1057. The existing draft #1055 touches the ForwardDiff extension for a separate singular implicit-sensitivity fix but does not remove or publicize the SingleCacheStorage access, so it does not resolve this QA cluster. A separate clean-main reproduce/bisect/fix audit will handle these findings, preserving the public-API rule rather than allow-listing the internal name without justification. PR job: https://github.com/SciML/NonlinearSolve.jl/actions/runs/29250645138/job/86820096795

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Additional CI classification:

  • Core / Julia pre / Ubuntu job 86820153071 reaches the same clean-master allocation regression already recorded above: sweep_per_step = 154.4, arclength_per_step = 80.0 at homotopy_alloc_tests__item1.jl:83-84.
  • Bounds / Julia LTS / macOS job 86820153551 never checks out or runs package code. GitHub fails while resolving action download metadata after two retries, ending with Service Unavailable / Failed to resolve action download info.

Thus neither new red check indicates a #1057 code failure. Bounds job: https://github.com/SciML/NonlinearSolve.jl/actions/runs/29250644943/job/86820153551 ; prerelease Core job: https://github.com/SciML/NonlinearSolve.jl/actions/runs/29250644943/job/86820153071

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Substantive target CI is green across the full Julia matrix:

  • lib/NonlinearSolveBase Core: LTS, Julia 1, and Julia pre all SUCCESS.
  • lib/NonlinearSolveQuasiNewton Core: LTS, Julia 1, and Julia pre all SUCCESS.
  • QuasiNewton QA is also SUCCESS.

This independently exercises the changed triangular Jacobian solve utility and the Broyden regression coverage on all three supported Julia lanes. The remaining red checks are the separately documented clean-master downgrade, Base-QA, and root homotopy-allocation clusters, plus the macOS action-download service outage.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

The newly red Wrappers / Julia 1 / macOS check is the same GitHub Actions service outage as the previously classified Bounds/macOS job. It never checks out or runs package code: action metadata resolution receives Service Unavailable, retries twice, and exits with Failed to resolve action download info. Job: https://github.com/SciML/NonlinearSolve.jl/actions/runs/29250644943/job/86820153644

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Additional CI classification: the macOS/current Core job (Julia 1.12.6 ARM64) fails the same clean-main homotopy allocation assertions already reproduced locally, not the Broyden change in this PR. sweep_per_step(prob_big) evaluated to 1111.68 and arclength_per_step(prob_big) to 576.0, both against < 48, at test/Core/homotopy_alloc_tests__item1.jl:83-84. Job: https://github.com/SciML/NonlinearSolve.jl/actions/runs/29250644943/job/86820153544. A dedicated clean-main bisect/fix audit remains tracked separately.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

The macOS/pre-release Core lane independently repeats the same clean-main homotopy allocation cluster: on Julia pre ARM64, sweep_per_step(prob_big) is 926.4 and arclength_per_step(prob_big) is 480.0 against < 48 at homotopy_alloc_tests__item1.jl:83-84. This adds platform/version evidence to the already assigned allocation audit; it is not a Broyden regression. Job: https://github.com/SciML/NonlinearSolve.jl/actions/runs/29250644943/job/86820153709.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebase and local verification update:

  • Safely rebased onto merged master 579761dd2 (Align downgrade floors with LinearSolve 4.3 #1059) after fetching both remotes and confirming the feature remote still pointed to my prior 17d95c4e9 with no contributor commits.
  • During review I found that the new alias-preservation branch copied an A === rhs input into A_buf but the generic fallback still passed the overwritten A. Commit b6a3f4ede now passes A_solve and adds an explicit nontriangular aliased-buffer regression.
  • Julia 1.12.6 NonlinearSolveBase Core exited 0; the affected linsolve_identity!! workspace (#1020) set passed 53/53 and the package tests passed.
  • Julia 1.12.6 NonlinearSolveQuasiNewton Core exited 0: Broyden 594/594, Klement 216/216, LimitedMemoryBroyden 99/99, their iterator/termination sets all passed, and Bad Broyden with a triangular true Jacobian passed 2/2.
  • Whole-repository Runic check exited 0 across 373 Julia files; git diff --check also passed.

The rebased branch was pushed with an exact --force-with-lease tied to the verified old remote SHA; no maintainer changes were overwritten.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

CI classification update:

  • Root Core on Julia current and LTS, plus the root downgrade lane, reach the robustness suite and fail only because the Brown almost-linear bad-Broyden case unexpectedly passes its existing broken assertion. That clean-main environment-sensitive failure is tracked in Brown bad-Broyden broken test is environment-sensitive #1056.
  • NonlinearSolveBase QA reproduces the two clean-main findings already recorded here: the JacobianCache Nothing/operator ambiguity and the non-public FunctionWrappersWrappers.SingleCacheStorage access. The public-API prerequisite is FunctionWrappersWrappers.jl GPU Compatibility for 1D Arrays  #70; a separate clean-main audit is in progress.
  • The focused Base and QuasiNewton tests for this PR passed locally, including the aliased nontriangular fallback regression. No CI failure seen so far points to the matrix-preservation change.

This comment only classifies observed logs; it does not suppress or alter any failing test.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Targeted CI result for this PR:

  • NonlinearSolveBase Core passed on Julia LTS, current, and prerelease; its downgrade lane also passed.
  • NonlinearSolveQuasiNewton Core passed on Julia LTS, current, and prerelease; QuasiNewton QA and downgrade also passed.
  • The remaining root Julia-current/LTS/downgrade failures are the clean-main Brown unexpected pass tracked in Brown bad-Broyden broken test is environment-sensitive #1056. Root prerelease fails the independently reproduced clean-main homotopy allocation regression (154.4/80.0 B per step), for which a separate focused fix is being prepared. NonlinearSolveBase QA is also under its separate public-API/ambiguity audit.

These CI results agree with the local Base and QuasiNewton test runs recorded above.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

CI has now reached a terminal state: 85 checks passed, 3 matrix placeholders were skipped, and the 7 failures are the already-classified clean-main NonlinearSolveBase QA / root Core / downgrade failures tracked separately. Every intended NonlinearSolveQuasiNewton Core lane passed on Julia LTS, current, prerelease, and downgrade, along with QuasiNewton QA and the local 594/594 Broyden regression coverage. No additional #1057-specific failure appeared.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebased onto current master at 41681c5e4 after #1060 merged; the branch head is now 3a0b031bb.

Post-rebase local validation on Julia 1.12 and the current resolved graph:

  • NonlinearSolveBase, GROUP=Core: passed (workspace 53/53, routing 12/12, operator Jacobian 1/1, dense-LU 14/14).
  • NonlinearSolveQuasiNewton, GROUP=Core: passed (Broyden 594/594, Klement 216/216, LimitedMemoryBroyden 99/99, triangular regression 2/2).
  • Exact root Broyden file with local-path assertions: the target generalized-Rosenbrock bad-Broyden case passed; the file later stopped at the already tracked clean-master Brown unexpected pass (Brown bad-Broyden broken test is environment-sensitive #1056).
  • Whole-repository Runic and git diff --check: passed.

No tests or broken-test annotations were changed to accommodate the separate Brown result.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

The two immediate downgrade failures are pre-test resolver failures, not failures in this branch's code:

  • root Downgrade pins SciMLOperators 1.22.0 while the current in-tree NonlinearSolveQuasiNewton requires 1.24.
  • SimpleNonlinearSolve downgrade likewise pins 1.22.0 while the current in-tree NonlinearSolveBase requires 1.24.

Both jobs exit during Pkg.test resolution before loading or running tests. This is the known downgrade-action stale-floor conflict being addressed by julia-actions/julia-downgrade-compat#54 and #55; it is independent of the triangular Broyden changes here.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Current functional CI failures are all separately tracked clean-master issues; the target triangular Broyden regression is not failing:

No allocation threshold or broken-test annotation is changed in this PR. The full current-graph NonlinearSolveQuasiNewton Core suite, including the new triangular regression 2/2, passed locally after the rebase.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Scheduled CI follow-up at 2026-07-13 18:47 EDT: the target NonlinearSolveQuasiNewton matrix remains green on Julia LTS, current, and prerelease; QuasiNewton QA and downgrade are also green. The six completed failures are the already-classified clean-master resolver/ExplicitImports, homotopy-allocation (#1061), and generalized-Rosenbrock (#1058/#1057 dependency-chain) failures. No new #1057-specific failure appeared. Twenty-one macOS/GPU jobs are still queued, so this is not yet a terminal CI state.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebased validation update (2026-07-15):

The PR body now contains the full commands/results summary. New CI run: https://github.com/SciML/NonlinearSolve.jl/actions/runs/29402824369

Copy link
Copy Markdown
Member Author

Rebased the two focused commits onto current upstream 6264446f4 (#1070) after fetching the remote branch and confirming it still ended at 8ff2f090f with no intervening contributor commits.

git range-diff reports both patches unchanged:

73ed37451 = fd564a107 Preserve triangular Broyden initialization
8ff2f090f = 30613e651 Preserve aliased matrix in fallback solve

The branch update used an exact --force-with-lease against 8ff2f090f.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the agent/fix-broyden-true-jacobian branch from 30613e6 to e1e5599 Compare July 16, 2026 18:20
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebased the two focused commits onto current upstream 60736b5b6 (#1079) after fetching the fork branch and verifying its exact prior tip 30613e651, two-commit history, authorship, comments, and absence of reviews.

git range-diff reports both patches unchanged:

fd564a107 = c555453c1 Preserve triangular Broyden initialization
30613e651 = e1e559958 Preserve aliased matrix in fallback solve

The update used --force-with-lease pinned to the audited prior SHA. No contributor commits were overwritten.

Copy link
Copy Markdown
Member Author

Exact LinearSolve-downstream validation (2026-07-17):

  • Baseline: NonlinearSolve 60736b5b6a30a42001c1938647362ef138b1c6a7, LinearSolve main 0496f1351010ed70c1ee764534012dbc674ede59, Julia 1.10.11, Linux x86_64, GROUP=Core, fresh isolated depot. The root Broyden block reproduced 94 passed, 1 failed, 1 errored, 19 broken: generalized Rosenbrock alg Make iip a compile-time constant #4 had residual 4.3999999999999995 > 0.001, and Brown alg Make iip a compile-time constant #4 unexpectedly passed.
  • Validation: applied this PR's two code/test changes to the exact NonlinearSolve baseline and developed the local patched NonlinearSolveBase/NonlinearSolveQuasiNewton plus the exact LinearSolve main checkout.
  • NonlinearSolveBase GROUP=Core passed, including linsolve_identity!! workspace 53/53, routing 12/12, operator dispatch 1/1, and dense-LU allocations 14/14.
  • NonlinearSolveQuasiNewton GROUP=Core passed: Broyden 810/810 + 2/2 + 27/27; Klement 297/297 + 2/2 + 27/27; LimitedMemoryBroyden 135/135 + 2/2 + 27/27; dedicated generalized-Rosenbrock regression 2/2.
  • Patched root Core reached the Broyden block with 95 passed, 0 failed, 1 errored, 19 broken. Generalized Rosenbrock alg Make iip a compile-time constant #4 passed normally. The sole remaining error was the environment-sensitive Brown unexpected pass already tracked in Brown bad-Broyden broken test is environment-sensitive #1056.
  • Full-repository Runic check and git diff --check passed.

No assertion, tolerance, broken-list, or skip change was used. This confirms that the downstream failure seen against clean LinearSolve main is pre-existing NonlinearSolve behavior, and that this PR addresses the Rosenbrock half without masking Brown.

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