From 1362e4aaee1ce0c3234a63706857e1cd479db4de Mon Sep 17 00:00:00 2001 From: Min-Gu Yoo Date: Mon, 10 Aug 2026 12:20:29 -0700 Subject: [PATCH] fix: preserve the concrete type in DD deepcopy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Base.deepcopy_internal(x::IMAS.DD{T}, dict)` dispatches for every `DD` subtype but built an `IMAS.dd{T}`, while `fieldnames(typeof(x))` kept walking the original's fields. Any container other than `IMAS.dd` — e.g. one defined by a satellite package — therefore came back as an `IMAS.dd`. A bare `deepcopy(dd)` never reached this: IMASdd's `Base.deepcopy(::DD)` shadows it. The bug only surfaced when a dd was reached through a container, such as `deepcopy([dd])` or `FUSE.Checkpoint`'s `getindex`. Adds test/runtests_satellite.jl, which defines a stand-in satellite container on top of `IMAS.DD` and runs it through the same assertions as `IMAS.dd`: direct and nested deepcopy, `_frozen`, and the `_aux[:fxp]` sharing this method exists for. `deepcopy_internal` had no coverage before — only test_fxp.ipynb, which CI does not run. --- src/control/fxp.jl | 2 +- test/runtests.jl | 2 + test/runtests_satellite.jl | 130 +++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 test/runtests_satellite.jl diff --git a/src/control/fxp.jl b/src/control/fxp.jl index 0ea25cb4..8f78af15 100644 --- a/src/control/fxp.jl +++ b/src/control/fxp.jl @@ -22,7 +22,7 @@ function Base.deepcopy_internal(x::IMAS.DD{T}, dict::IdDict) where {T<:Real} # Use the default `deepcopy_internal` to handle the copying process. # This creates a shallow copy of `x`, so all mutable fields will # still be copied correctly later. - new_obj = IMAS.dd{T}() + new_obj = typeof(x)() # Register the new object in `dict` to handle cyclic references. dict[x] = new_obj diff --git a/test/runtests.jl b/test/runtests.jl index 7d79efba..fbebbfb2 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -23,4 +23,6 @@ else include("runtests_extract.jl") include("runtests_plot_recipes.jl") + + include("runtests_satellite.jl") end diff --git a/test/runtests_satellite.jl b/test/runtests_satellite.jl new file mode 100644 index 00000000..a22983dc --- /dev/null +++ b/test/runtests_satellite.jl @@ -0,0 +1,130 @@ +using IMAS +using Test + +""" +Stand-in for a satellite package (e.g. IFEdd) that defines its own concrete +container on top of IMAS's abstract `DD`. Mirrors the shape `GenerateDD` emits, +trimmed to a single reused IMAS IDS. + +Lets IMAS verify that its `DD`-generic code paths work for containers other than +`IMAS.dd`, without depending on any satellite package. +""" +module FakeSatellite + +import IMAS + +const TSD = IMAS.IMASdd.ThreadSafeDicts + +mutable struct FilledFields____my_own_dd <: IMAS.FilledFields + var"requirements"::Bool +end + +mutable struct my_own_dd{T} <: IMAS.DD{T} + var"requirements"::IMAS.requirements{T} + global_time::Float64 + _aux::TSD.ThreadSafeDict{Symbol,Any} + _name::Symbol + _filled::FilledFields____my_own_dd + _frozen::Bool + _threads_lock::ReentrantLock + _in_expression::TSD.ThreadSafeDict{Int,Vector{Symbol}} + _parent::WeakRef +end + +function my_own_dd{T}(; frozen::Bool=false) where {T} + ids = my_own_dd{T}( + IMAS.requirements{T}(; frozen), + 0.0, + TSD.ThreadSafeDict{Symbol,Any}(), + Symbol(""), + FilledFields____my_own_dd(false), + frozen, + ReentrantLock(), + TSD.ThreadSafeDict{Int,Vector{Symbol}}(), + WeakRef(nothing)) + setfield!(ids.requirements, :_parent, WeakRef(ids)) + return ids +end + +my_own_dd(; frozen::Bool=false) = my_own_dd{Float64}(; frozen) + +# satellites register their own paths into the shared registry; keyed by our own +# type, so this cannot collide with IMAS's entries +merge!(IMAS._all_info, Dict( + (my_own_dd, :requirements) => IMAS.Info(String[], "-", "STRUCTURE", "Reused IMAS requirements IDS", true, String[]) +)) + +end # module FakeSatellite + +# Exercises both `IMAS.dd` and a foreign `<: IMAS.DD` container through the same +# assertions. `nested` matters because a bare `deepcopy(dd)` is served by IMASdd's +# `Base.deepcopy(::DD)`, while a dd reached through a container goes through +# `IMAS.deepcopy_internal` in src/control/fxp.jl — a different code path. +function check_deepcopy(make) + original = make() + original.global_time = 3.0 + original.requirements.cost = 7.0 + + copied = deepcopy(original) + @test typeof(copied) === typeof(original) + @test copied !== original + @test copied.global_time == 3.0 + @test copied.requirements.cost == 7.0 + + # a real copy, not an alias + copied.requirements.cost = 9.0 + @test original.requirements.cost == 7.0 + + # the method the container paths funnel into + direct = Base.deepcopy_internal(original, IdDict()) + @test typeof(direct) === typeof(original) + @test direct.requirements.cost == 7.0 + + for (label, wrapped, unwrap) in ( + ("Vector", [original], r -> r[1]), + ("NamedTuple", (dd=original,), r -> r.dd), + ("Dict", Dict(:k => original), r -> r[:k])) + inner = unwrap(deepcopy(wrapped)) + @test typeof(inner) === typeof(original) + @test inner.requirements.cost == 7.0 + end + + # `_frozen` travels with the copy + @test getfield(deepcopy([make(; frozen=true)])[1], :_frozen) + + return nothing +end + +@testset "satellite DD" begin + + @testset "type contract" begin + @test isabstracttype(IMAS.DD) + @test IMAS.dd{Float64} <: IMAS.DD{Float64} + @test FakeSatellite.my_own_dd{Float64} <: IMAS.DD{Float64} + + # a satellite container is a sibling of IMAS.dd, not a subtype of it + @test !(FakeSatellite.my_own_dd{Float64} <: IMAS.dd) + @test !(IMAS.dd{Float64} <: FakeSatellite.my_own_dd) + end + + @testset "deepcopy — IMAS.dd (baseline)" begin + check_deepcopy(IMAS.dd) + end + + @testset "deepcopy — user-defined DD subtype" begin + check_deepcopy(FakeSatellite.my_own_dd) + end + + # `deepcopy_internal` exists so that a deep-copied dd keeps talking to the same + # FXP client rather than cloning it + @testset "deepcopy_internal shares _aux[:fxp]" begin + for make in (IMAS.dd, FakeSatellite.my_own_dd) + original = make() + client = Ref(0) # stands in for a Jedis client + getfield(original, :_aux)[:fxp] = client + copied = Base.deepcopy_internal(original, IdDict()) + @test getfield(copied, :_aux)[:fxp] === client + end + end + +end