JuliaIO / JLD2.jl

HDF5-compatible file format in pure Julia
Other
537 stars 84 forks source link

Upgrading old structures with parametric composite types #474

Closed dcerkoney closed 9 months ago

dcerkoney commented 1 year ago

Hello,

I am encountering a small issue related to upgrading old structures on load using JLD2 v0.4.32 which I am struggling to debug.

Are parametric composite types supported for JLD2.rconvert and JLD2.Upgrade? If so, how should the type mapping be specified?

Here is a small example illustrating the behavior I am expecting, which does not work as intended:

using JLD2
using Test

struct OldStruct{T}
    x::T
end

jldsave("test.jld2"; data=OldStruct{Float64}(0))

struct NewStruct{T}
    x::T
    b::Bool
end

function JLD2.rconvert(::Type{NewStruct{T}}, nt::NamedTuple) where {T}
    return NewStruct{T}(nt.x, true)
end

@test JLD2.rconvert(NewStruct{Float64}, (x=0.0,)) == NewStruct{Float64}(0, true)

d = load(
    "test.jld2",
    "data";
    typemap=Dict("Main.OldStruct{Float64}" => JLD2.Upgrade(NewStruct{Float64}))
)

@test_broken d isa NewStruct{Float64}
@test_broken !(d isa OldStruct{Float64})

If there is a way to achieve something like this, it would be great if we could add an example to the docs.

Thanks, Daniel

JonasIsensee commented 1 year ago

Hi @dcerkoney,

this should be possible with (note, that I removed the type parameter from the OldStruct)

d = load(
    "test.jld2",
    "data";
    typemap=Dict("Main.OldStruct" => JLD2.Upgrade(NewStruct{Float64}))
)

However, because of a bug, this still doesn't work. I think I fixed it in #475

475

dcerkoney commented 1 year ago

Great, thanks for the swift response!

dcerkoney commented 12 months ago

One problem comes to mind with the proposed solution: doesn't it mean that an archive holding different specializations of OldStruct{T} cannot be straightforwardly upgraded to hold NewStruct{T} objects?

For example, suppose we have a situation like this:

using JLD2

struct OldStruct{T}
    x::T
end

struct NewStruct{T}
    x::T
    b::Bool
end

function JLD2.rconvert(::Type{NewStruct{T}}, nt::NamedTuple) where {T}
    return NewStruct{T}(nt.x, true)
end

jldsave(
    "test.jld2";
    a=OldStruct{Int}(1),
    b=OldStruct{Vector{Int}}([1, 0]),
    c=OldStruct{Matrix{Int}}([1 0; 0 1])
)

Since I have defined rconvert for all T, I would then hope to see one or both of the following options:

# Option 1: Try to upgrade every OldStruct{T} to NewStruct{T} where {T}
d1 = load(
    "test.jld2";
    typemap=Dict("Main.OldStruct" => JLD2.Upgrade(NewStruct))
)

# Option 2: Upgrade OldStruct{T} to NewStruct{T} for explicitly specified values of T
d2 = load(
    "test.jld2";
    typemap=Dict(
        "Main.OldStruct{Vector{Int}}" => JLD2.Upgrade(NewStruct{Vector{Int}}),
        "Main.OldStruct{Matrix{Int}}" => JLD2.Upgrade(NewStruct{Matrix{Int}}),
    )
)

Any thoughts?

JonasIsensee commented 12 months ago

Would it work to define a single upgrade statement that does "Main.OldStruct" => Upgrade(NewStruct)

dcerkoney commented 12 months ago

Sure, that approach makes sense to me.