Closed Octogonapus closed 3 months ago
JonasIsensee
commented
2 years ago Hi @Octogonapus , thanks for reporting this. I've labelled this as a bug.
Defining Base.convert(::Type{Foo.Inner}, x) = Foo.Inner(x.x, 0) works for me.
Another workaround is described in #242
Octogonapus
commented
2 years ago I think this could be handled with a look-ahead inside constructrr, but that might be computationally expensive. The type of Outer.a would also need to be serialized properly. Right now, it's just Any instead of Vector{Inner}.
┌ Debug: constructrr
│ T = Main.Foo.Outer
│ dt = JLD2.CompoundDatatype(0x00000008, [:a], [0], JLD2.H5Datatype[JLD2.BasicDatatype(0x37, 0x00, 0x00, 0x00, 0x00000008)])
│ attrs =
│ 1-element Vector{JLD2.ReadAttribute}:
│ JLD2.ReadAttribute(:julia_type, JLD2.ReadDataspace(0x00, 0x00, 4891), 0xff, 560, 4891)
└ @ JLD2 ~/Documents/code/JLD2.jl/src/data/reconstructing_datatypes.jl:121
┌ Debug: field_datatypes
│ field_datatypes = JLD2.RelOffset[]
└ @ JLD2 ~/Documents/code/JLD2.jl/src/data/reconstructing_datatypes.jl:123
┌ Debug: isconcretetype(T)
│ isconcretetype(T) = true
└ @ JLD2 ~/Documents/code/JLD2.jl/src/data/reconstructing_datatypes.jl:126
┌ Debug: dtnames & mapped
│ dtnames =
│ Dict{Symbol, Int64} with 1 entry:
│ :a => 1
│ mapped =
│ 1-element BitVector:
│ 0
└ @ JLD2 ~/Documents/code/JLD2.jl/src/data/reconstructing_datatypes.jl:138
┌ Debug: determine dtrr
│ field_datatypes = JLD2.RelOffset[]
│ dtindex = 1
│ dt.members =
│ 1-element Vector{JLD2.H5Datatype}:
│ JLD2.BasicDatatype(0x37, 0x00, 0x00, 0x00, 0x00000008)
└ @ JLD2 ~/Documents/code/JLD2.jl/src/data/reconstructing_datatypes.jl:161
┌ Debug: dtrr
│ dtrr = JLD2.ReadRepresentation{Any, JLD2.RelOffset}()
│ typeof(dtrr) = JLD2.ReadRepresentation{Any, JLD2.RelOffset}
│ (typeof(dtrr)).parameters = svec(Any, JLD2.RelOffset)
└ @ JLD2 ~/Documents/code/JLD2.jl/src/data/reconstructing_datatypes.jl:168
┌ Debug: constructrr type intersection
│ readtype = Any
│ wstype = Vector{Main.Foo.Inner} (alias for Array{Main.Foo.Inner, 1})
│ typeintersect(readtype, wstype) = Vector{Main.Foo.Inner} (alias for Array{Main.Foo.Inner, 1})
│ hasmethod(convert, Tuple{Type{wstype}, readtype}) = false
└ @ JLD2 ~/Documents/code/JLD2.jl/src/data/reconstructing_datatypes.jl:171If readtype was Vector{Inner} instead of Any, and we knew if reconstructing Inner was required, then we would know to reconstruct Outer. I'm not too familiar with this package's internals, can you think of a way to do that? I can work on implementing it.
I may also work on implementing a "reconstructed types only" feature, as described in #242.
JonasIsensee
commented
2 years ago If readtype was Vector{Inner} instead of Any, and we knew if reconstructing Inner was required, then we would know to reconstruct Outer. I'm not too familiar with this package's internals, can you think of a way to do that? I can work on implementing it.
I've looked at the source code and the files. Sadly I'm pretty sure this is not possible.
JLD2 does not store the julia-eltype of structs. It only stores the HDF5 types of fields.
It's easy to map this in the simple cases of e.g. floats and ints but arrays are mutable objects
and are therefore stored separately and referenced by a RelOffset.
constructrr cannot know the field type of Outer.a - not without a serious reworking of JLD2 internals. (See #316 and #338 for my unfinished attempts)
I believe it may be more useful to implement the explicit type remapping that's already working in #316. If you're interested in contributing, have a look at #316 and what I did with the type remapping. Essentially, every file gets a dictionary that maps datatype names to datatypes in the running session. This already worked nicely but #316 tried to do anonymous functions as well, which I didn't finish.
JonasIsensee
commented
2 years ago Since I had the code lying around already I ended up doing the above myself in #376 . I hope this helps
Octogonapus
commented
3 months ago After revisiting this today, I think this is not a problem on the latest version. As seen in https://github.com/Octogonapus/JLD2Benchmark we can use the Upgrade and rconvert mechanisms to efficiently load into the struct containing the added field.
Description
Using Julia v1.6 and JLD2 v0.4.19, JLD2 fails to load a struct containing a vector of a type which must be reconstructed. In principal, this makes sense, as the reconstructed type isn't convertible to the required type on the vector. This behavior isn't very helpful, though, as the exception occurs inside JLD2 code so I don't have an opportunity to address the problem.
My code can provide a way to convert a reconstructed
Foo.Innerinto a trueFoo.Inner. Would it be possible to pass this function to JLD2 somehow? I'm not sure how my conversion method would get the required type information, though. I can see that JLD2 has this information, since it creates type names like thisJLD2.ReconstructedTypes.var"##Main.Foo.Inner#258". That is probably enough to implement the conversion function I have in mind. In our own code, we save the type name as a string next to the data, and then use that to convert any reconstructed types into the true types. We need to do this conversion in the first place because we need to dispatch on these types; dispatch won't work with reconstructed types.I'd also like I note that I can help work on this change, if it's something that would be accepted.
Workaround
I could change the type of
Outer.atoVector, but this not ideal as I want to require a specific eltype on that vector.I could also define a method of
convertlike thisBase.convert(::Type{Inner}, other) = Inner(other.x, 0), but I can't think of a way to provide a tight enough type bound onotherso that this function won't get accidentally called.MWE
Step 1: Serialize
Step 2: Deserialize with an Added Field
Exception