search

JuliaIO / BSON.jl

Other
158 stars 39 forks source link

Allow saving IdDict #70

Closed DhairyaLGandhi closed 4 years ago

DhairyaLGandhi commented 4 years ago

Another approach to solve the issues in saving parameters from Flux/ Zygote directly, for structures based on using IdDict. In the basic saving and loading test, things seem to work fine, but I would like to know if this would break any assumptions.

MWE:

julia> d = IdDict(:a => 1, :b => rand(3))
IdDict{Symbol,Any} with 2 entries:
  :a => 1
  :b => [0.819137, 0.668358, 0.611674]

# without the PR:
julia> @save "d.dict" d
# error: Access to undefined reference

cc @MikeInnes

DhairyaLGandhi commented 4 years ago

Ref #47, FluxML/Flux.jl#737 and sidesteps #8, which came up in this use case the most, apart from where user code was causing undefs in places where they shouldn't be.

DhairyaLGandhi commented 4 years ago

Does it make it a technically breaking change? I am not sure if it can read dumps that were made prior to the breakage since the dictionary is different now.

MikeInnes commented 4 years ago

I guess it could technically be a breaking change if anyone had managed to save an IdDict without any undefs in it, but that seems relatively unlikely.

I only feel a bit uneasy about applying this to all AbstractDicts, and suggest the more convservative Union{IdDict,Dict} for now.