Repository navigation
Stop forcing ForwardDiff chunk size 1 on AutoSpecialize-wrapped Jacobians - #1364
Closed
ChrisRackauckas-Claude wants to merge 2 commits into
Closed
ChrisRackauckas-Claude wants to merge 2 commits into
ChrisRackauckas-Claude wants to merge 2 commits into
Conversation
…ians The wrapped residual's FunctionWrappersWrapper only has Dual signatures for chunk size 1, and standardize_forwarddiff_tag stamped that chunk size onto autodiff unconditionally, so every Jacobian column was computed one partial at a time regardless of state size (~4x slower init on a 100-dim dense NewtonRaphson problem than the unwrapped path). Baking pickchunksize(length(u)) into the wrapper's own type instead (the initial approach tried here) makes the whole solver cache type-unstable, since a Vector's length isn't part of its type -- @inferred catches this on existing AutoFiniteDiff and ForwardDiff inference tests. construct_jacobian_cache unwraps AutoSpecializeCallable for ForwardDiff/PolyesterForwardDiff instead, mirroring the existing Enzyme/sparsity-detector unwrap, so DifferentiationInterface sees the raw user function and ForwardDiff picks its own, usually much wider, chunk size. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Agent-Harness: Claude Code Agent-Model: claude-sonnet-5-5 Agent-Session: https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv
…guarantee CI (https://github.com/SciML/NonlinearSolve.jl/actions/runs/37664751556/job/112948772756, and the matching Downgrade / Core job) caught a real regression the earlier commit's local testing missed by only covering NonlinearSolveBase and NonlinearSolveFirstOrder: top-level test/Core/homotopy_alloc_tests__item1.jl, "Continuation drivers infer a concrete inner cache (AutoSpecialize functor wrapper) + zero-alloc stepping". Unwrapping AutoSpecializeCallable for ForwardDiff (construct_jacobian_cache calling get_raw_f) routes the Jacobian-AD call through the callable's `orig::Any` field, which is deliberately type-erased so every wrapped function shares one Julia type (that's the whole point of AutoSpecialize's "norecompile" design). For a simple top-level closure this happened to still infer concretely in isolated testing (Julia's inliner can narrow `::Any` back down through a short, local call chain), but it does not survive the homotopy/continuation drivers' longer call graph (FixLambda / AugmentedHomotopy / HomotopyResidual functors, wrapped and `init`-ed behind a function barrier): there, `SciMLBase.init` on the wrapped inner problem now infers to a non-concrete type, and the reused inner cache's per-step `solve!`/`reinit!` that used to be allocation-free starts allocating. This is the same chunk-size-depends-on-a-runtime-value problem the first (also reverted) approach hit, one level up: ForwardDiff's own chunk size for a plain `Vector` is necessarily a runtime quantity (`length(u0)` is not part of the type), and anything that makes the wrapped path's behavior depend on it -- baking the chunk into the wrapper's own type, or bypassing the wrapper via `get_raw_f` so DI picks its own chunk -- reopens the same inference gap. A fixed, wrapper-only chunk size greater than 1 is not an option either: ForwardDiff requires chunk <= length(u), and the continuation drivers' own inner problems are frequently length 1 (a single homotopy parameter), so any such change would throw on exactly the workload this invariant protects. Net effect: this PR is now a no-op versus master for lib/NonlinearSolveBase/src/{autospecialize,jacobian}.jl and test/runtests.jl, restoring the concrete-cache / zero-alloc guarantee this checks for. The N=100 dense-Vector ForwardDiff Jacobian speedup from the earlier commits does not survive this constraint with the current AutoSpecializeCallable design; see the PR body for the full trade-off. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Agent-Harness: Claude Code Agent-Model: claude-sonnet-5-5 Agent-Session: https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv
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.
Please ignore until reviewed by @ChrisRackauckas.
BLOCKED: no fix found that keeps both properties — this PR is now a no-op vs master
Two earlier commits on this branch tried to fix the audited regression (
AutoSpecializeforcing ForwardDiff chunk size 1 on wrapped IIP residuals, ~4x slower
init/solveon a100-dim dense
NewtonRaphsonproblem). Both were caught by CI and reverted. The currentHEAD is an exact no-op against
masterfor every file this PR touches(
lib/NonlinearSolveBase/src/autospecialize.jl,src/jacobian.jl,test/runtests.jl) —git diff origin/masteris empty.What was tried, and why each one breaks something real
Build the wrapper's own
FunctionWrappersWrapperwithpickchunksize(length(u0))Dual signatures instead of a hardcoded 1, and stamp that chunk back onto
autodiff.This worked and gave the measured speedup below, but it makes the solver cache
type-unstable: a
Vector's length isn't part of its type, so a chunk size derivedfrom
length(u0)can't be a compile-time constant.@inferredcatches this — theexisting
lib/NonlinearSolveFirstOrder/test/inference_tests__item1.jl("Full init:in-place, TrustRegion/NewtonRaphson", using
AutoFiniteDiff, not even ForwardDiff)started failing, because
wrapfun_iipruns for any wrapped IIP problem regardless ofwhich AD backend is eventually selected. Reverted.
Unwrap
AutoSpecializeCallableto the raw user function forAutoForwardDiff/AutoPolyesterForwardDiffinconstruct_jacobian_cache,mirroring the existing Enzyme/sparsity-detector unwrap right next to it. This fixed (1)'s
@inferredfailure and gave the same speedup, and passed everything tested at the time—
NonlinearSolveBaseandNonlinearSolveFirstOrderCore+QA, plus a newregression test. But top-level CI caught what that local testing missed:
test/Core/homotopy/continuation tests
(https://github.com/SciML/NonlinearSolve.jl/actions/runs/37664751556/job/112948772756,
same failure on the
Downgrade / Downgrade Tests - Corejob).HomotopySweep/ArcLengthContinuationwrap their residual in aFixLambda/AugmentedHomotopy/HomotopyResidualfunctor, build the inner problem at theAutoSpecializedefault, andrely on
init(inner_prob, NewtonRaphson())inferring to exactly one concrete cachetype so the driver's reused inner cache and every per-step
solve!/reinit!areallocation-free —
test/Core/homotopy_alloc_tests__item1.jl, "Continuation drivers infera concrete inner cache (AutoSpecialize functor wrapper) + zero-alloc stepping", pins this.
get_raw_f(f::AutoSpecializeCallable) = f.origreturns a field declared::Any(deliberately — that's what lets every wrapped function share one Julia type, the whole
point of
AutoSpecialize's "norecompile" design). For a trivial top-level closure inisolated testing, Julia's inliner happened to narrow
::Anyback down to the concreteclosure type through a short, local call chain, so my own ad hoc inference checks passed —
but it does not survive the continuation drivers' longer call graph (functor construction
and
inithappen behind a function barrier, several calls apart). Reverted (this commit).Both failures are the same underlying problem one level up: ForwardDiff's own chunk
size for a plain
Vectoris necessarily a runtime quantity (length(u0)isn't part ofthe type), and anything that makes the wrapped path's behavior depend on it — baking the
chunk into the wrapper's type, or routing around the wrapper so DI picks its own chunk —
reopens the same inference gap, just at a different point in the call graph.
Why a fixed, wrapper-only chunk > 1 isn't an option either
ForwardDiff requires
chunk <= length(u)(confirmed directly:ForwardDiff.JacobianConfig(f!, du, u0, Chunk{12}())on a length-2u0throwsArgumentError: chunk size cannot be greater than ForwardDiff.structural_length(x) (12 > 1)scaled to length 2 here). The continuation drivers' own inner problems are frequently
length 1 — a single homotopy parameter, exactly
test/Core/homotopy_alloc_tests__item1.jl'sown
HomotopyProblem(Hiip!, [4.0], [4.0]). A hardcoded chunk of, say, 4 would not just betype-stable-but-wrong, it would crash on that workload. The only chunk size that is
simultaneously a compile-time literal (required for the concrete-cache invariant) and valid
for every possible state size (required for correctness) is 1.
Changing
AutoSpecializeCallable'sorig::Anyfield into a type parameter (soget_raw_fstays concrete) was considered and rejected: it would make
AutoSpecializeCallable{FW,F}depend on the user's exact function type
F, reintroducing per-closure recompilation —undoing the entire reason
AutoSpecializetype-erasesorigin the first place. That's adifferent, much larger change than a performance-regression fix, out of scope here.
Reproduction (local, top-level
NonlinearSolve,GROUP=Core-equivalent)With commit f1da103 (approach 2, now reverted) checked out:
Full before/after on the four CI-reported assertions (
taskset-pinned,NewtonRaphsoninner, Julia 1.12.4; CI's own numbers differ in magnitude — different Julia version/
environment, and
@allocatedis sensitive to that — but the qualitative failure,isconcretetype == false, is the same root cause and reproduces locally either way):isconcretetype(inner_rts[1])inner_rtsisUnion-widenedUnion)cache_solve_allocs == 0sweep_cache_allocs == 0sweep_per_step(prob_big) < 48arclength_per_step(prob_big) < 48The
Downgrade / Downgrade Tests - Corefailure(https://github.com/SciML/NonlinearSolve.jl/actions/runs/37664751136/job/112940995685) is
the same test, same assertion, same line (
homotopy_alloc_tests__item1.jl:31) — it isthis PR's regression, not a pre-existing/unrelated failure.
N=10 / N=100 numbers (for the record — not realized by the current, reverted HEAD)
Dense in-place
NewtonRaphson, SPD-matrix residual, Julia 1.12.4,taskset -c 3,128-core AMD EPYC 7502 (shared box):
N=100 init/solve would be ~3.7–3.8x faster under approach 2, but that approach breaks
zero-alloc continuation stepping, so it isn't shippable.
What was not verified
AutoSpecializeCallabletype-parameter redesign mentioned above as a real patch (onlyreasoned about why it's out of scope) — it's a bigger, breaking-adjacent change to
AutoSpecialize's core mechanism, not a quick fix, and would need its own PR and review.Anything a reviewer should push back on
isconcretetype/zero-alloc guarantee forAutoSpecialize-wrapped functorresiduals (continuation drivers) should outrank fixing the dense-
VectorForwardDiffchunk-1 regression, given the current
AutoSpecializeCallabledesign can't give both. Ibelieve it should (a crash on
chunk > length(u)for small homotopy-continuationproblems, or a correctness-by-luck inference artifact that already failed once in CI, are
both worse than shipping no change), but that's a judgment call, not mine to make final.
AutoSpecializeCallabletype-parameter redesign named above — a separate, bigger PR.Risk assessment
git diff origin/masteris empty for every file this PRtouches. This commit undoes the only functional changes this branch ever made.
(
isconcretetype(inner_rts[1]) == false), confirmed fixed by the revert(
== true);Downgrade / Downgrade Tests - Coretraced to the same assertion/line.leave open as the record of what was tried and why, per @ChrisRackauckas's call.
🤖 Generated with Claude Code (model: claude-sonnet-5-5)
https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv