Fix immutable-state dense-Jacobian adjoint fallback - #1686
Draft
ChrisRackauckas-Claude wants to merge 6 commits into
Draft
ChrisRackauckas-Claude wants to merge 6 commits into
ChrisRackauckas-Claude wants to merge 6 commits into
Conversation
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Codex <noreply@openai.com> Agent-Harness: Codex CLI 0.157.1 Agent-Model: gpt-6-sol Agent-Session: local session, transcript at /home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/log.txt on amdci2.julia.csail.mit.edu
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Codex <noreply@openai.com> Agent-Harness: Codex CLI 0.157.1 Agent-Model: gpt-6-sol Agent-Session: local session, transcript at /home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/log.txt on amdci2.julia.csail.mit.edu
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Codex <noreply@openai.com> Agent-Harness: Codex CLI 0.157.1 Agent-Model: gpt-6-sol Agent-Session: local session, transcript at /home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/log.txt on amdci2.julia.csail.mit.edu
The out-of-place parameter-Jacobian helper fell back to allocating `jacobian`, which hard-coded forward finite differences and used the parameter element type as the FiniteDiff return type. Preserve the sensealg finite-difference scheme and promote with the state/RHS type, and call SciMLBase.isinplace for the mutability branch. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Cursor Agent <noreply@cursor.com> Agent-Harness: Cursor Agent CLI 2026.09.26-dd393fe Agent-Model: auto Agent-Session: local session, transcript at /home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-cursor/log.txt on amdci2.julia.csail.mit.edu
Cover GaussAdjoint and QuadratureAdjoint with autodiff=false central differences (analytical derivative 1), complex state with real parameters, mixed Float64/Float32 precision, and the allocating immutable-state jacobian path. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Cursor Agent <noreply@cursor.com> Agent-Harness: Cursor Agent CLI 2026.09.26-dd393fe Agent-Model: auto Agent-Session: local session, transcript at /home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-cursor/log.txt on amdci2.julia.csail.mit.edu
Switch the complex-state and mixed-precision Core7 cases to immutable SVector state/parameters with out-of-place discrete costs so they hit the allocating `jacobian` branch. Clarify the RHS-eltype comment, drop the PR-history note, and cover InterpolatingAdjoint in the central-FD probe. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Cursor Agent <noreply@cursor.com> Agent-Harness: Cursor Agent CLI 2026.09.26-dd393fe Agent-Model: auto Agent-Session: local session, transcript at /home/crackauc/sandbox/agent-jobs/SMS-b/jobs/1684-cursor/log.txt on amdci2.julia.csail.mit.edu
Draft
3 of 5 tasks
This branch has not been deployed
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 out-of-place Gauss adjoint had no dense-Jacobian (
autojacvec=false) vector-Jacobian method, so its automatic fallback errored on immutable states. This adds that method and computes parameter Jacobians without writing into immutable ODE outputs for both Gauss and Quadrature adjoints. The regression cases compare their gradients with the existing through-solve ForwardDiff reference.Part of #1684: #1684
Verification
All Julia commands used
TMPDIR=/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/tmpandJULIA_DEPOT_PATH=/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/depot:/home/crackauc/.julia.Before the fix, with the new Gauss regression case:
JULIA_LOAD_PATH=@:/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/tmp/jl_CDEroy /home/crackauc/.juliaup/bin/julia +1 --project -e 'include("test/Core7/adjoint_oop.jl")'exited 1. The first error line wasERROR: LoadError: MethodError: no method matching _vecjacobian(::SVector{2, Float64}, ::SVector{2, Float64}, ::SVector{4, Float64}, ...)(long type signature clipped here); the output ended within expression starting at .../test/Core7/adjoint_oop.jl:280.After the fix,
JULIA_LOAD_PATH=@:/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/tmp/jl_DfddKF /home/crackauc/.juliaup/bin/julia +1 --project -e 'include("test/Core7/adjoint_oop.jl")'exited 0 with no output.On the final commit,
GROUP=Core7 /home/crackauc/.juliaup/bin/julia +1 --project -e 'using Pkg; Pkg.test()'exited 0:On the final commit,
GROUP=QA /home/crackauc/.juliaup/bin/julia +1 --project -e 'using Pkg; Pkg.test()'exited 0:The initial fix caused the existing Core3 complex-state test to fail:
JULIA_LOAD_PATH=@:/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/tmp/jl_Z6GtNx /home/crackauc/.juliaup/bin/julia +1 --project -e 'include("test/Core3/gauss_zygote_inplace.jl")'exited 1 withAssertionError: eltype(fx) == T2inGaussAdjoint edge cases. The same command on the final commit exited 0; its tail wasGaussAdjoint edge cases | 4 4 2m50.8s.On the final commit,
JULIA_LOAD_PATH=@:/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/tmp/jl_Z6GtNx /home/crackauc/.juliaup/bin/julia +1 --project -e 'include("test/Core7/adjoint_oop.jl")'exited 0 with no output.Julia Runic
--checkon all changed files,typosover the diff, andgit diff --checkexited 0.PR CI for the final commit passed Core7 on Julia 1, 1.11, and LTS (https://github.com/SciML/SciMLSensitivity.jl/actions/runs/36345924040). The initial fix commit failed Core3 LTS; the follow-up commit preserves the cached Jacobian for mutable outputs, and Core3 CI passes on Julia 1, 1.11, and LTS (https://github.com/SciML/SciMLSensitivity.jl/actions/runs/36345924040).
Not verified
Core1, Core6, and Core8 are separate failures in the issue; this PR does not fix them. Core1 locally reproduced the Mooncake
MethodError: no constructors have been defined for Anyattest/Core1/concrete_solve_derivatives.jl:451. Core6 locally reproducedTypeError: in TrackedReal, in V, expected V<:Real, got Type{ComplexF64}attest/Core6/complex_matrix_finitediff.jl; a standalone ReverseDiff reproducer with no SciML imports failed in a clean scratch environment with the same error. Those diagnostic group runs were stopped after the errors appeared, so neither is a completed test run. GPU and downstream jobs were not run locally; the documentation job was skipped. In PR CI, GPU failed with aMixed GPU/CPUscalar-indexing error also present on master (https://github.com/SciML/SciMLSensitivity.jl/actions/runs/35232980090/job/105241613630). The downstream DeepEquilibriumNetworks Core check failed in the unchanged ReverseDiffVJP parameter-struct guard; the comparable master integration run stopped on an API rate limit before its tests, so that failure is not confirmed as pre-existing. Core8 locally reproducedEnzymeNoTypeErrorinEnzyme through initattest/Core8/desauty_dae_mwe.jl:123; its diagnostic group was stopped after that error and did not complete. The corrected Core3 group is still running locally on Julia LTS in a clean scratch worktree, and the remaining follow-up CI checks are pending. There was no independent reviewer in this single-agent job.Reviewer attention
Please review the container reconstruction in
_vecjacobianand the allocation in the out-of-place parameter-Jacobian fallback, particularly for array types beyondSVector.Please ignore this draft until reviewed by @ChrisRackauckas.
Risk assessment
🤖 Generated with Codex CLI 0.157.1 (model: gpt-6-sol); session transcript: /home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/log.txt on amdci2.julia.csail.mit.edu
Update (review findings, Cursor Agent)
Addressed the independent review of PR head
0ddf381/ follow-up0ae106d:jacobiannow usesdiff_type(alg)instead of hard-coded forward differences, and promotes the FiniteDiff return type with the state/RHS element type forParamJacobianWrapper(and related wrappers)._adjoint_param_jacobianusesSciMLBase.isinplace.dp = 1), complex-state/real-parameter FD, mixed Float64/Float32 FD, and an allocating immutable-statejacobianunit check for both GaussAdjoint and QuadratureAdjoint.Fail before / pass after
Reviewer's central-FD probe against the pre-mutable-cache head (
0ddf381, reviewtestenv):Same probe on
origin/master(baseenv):0.9999999888946556/0.9999999888946554, 2 Pass.Allocating
jacobianunit probe on0ae106d(before this update): returned forward-FD value2.4901161193847656vs central reference0.9999999888946555(2 Fail). After the fix: both match central (2 Pass).Verification (this update)
All Julia commands used
TMPDIR=/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-cursor/tmpandJULIA_DEPOT_PATH=/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-cursor/scratch/depot:/home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-codex/depot:/home/crackauc/.julia.GROUP=Core7 julia +1 --project -e 'using Pkg; Pkg.test()'exited 0:GROUP=QA julia +1 --project -e 'using Pkg; Pkg.test()'exited 0:Focused
include("test/Core7/adjoint_oop.jl")via the review-matched testenv exited 0; new testsets each 2 Pass (central, complex, mixed precision, allocating jacobian).Runic
--checkon changed files andtyposover the diff exited 0.Not verified
Other Core groups, GPU/downstream, docs, and the separate terminal-only SVector
setindex!issue noted in the review (not introduced by the allocating-jacobian path). Core1/Core6/Core8 remain out of scope for this PR.Reviewer attention
Please push back if promoting
eltype(f.u)witheltype(p)is the wrong FiniteDiff return type for any custom array container, or if always respectingdiff_typein allocatingjacobianshould remain forward for some non-parameter callers.Please ignore this draft until reviewed by @ChrisRackauckas.
Cursor Agent CLI 2026.09.26-dd393fe, model auto; transcript: /home/crackauc/sandbox/agent-jobs/SciMLSensitivity.jl/jobs/1684-cursor/log.txt on amdci2.julia.csail.mit.edu
Update 2 (round-2 review: SVector coverage)
The complex-state and mixed-precision regressions now use immutable
SVectorstates/parameters with out-of-placedgdu_discrete, so they take the allocatingjacobianpath that the eltype promotion guards. The central-FD probe also coversInterpolatingAdjoint(thediff_type(alg)change affects more than Gauss/Quadrature). Comments no longer narrate PR history.Mutation check (Union reverted → fail; fix → pass)
In a scratch copy of this head, the element-type branch was reverted from
f isa Union{ParamGradientWrapper, SciMLBase.ParamJacobianWrapper, RODEParamJacobianWrapper}to
f isa ParamGradientWrapperonly. The updated SVector probes then fail:With the Union restored on this head, the same probes pass:
Focused
scratch/newtests.jl(central + complex + mixed + allocating jacobian) on the fixed package:Verification (this update)
All Julia commands used
TMPDIR=/home/crackauc/sandbox/agent-jobs/SMS-b/jobs/1684-cursor/tmpandJULIA_DEPOT_PATH=.../SMS-b/jobs/1684-cursor/scratch/depot:.../SciMLSensitivity.jl/jobs/1684-cursor/scratch/depot:.../1684-codex/depot:$HOME/.julia, withjulia +1(1.13.1).GROUP=Core7 julia +1 --project=<fixenv> -e 'using Pkg; Pkg.test("SciMLSensitivity")'exited 0:GROUP=QA julia +1 --project=<fixenv> -e 'using Pkg; Pkg.test("SciMLSensitivity")'exited 0:Julia Runic
--checkonsrc/derivative_wrappers.jlandtest/Core7/adjoint_oop.jlexited 0;typosover the diff andgit diff --checkexited 0.Behavior note (round-2 non-blocking)
Allocating
jacobiannow honorsdiff_type(alg)for all callers, not only Gauss/Quadrature. Withautodiff=falseand the defaultVal{:central}, InterpolatingAdjoint/BacksolveAdjoint stop silently using forward differences (master could return ~2.49 on the central-FD probe problem; this head recovers ~1).diff_type=Val{:complex}with a complex-valued RHS now errors loudly instead of silently falling back to forward FD.Not verified
Other Core groups, GPU/downstream, docs, SteadyStateAdjoint/SDE numerical effect of
diff_type, and the pre-existing terminal-only SVectorsetindex!path in QuadratureAdjoint. Core1/Core6/Core8 remain out of scope.Reviewer attention
Please push back if
t = [0.0, 1.0](instead of terminal-only[1.0]) is too weak a discrete-cost setup for the SVector eltype cases, or if promotingeltype(f.u)witheltype(p)is wrong for some custom container.Please ignore this draft until reviewed by @ChrisRackauckas.
Cursor Agent CLI 2026.09.26-dd393fe, model auto; transcript: /home/crackauc/sandbox/agent-jobs/SMS-b/jobs/1684-cursor/log.txt on amdci2.julia.csail.mit.edu