Skip to content

Commit ed4ca38

Browse files
Fix linear-problem fast path and DelayDiffEq @test_broken
_rosenbrock_jac_reuse_decision was returning (true, true) for linear operator ODEs with a WOperator-wrapped W because the WOperator check fired before the linear-function check. That made Rosenbrock23 rebuild J and W every step on ODEFunction(::MatrixOperator) problems (seen as nw = 628 / 454 in lib/OrdinaryDiffEqNonlinearSolve linear_solver_tests expecting nw == 1), instead of building W once and reusing it. Reorder the decision so the linear-function branch fires immediately after the iter<=1 check, matching the pre-reuse do_newJW behavior: - iter <= 1 -> (true, true) [first step must build W] - islin -> (false, false) [reuse afterwards, regardless of W type] - non-adaptive / WOperator / mass-matrix / composite checks follow Also flip DelayDiffEq jacobian.jl:57 from @test_broken to @test — the nWfact_ts[] == njacs[] assertion now passes with the Rosenbrock J/W accounting from this PR. Verified locally: Rosenbrock23 on ODEFunction(MatrixOperator, mass_matrix=...): nw=1 Rodas5P+KrylovJL convergence test: L2 order 5.004 (tight reltol) Rosenbrock23 convergence on prob_ode_2Dlinear: order 1.996 Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 88400a3 commit ed4ca38

2 files changed

Lines changed: 17 additions & 13 deletions

File tree

lib/DelayDiffEq/test/interface/jacobian.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ using Test
5454

5555
# check number of function evaluations
5656
@test !iszero(nWfact_ts[])
57-
@test_broken nWfact_ts[] == njacs[]
57+
@test nWfact_ts[] == njacs[]
5858
@test iszero(sol_Wfact_t.stats.njacs)
5959
@test_broken nWfact_ts[] == sol_Wfact_t.stats.nw
6060

lib/OrdinaryDiffEqDifferentiation/src/derivative_utils.jl

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,22 @@ function _rosenbrock_jac_reuse_decision(integrator, cache, dtgamma)
4646
return (true, true)
4747
end
4848

49+
# First iteration: always compute J and W.
50+
if integrator.iter <= 1
51+
return (true, true)
52+
end
53+
54+
# Linear problems: J is the constant linear operator and W wrapping it
55+
# doesn't need rebuilding after the first step — SciMLOperators update
56+
# W's internal gamma in place. This must come before the non-adaptive,
57+
# WOperator, mass-matrix, and composite checks so linear operator ODEs
58+
# (e.g. ODEFunction(::MatrixOperator), including with a mass matrix) get
59+
# the fast path — matches the pre-reuse `do_newJW` behavior.
60+
islin, _ = islinearfunction(integrator)
61+
if islin
62+
return (false, false)
63+
end
64+
4965
# Non-adaptive solves always recompute.
5066
# J reuse provides negligible benefit with prescribed timesteps and causes
5167
# IIP/OOP inconsistency (adaptive solves have step rejections that reset
@@ -61,13 +77,6 @@ function _rosenbrock_jac_reuse_decision(integrator, cache, dtgamma)
6177
return (true, true)
6278
end
6379

64-
# Linear problems: J is constant, never needs recomputation after first eval.
65-
# But W still depends on dtgamma, so always rebuild W.
66-
islin, _ = islinearfunction(integrator)
67-
if islin
68-
return (false, false)
69-
end
70-
7180
# Mass matrix (DAE) problems always recompute.
7281
# Stale Jacobians cause order reduction for DAEs because the algebraic
7382
# constraint derivatives must remain accurate. See Steinebach (2024).
@@ -81,11 +90,6 @@ function _rosenbrock_jac_reuse_decision(integrator, cache, dtgamma)
8190
return (true, true)
8291
end
8392

84-
# First iteration: always compute J and W.
85-
if integrator.iter <= 1
86-
return (true, true)
87-
end
88-
8993
# Commit pending_dtgamma from previous step if it was accepted.
9094
# This ensures rejected steps don't pollute last_dtgamma.
9195
naccept = integrator.stats.naccept

0 commit comments

Comments
 (0)