Repository navigation
Pick a GPU-compatible default AD when u0 lacks fast scalar indexing - #1368
ChrisRackauckas-Claude wants to merge 2 commits into
Conversation
`select_forward_mode_autodiff`, `select_reverse_mode_autodiff`, and `select_jacobian_autodiff` scalar-index the state when probing ForwardDiff-based backends, which fails for arrays such as `JLArray` under `allowscalar(false)`. When `ArrayInterface.fast_scalar_indexing(u0)` is `false`, candidate backends are now restricted to finite-differencing ones (`AutoFiniteDiff`/`AutoFiniteDifferences`); if none is loaded, the `ArgumentError` names the fix. Vector/StaticArray problems are unaffected (fast scalar indexing stays `true`), verified by a byte-identical before/after comparison of `solve` output on a Vector problem. Adds a JLArray regression test (`allowscalar(false)`, NewtonRaphson and TrustRegion) via a test-only `[extras]`/`[targets]` dependency on JLArrays (MIT-licensed, matching NonlinearSolveFirstOrder). Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude <noreply@anthropic.com> Agent-Harness: Claude Code 2.1.293 Agent-Model: claude-sonnet-5 Agent-Session: https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv
Extends the JLArray regression test to cover matrix-free Krylov linsolve and the Yuan/Bastin trust-region radius updates, which exercise the VJP operator a reviewer flagged as a possible scalar-indexing crash site. Running them confirms `DifferentiationInterface`'s `GPUArraysCoreExt` already makes that path GPU-safe; the tests lock in that behavior. Raises the `LinearSolve` compat floor from "5.6" to "5.14": at the lower floor, `LinearSolve`'s default-algorithm cache construction eagerly runs `cholesky!` on every candidate algorithm, including on the non-square augmented system `MoreTrustRegionDescent` builds for `JLArray`, before any algorithm is chosen. Bisected against a faithful reproduction of the downgrade-sublibraries job (every dependency pinned to the exact versions that job installs); `LinearSolve` 5.6.0-5.13.0 fail, 5.14.0+ pass, and the fix is independent of the `JLArrays` version. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude <noreply@anthropic.com> Agent-Harness: Claude Code 2.1.293 Agent-Model: claude-sonnet-5 Agent-Session: https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv
Independent review (Devin CLI 3000.11.3, model fusion-claude-opus-5-5-high-sidekick-swe-2-medium): CHANGES, risk medium. Full reviewVERDICT: CHANGES PR: #1368 (head ba7a6fd, merge-base 0055e31). No linked/closing issue, so there was no parent or tracking issue to check sibling work against. Blocking findings
Non-blocking findings
What I ranAll on Julia 1.12.7 in scratch environments that
What I did not verify
🤖 Posted by an AI agent — harness: Devin CLI 3000.11.3 (review), Claude Code 2.1.285 (posting) · model: fusion-claude-opus-5-5-high-sidekick-swe-2-medium |
What changed and why
Solving a
JLArray-backedNonlinearProblemunderJLArrays.allowscalar(false)failsbecause NonlinearSolveBase's default AD/Jacobian selection
(
select_forward_mode_autodiff/select_reverse_mode_autodiff/select_jacobian_autodiff)always tries ForwardDiff-based backends first, and ForwardDiff's dual-seeding
scalar-indexes the state. When
ArrayInterface.fast_scalar_indexing(u0)isfalse,candidate backends are now filtered to finite-differencing ones
(
AutoFiniteDiff/AutoFiniteDifferences); if none is loaded, theArgumentErrornamesthe fix.
NonlinearSolveBaseForwardDiffExt'sadditional_incompatible_backend_checkapplies the same rule for an explicitly-requested
AutoForwardDiff/AutoPolyesterForwardDiff.Round 2 (responding to a Devin SWE-2 first-pass review, round1/REVIEW.md): the reviewer
raised one blocking correctness concern (could not execute code) and one blocking CI
failure. Both are now resolved — see below.
Review finding 1 (matrix-free/Krylov VJP path) — investigated, NOT confirmed as a bug
The reviewer argued (by static reading only, no execution available) that because
vjp_autodiffresolves toAutoFiniteDiff()for non-scalar-indexable state, anymatrix-free path (
NewtonRaphson(linsolve = KrylovJL_GMRES()), orTrustRegionwithradius_update_scheme = Yuan/Bastin, which eagerly computeJ'fuvia a VJP operator)would crash during
JacobianOperator's eagerprepare_vjp, becauseDifferentiationInterface's genericPullbackSlowfallback builds Jacobian columns viabasis(x, j), which does a scalarsetindex!.I ran all four configurations the reviewer named, on
JLArrayunderallowscalar(false):NewtonRaphson(linsolve=KrylovJL_GMRES()),TrustRegion(linsolve=KrylovJL_GMRES()),TrustRegion(radius_update_scheme=Yuan),TrustRegion(radius_update_scheme=Bastin)(the last two also combined with Krylov). All pass and return the correct root, e.g.:
Root cause of the discrepancy with the reviewer's static analysis:
DifferentiationInterfaceships
ext/DifferentiationInterfaceGPUArraysCoreExt, which overridesDI.basis(a::AbstractGPUArray, i)to wrap the single-element seed write inGPUArraysCore.@allowscalar:This extension auto-loads whenever
GPUArraysCoreis in the session (it is, transitivelyvia
JLArrays/GPUArrays), so thePullbackSlowper-column pushforward-based pullbackis already GPU-safe — the reviewer's read of
basis(and of_prepare_pullback_aux,which calls the zero-argument
basis(x)that never indexes at all) didn't account forthis extension, understandably, since they had no shell to load it and check.
I still added the four configurations above as regression tests in
misc_tests__item11.jl(8 new@tests) to lock in this currently-correct behavior,per the reviewer's "add tests" request.
Review finding 2 (downgrade CI) — confirmed, root-caused, fixed
Reproduced exactly: pinning
JLArrays=0.3.0, ArrayInterface=7.19.0, GPUArrays=11.5.5, GPUArraysCore=0.2.0, LinearSolve=5.6.0, FiniteDiff=2.32.1, DifferentiationInterface=0.7.3, ADTypes=1.13.0, ForwardDiff=1.0.1, KernelAbstractions=0.9.44, Adapt=4.5.2(the exactversions the downgrade-sublibraries job installed, read from its log) while keeping
NonlinearSolveBase/NonlinearSolveFirstOrder/SciMLJacobianOperatorson local devpaths (this PR's actual code) reproduces the CI failure byte-for-byte:
Root cause:
TrustRegion's default descent (MoreTrustRegionDescent) solves itssubproblem through an augmented, non-square
[J; √λ·D]system (4×2 here) viaLinearSolve.DefaultLinearSolver. AtLinearSolve5.6.0–5.13.0, the default-algorithminfrastructure (
_init_default_cacheval, a@generatedfunction) eagerly builds thecache for every algorithm in
DefaultAlgorithmChoice, includingCholeskyFactorization,before any algorithm is actually chosen — and that eager build calls
cholesky!(A)unconditionally, which requires a square matrix. For ordinary
Array/Matrix, aspecialized dispatch path avoids ever touching this generic fallback for non-square input;
for
JLArrayin this oldLinearSolverange, it does not. Not aJLArraysversionissue: I bisected by varying only
LinearSolvewhile holding every other package(including
JLArrays) at the downgrade floor — bumpingJLArraysalone to the newest0.3.x (0.3.3) did not fix it;
LinearSolve5.10.0–5.13.0 still fail,LinearSolve5.14.0 and above (tested up to current 5.18.2) pass.
Fix: raise
NonlinearSolveFirstOrder'sLinearSolvecompat floor from"5.6"to"5.14"(the verified minimum). Bumped the package's patch version (2.10.0→2.10.1)for the compat-only change. Re-ran the exact faithful downgrade reproduction above with
this floor applied (
LinearSolvepinned to5.14.0, everything else unchanged at itsdowngrade floor) and the full, now-expanded
misc_tests__item11.jl(14 assertions) passescleanly.
I did not bump
NonlinearSolveBase's own (lower,"5.4")LinearSolvefloor:MoreTrustRegionDescent/construct_linear_solverlive there, so the same crash could inprinciple occur there too, but
NonlinearSolveBasehas noJLArraytest to exercise itand its own downgrade job doesn't fail, so I left it alone rather than widen the diff —
flagging it here for a maintainer to decide.
Vector-unchanged evidence (round 1, re-confirmed)
solveon aVectorNonlinearProblemwithNewtonRaphson()/TrustRegion()— outputcaptured on
masterand on this branch, byte-identical:Failing-before / passing-after (round 1, re-confirmed)
Before (src reverted to
master, keeping only the test file):ERROR: LoadError: Scalar indexing is disallowedfromForwardDiff.seed!on theJLArray. After (fix applied): passes.Test-group tails (Julia 1.10.12, this branch, round 2)
GROUP=Core(full suite, ~9000 assertions across allalgorithms, now including the 14-assertion
JLArraytest): all pass, e.g.Error/Fail/Some tests did not passanywhere in the run.GROUP=Core: all pass, noError/Failentries (e.g.Bounds transform (#955) 138/138,construct_extension_jac scalar and matrix u0 10/10,Dense LU refactorization allocations 14/14).GROUP=QA:GROUP=QA: still blocked locally by the pre-existing, unrelatedSciMLJacobianOperatorsregistry-lag issue noted in round 1 (confirmed reproduces withthis PR's diff reverted).
typos: clean.Non-blocking review items
Reviewed all; left as-is per the reviewer's own "minor polish" / "user-explicit backends
are allowed to fail" characterization — in particular,
AutoSparse(AutoForwardDiff())passed explicitly by a user on a non-scalar-indexable state is not intercepted by
additional_incompatible_backend_check/is_finite_differences_backend(neither unwrapsAutoSparse). Fixing it would touch the same selection logic again for a case the useropted into explicitly; flagging it as a follow-up rather than widening this PR.
What I did not verify
JLArrays.JLArray(CPU-backed, GPU-arraysemantics).
NonlinearSolveBaseGROUP=QAlocally (pre-existing registry-lag gap, round 1).NonlinearSolveBase's own lowerLinearSolvefloor (5.4) hits the sameMoreTrustRegionDescent/JLArraycrash — plausible by the same root cause, butuntested since no local test exercises it.
All/Everythinggroups.Anything a reviewer should push back on
LinearSolvefloor bump ("5.6"→"5.14") is a compat-only change verified bybisection on this machine, not by reading
LinearSolve's own changelog/PR history forthe actual fixing commit — if a maintainer knows the exact PR, a tighter/more precisely
justified floor might be possible.
NonlinearSolveBase'sLinearSolvefloor should also move, per the note above.Please ignore until reviewed by @ChrisRackauckas.
Risk assessment
NonlinearSolveBase's default AD/Jacobian selection andNonlinearSolveFirstOrder'sLinearSolvecompat floor. No public API signaturechanges; behavior changes only when
ArrayInterface.fast_scalar_indexing(u0) == false(GPU-style arrays) or when installed with a very old (pre-5.14)
LinearSolve;Vectoroutput confirmed byte-identical.
GROUP=Corepass for bothpackages; byte-identical Vector-output comparison; the downgrade-CI failure faithfully
reproduced locally and confirmed fixed by the
LinearSolvefloor bump alone (bisectedagainst
JLArraysseparately to rule it out as the cause).floor on a widely-used code path
🤖 Generated with Claude Code (model: claude-sonnet-5)
https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv