Skip to content

Commit 2bcca7a

Browse files
ChrisRackauckas-ClaudeChrisRackauckasclaude
authored
Fix copy method for StatefulJacobianOperator with Tuple parameters (#753)
* Fix copy method for StatefulJacobianOperator with Tuple parameters - Add `_safe_copy` helper functions to handle immutable types (Tuple, NamedTuple, Number) that don't need copying - Fix the `Base.copy` method for `StatefulJacobianOperator` to use `_safe_copy(J.p)` instead of `applicable(copy, J.p) ? copy(J.p) : J.p` which fails for Tuple types - Fix typo: `nohting` -> `nothing` in `JacobianOperator.copy` This fixes the JFNK (Jacobian-Free Newton-Krylov) example from the documentation which was failing with: MethodError: no method matching copy(::NTuple{4, Float64}) Fixes #752 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * Add regression test for issue #752 and fix stale ForwardDiff dependency - Add regression test for copy(StatefulJacobianOperator) with Tuple and NamedTuple parameters to prevent future regressions - Test the _safe_copy helper functions for Tuple, NamedTuple, Number, and Array types - Remove ForwardDiff from main deps (it's only needed for tests, already in extras) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: ChrisRackauckas <[email protected]> Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent 22728ed commit 2bcca7a

File tree

3 files changed

+104
-6
lines changed

3 files changed

+104
-6
lines changed

lib/SciMLJacobianOperators/Project.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name = "SciMLJacobianOperators"
22
uuid = "19f34311-ddf3-4b8b-af20-060888a46c0e"
3-
authors = ["Avik Pal <[email protected]> and contributors"]
43
version = "0.1.12"
4+
authors = ["Avik Pal <[email protected]> and contributors"]
55

66
[deps]
77
ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b"

lib/SciMLJacobianOperators/src/SciMLJacobianOperators.jl

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -404,22 +404,30 @@ function get_dense_ad(ad::AutoSparse)
404404
return dense_ad
405405
end
406406

407+
# Helper functions for safe copying of parameters
408+
# Tuples and NamedTuples are immutable and don't need copying
409+
# Numbers are immutable as well
410+
_safe_copy(x::Tuple) = x
411+
_safe_copy(x::NamedTuple) = x
412+
_safe_copy(x::Number) = x
413+
_safe_copy(x) = copy(x)
414+
407415
function Base.copy(J::JacobianOperator{iip, T}) where {iip, T}
408-
return JacobianOperator{iip,T}(
416+
return JacobianOperator{iip, T}(
409417
J.mode,
410418
J.jvp_op,
411419
J.vjp_op,
412420
J.size,
413421
J.input_cache === nothing ? nothing : copy(J.input_cache),
414-
J.output_cache === nothing ? nohting : copy(J.output_cache)
422+
J.output_cache === nothing ? nothing : copy(J.output_cache)
415423
)
416424
end
417425

418426
function Base.copy(J::StatefulJacobianOperator)
419427
return StatefulJacobianOperator(
420428
copy(J.jac_op),
421429
J.u === nothing ? nothing : copy(J.u),
422-
applicable(copy, J.p) ? copy(J.p) : J.p
430+
_safe_copy(J.p)
423431
)
424432
end
425433

@@ -429,10 +437,8 @@ function Base.copy(J::StatefulJacobianNormalFormOperator{T}) where {T}
429437
J.jvp_operator === nothing ? nothing : copy(J.jvp_operator),
430438
J.cache === nothing ? nothing : copy(J.cache)
431439
)
432-
433440
end
434441

435-
436442
export JacobianOperator, VecJacOperator, JacVecOperator
437443
export StatefulJacobianOperator
438444
export StatefulJacobianNormalFormOperator

lib/SciMLJacobianOperators/test/core_tests.jl

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,3 +311,95 @@ end
311311
@test check_no_stale_explicit_imports(SciMLJacobianOperators) === nothing
312312
@test check_all_qualified_accesses_via_owners(SciMLJacobianOperators) === nothing
313313
end
314+
315+
@testitem "Copy with Tuple parameters (Issue #752)" begin
316+
using ADTypes, SciMLBase, ForwardDiff, FiniteDiff
317+
using SciMLJacobianOperators
318+
using SciMLJacobianOperators: _safe_copy
319+
320+
# Test _safe_copy helper functions
321+
@testset "_safe_copy helpers" begin
322+
# Tuples should be returned as-is (immutable)
323+
t = (1.0, 2.0, 3.0, 4.0)
324+
@test _safe_copy(t) === t
325+
326+
# NamedTuples should be returned as-is (immutable)
327+
nt = (a = 1.0, b = 2.0)
328+
@test _safe_copy(nt) === nt
329+
330+
# Numbers should be returned as-is (immutable)
331+
@test _safe_copy(42.0) === 42.0
332+
@test _safe_copy(42) === 42
333+
334+
# Arrays should be copied
335+
arr = [1.0, 2.0, 3.0]
336+
arr_copy = _safe_copy(arr)
337+
@test arr_copy == arr
338+
@test arr_copy !== arr # Different object
339+
end
340+
341+
# Test copy(StatefulJacobianOperator) with Tuple parameter
342+
# This is a regression test for issue #752 where JFNK failed with:
343+
# MethodError: no method matching copy(::NTuple{4, Float64})
344+
@testset "copy(StatefulJacobianOperator) with Tuple p" begin
345+
# Brusselator-like problem setup with Tuple parameters
346+
function brusselator_f(du, u, p)
347+
α, β, γ, δ = p
348+
du[1] = α + u[1]^2 * u[2] -+ 1) * u[1]
349+
du[2] = β * u[1] - u[1]^2 * u[2]
350+
return nothing
351+
end
352+
353+
# Parameters as a Tuple (this is what caused issue #752)
354+
p_tuple = (1.0, 3.0, 1.0, 1.0)
355+
u0 = [1.0, 1.0]
356+
fu0 = similar(u0)
357+
brusselator_f(fu0, u0, p_tuple)
358+
359+
prob = NonlinearProblem(
360+
NonlinearFunction{true}(brusselator_f), u0, p_tuple)
361+
362+
jac_op = JacobianOperator(
363+
prob, fu0, u0; jvp_autodiff = AutoForwardDiff(), vjp_autodiff = AutoFiniteDiff())
364+
sop = StatefulJacobianOperator(jac_op, u0, p_tuple)
365+
366+
# This should not throw MethodError: no method matching copy(::NTuple{4, Float64})
367+
sop_copy = copy(sop)
368+
369+
@test sop_copy.p === p_tuple # Same tuple (immutable, no copy needed)
370+
@test sop_copy.u == u0
371+
@test sop_copy.u !== sop.u # Different array object (copied)
372+
373+
# Verify the copied operator still works
374+
v = [1.0, 0.0]
375+
result_original = sop * v
376+
result_copy = sop_copy * v
377+
@test result_original result_copy
378+
end
379+
380+
# Test copy(StatefulJacobianOperator) with NamedTuple parameter
381+
@testset "copy(StatefulJacobianOperator) with NamedTuple p" begin
382+
function simple_f(du, u, p)
383+
du[1] = p.a * u[1] + p.b * u[2]
384+
du[2] = p.c * u[1] + p.d * u[2]
385+
return nothing
386+
end
387+
388+
p_namedtuple = (a = 1.0, b = 2.0, c = 3.0, d = 4.0)
389+
u0 = [1.0, 1.0]
390+
fu0 = similar(u0)
391+
simple_f(fu0, u0, p_namedtuple)
392+
393+
prob = NonlinearProblem(
394+
NonlinearFunction{true}(simple_f), u0, p_namedtuple)
395+
396+
jac_op = JacobianOperator(
397+
prob, fu0, u0; jvp_autodiff = AutoForwardDiff(), vjp_autodiff = AutoFiniteDiff())
398+
sop = StatefulJacobianOperator(jac_op, u0, p_namedtuple)
399+
400+
# This should not throw
401+
sop_copy = copy(sop)
402+
403+
@test sop_copy.p === p_namedtuple # Same NamedTuple (immutable)
404+
end
405+
end

0 commit comments

Comments
 (0)