Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arrays of non-vector arrays are broken #131

Closed
simonster opened this issue Aug 12, 2014 · 3 comments
Closed

Arrays of non-vector arrays are broken #131

simonster opened this issue Aug 12, 2014 · 3 comments

Comments

@simonster
Copy link
Member

e.g.:

julia> a = Matrix{Int}[[1 2; 3 4], [5 6; 7 8]];

julia> @save "test.jld" a

julia> @load "test.jld"
ERROR: `convert` has no method matching convert(::Type{Array{Int64,2}}, ::Array{Int64,1})
 in convert at base.jl:13
 in copy! at abstractarray.jl:149
 in convert at array.jl:220
 in read at /home/simon/.julia/HDF5/src/jld.jl:358
 in read at /home/simon/.julia/HDF5/src/jld.jl:286
 in read at /home/simon/.julia/HDF5/src/jld.jl:241
 in anonymous at no file

Presumably broken by 3a95493

hdfgroup.org appears to be down right now, but if my memory serves, HDF5 vlen types have to no way to encode >1 dimension, so we'd either need to go back to saving these as references or come up with an alternative way to encode the dimensions.

FWIW, I'm not entirely convinced that saving arrays of arrays as vlen types is a great idea. With #102 (which I've incorporated into my work on #27) we can actually preserve the relationships among Julia data structures when saving them and reading them back out, but I think this can only be made to work if arrays are stored as references. OTOH, if performance entirely sucks otherwise we may have to use vlens, and maybe investigate saving an additional ID so that we can determine when two vlens point to the same array when we read the data out. But with #27 the performance situation with references should be at least a little better since we will no longer have to save the type name with every dataset.

@timholy
Copy link
Member

timholy commented Aug 12, 2014

Argh.

Yes, I agree that the combination of #27 and #102 is a new chapter, and we might want to revisit this decision.

Given that you're partway through #27, what do you recommend as the best fix? I'm fine with reverting or whatever.

@simonster
Copy link
Member Author

I think we should do some benchmarks with #27 before making a final decision. (I can presently get through all of the write tests and about half of the read tests, so it shouldn't be too far off.) For now, the easiest fix is probably to restrict the write method signature to vectors, which I think should be sufficient. I'll do that momentarily.

@timholy
Copy link
Member

timholy commented Aug 12, 2014

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants