Skip to content

Commit bd5af11

Browse files
OOP NSA W-reuse: pass jac_prototype + refresh J each should_update
Fixes two defects in the OOP W-reuse path: (1) closure handed raw WOperator to NonlinearSolve whose jac cache is Matrix, causing convert(Matrix, WOperator) MethodError; (2) J stayed at build-time since update_coefficients!(W, ...) is a no-op for concrete Matrix J. Now stores W in a Ref, passes jac_prototype to inner NonlinearFunction, and recomputes J via calc_J semantics inside should_update. Tests cover Vector and StaticArrays paths.
1 parent 936de56 commit bd5af11

4 files changed

Lines changed: 90 additions & 14 deletions

File tree

lib/OrdinaryDiffEqNonlinearSolve/src/OrdinaryDiffEqNonlinearSolve.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ import OrdinaryDiffEqCore: _initialize_dae!,
5858
import OrdinaryDiffEqDifferentiation: update_W!, is_always_new, build_uf, build_J_W,
5959
WOperator, StaticWOperator, wrapprecs,
6060
build_jac_config, dolinsolve, alg_autodiff,
61-
resize_jac_config!, jacobian2W!, jacobian!
61+
resize_jac_config!, jacobian2W!, jacobian!, jacobian
6262

6363
import StaticArraysCore: StaticArray
6464

lib/OrdinaryDiffEqNonlinearSolve/src/newton.jl

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function initialize!(
5454
nlsolver.status === Divergence ||
5555
abs(inv(dtgamma) / inv(W_γdt) - 1) > alg.new_W_dt_cutoff
5656
if should_update
57-
update_coefficients!(cache.W, uprev, p, tstep; gamma = dtgamma)
57+
_update_nlsolvealg_W_oop!(cache, integrator, dtgamma, tstep)
5858
cache.W_γdt = dtgamma
5959
integrator.stats.nw += 1
6060
cache.new_W = true
@@ -155,6 +155,31 @@ function _update_nlsolvealg_W!(nlcache, integrator, dtgamma, tstep)
155155
return nothing
156156
end
157157

158+
function _update_nlsolvealg_W_oop!(nlcache, integrator, dtgamma, tstep)
159+
(; f, p, uprev, alg) = integrator
160+
mass_matrix = f.mass_matrix
161+
uf = nlcache.uf
162+
if SciMLBase.has_jac(f)
163+
J_new = f.jac(uprev, p, tstep)
164+
else
165+
uf.f = nlsolve_f(f, alg)
166+
uf.p = p
167+
uf.t = tstep
168+
J_new = jacobian(uf, uprev, integrator)
169+
end
170+
W = nlcache.W[]
171+
if W isa StaticWOperator
172+
nlcache.W[] = StaticWOperator(J_new - mass_matrix * inv(dtgamma))
173+
else
174+
copyto!(W.J, J_new)
175+
W.gamma = dtgamma
176+
if nlcache.J !== nothing && ArrayInterface.can_setindex(nlcache.J)
177+
copyto!(nlcache.J, J_new)
178+
end
179+
end
180+
return nothing
181+
end
182+
158183
## compute_step!
159184

160185
@muladd function compute_step!(nlsolver::NLSolver{<:NonlinearSolveAlg, false}, integrator)

lib/OrdinaryDiffEqNonlinearSolve/src/utils.jl

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -508,20 +508,31 @@ function build_nlsolver(
508508
else
509509
(tmp, γ, α, tstep, invγdt, DIRK, p, dt, f)
510510
end
511+
W_ref = use_w_reuse ? Ref{typeof(W)}(W) : nothing
511512
prob = if use_w_reuse
512-
nlf_jac = if W isa StaticWOperator
513-
let Ws = W
514-
(z, p) -> Ws.W
513+
if W isa StaticWOperator
514+
nlf_jac = let W_ref = W_ref
515+
(z, p) -> W_ref[].W
515516
end
517+
jac_prototype = W.W
518+
NonlinearProblem(
519+
NonlinearFunction{false, SciMLBase.FullSpecialize}(
520+
nlf; jac = nlf_jac, jac_prototype = jac_prototype
521+
),
522+
copy(ztmp), nlp_params
523+
)
516524
else
517-
let Ww = W
518-
(z, p) -> Ww
525+
nlf_jac = let W_ref = W_ref
526+
(z, p) -> convert(AbstractMatrix, W_ref[])
519527
end
528+
jac_prototype = copy(convert(AbstractMatrix, W))
529+
NonlinearProblem(
530+
NonlinearFunction{false, SciMLBase.FullSpecialize}(
531+
nlf; jac = nlf_jac, jac_prototype = jac_prototype
532+
),
533+
copy(ztmp), nlp_params
534+
)
520535
end
521-
NonlinearProblem(
522-
NonlinearFunction{false, SciMLBase.FullSpecialize}(nlf; jac = nlf_jac),
523-
copy(ztmp), nlp_params
524-
)
525536
else
526537
NonlinearProblem(
527538
NonlinearFunction{false, SciMLBase.FullSpecialize}(nlf),
@@ -533,8 +544,8 @@ function build_nlsolver(
533544
nlcache = NonlinearSolveCache(
534545
nothing, tstep, nothing, nothing, invγdt, prob, cache,
535546
use_w_reuse ? J : nothing,
536-
use_w_reuse ? W : nothing,
537-
nothing, nothing, nothing,
547+
W_ref,
548+
use_w_reuse ? uf : nothing, nothing, nothing,
538549
zero(tstep), true
539550
)
540551
else

lib/OrdinaryDiffEqNonlinearSolve/test/linear_nonlinear_tests.jl

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
using OrdinaryDiffEqSDIRK, OrdinaryDiffEqBDF, OrdinaryDiffEqRosenbrock, Test, Random,
2-
LinearAlgebra, LinearSolve, ADTypes
2+
LinearAlgebra, LinearSolve, ADTypes, NonlinearSolve, SciMLBase
33
Random.seed!(123)
44

55
A = 0.01 * rand(3, 3)
@@ -95,6 +95,46 @@ sol = @test_nowarn solve(
9595
);
9696
@test length(sol.t) < 20
9797

98+
# NonlinearSolveAlg OOP W-reuse: plain Vector u0 with concrete_jac=true must
99+
# not crash on the first Jacobian evaluation. Regression for two defects
100+
# flagged on #3798: (a) the (z, p) -> W closure handed a raw WOperator to
101+
# NonlinearSolve whose jac cache is a dense Matrix, causing a
102+
# `convert(Matrix, WOperator)` MethodError, and (b) J was frozen at
103+
# build_nlsolver-time since `update_coefficients!(W, ...)` is a no-op when
104+
# W.J is a concrete Matrix.
105+
using OrdinaryDiffEqNonlinearSolve: NonlinearSolveAlg
106+
let
107+
vdp(u, p, t) = [u[2], p[1] * ((1 - u[1]^2) * u[2] - u[1])]
108+
prob = ODEProblem(vdp, [2.0, 0.0], (0.0, 1.0), [1.0e3])
109+
sol = solve(
110+
prob,
111+
TRBDF2(
112+
nlsolve = NonlinearSolveAlg(NewtonRaphson(; autodiff = AutoForwardDiff())),
113+
concrete_jac = true
114+
); reltol = 1e-8, abstol = 1e-10
115+
)
116+
@test SciMLBase.successful_retcode(sol.retcode)
117+
@test sol.stats.njacs > 0
118+
end
119+
120+
# StaticArrays / StaticWOperator variant of the same repro.
121+
using StaticArrays
122+
let
123+
vdp_static(u, p, t) = SVector(u[2], p[1] * ((1 - u[1]^2) * u[2] - u[1]))
124+
prob = ODEProblem(
125+
vdp_static, SVector(2.0, 0.0), (0.0, 1.0), SVector(1.0e3)
126+
)
127+
sol = solve(
128+
prob,
129+
TRBDF2(
130+
nlsolve = NonlinearSolveAlg(NewtonRaphson(; autodiff = AutoForwardDiff())),
131+
concrete_jac = true
132+
); reltol = 1e-8, abstol = 1e-10
133+
)
134+
@test SciMLBase.successful_retcode(sol.retcode)
135+
@test sol.stats.njacs > 0
136+
end
137+
98138
sol = @test_nowarn solve(prob, TRBDF2(autodiff = AutoFiniteDiff()));
99139
@test length(sol.t) < 20
100140
sol = @test_nowarn solve(

0 commit comments

Comments
 (0)