Skip to content

Loading saved Chain object fails #1091

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

Closed
jonasmac16 opened this issue Feb 3, 2020 · 4 comments
Closed

Loading saved Chain object fails #1091

jonasmac16 opened this issue Feb 3, 2020 · 4 comments

Comments

@jonasmac16
Copy link
Contributor

As briefly discussed on the slack channel, when I save chain via write("chn.jls",chn) and load in the same session with read("chn.jls", Chains) it works fine, but in a new session with the same packages loaded I get the error below. This error goes away if you set save_state=false during sampling. It appears to be caused by .info field which refers to original Turing model which doesn't exist in the new session.

ERROR: UndefVarError: ###model#461 not defined
Stacktrace:
 [1] deserialize_datatype(::Serialization.Serializer{IOStream}, ::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:1193
 [2] handle_deserialize(::Serialization.Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:775
 [3] deserialize(::Serialization.Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [4] deserialize_datatype(::Serialization.Serializer{IOStream}, ::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:1217
 [5] handle_deserialize(::Serialization.Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:775
 [6] deserialize(::Serialization.Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [7] deserialize_datatype(::Serialization.Serializer{IOStream}, ::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:1217
 [8] handle_deserialize(::Serialization.Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:775
 [9] deserialize(::Serialization.Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [10] deserialize_datatype(::Serialization.Serializer{IOStream}, ::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:1212
 [11] handle_deserialize(::Serialization.Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:775
 [12] deserialize(::Serialization.Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [13] deserialize_datatype(::Serialization.Serializer{IOStream}, ::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:1217
 [14] handle_deserialize(::Serialization.Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:775
 [15] deserialize(::Serialization.Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [16] deserialize_datatype(::Serialization.Serializer{IOStream}, ::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:1217
 [17] handle_deserialize(::Serialization.Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:775
 [18] deserialize(::Serialization.Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [19] handle_deserialize(::Serialization.Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:782
 [20] deserialize(::Serialization.Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [21] handle_deserialize(::Serialization.Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:825
 [22] deserialize(::Serialization.Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [23] deserialize at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:709 [inlined]
 [24] #open#271(::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{,Tuple{}}}, ::typeof(open), ::typeof(Serialization.deserialize), ::String, ::Vararg{String,N} where N) at ./io.jl:298
 [25] open at ./io.jl:296 [inlined]
 [26] read(::String, ::Type{Chains}) at /home/jonas/.julia/packages/MCMCChains/YabhG/src/fileio.jl:4
 [27] top-level scope at REPL[37]:1
@devmotion
Copy link
Member

The general problem is that generic functions need to be redefined in a new session (anonymous functions should be fine, see PumasAI/PumasPostProcessing.jl#5 (comment)). A simple example: out of the following serialized functions

julia> y = x -> x^2
#7 (generic function with 1 method)

julia> f(x) = x^2
f (generic function with 1 method)

julia> using Serialization

julia> serialize("y.jls", y)

julia> serialize("f.jls", f)

only the anonymous function can be deserialized in a new session:

julia> using Serialization

julia> deserialize("y.jls")
#7 (generic function with 1 method)

julia> deserialize("f.jls")
ERROR: UndefVarError: #f not defined
Stacktrace:
 [1] deserialize_datatype(::Serializer{IOStream}, ::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:1193
 [2] handle_deserialize(::Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:775
 [3] deserialize(::Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [4] handle_deserialize(::Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:782
 [5] deserialize(::Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [6] handle_deserialize(::Serializer{IOStream}, ::Int32) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:825
 [7] deserialize(::Serializer{IOStream}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:722
 [8] deserialize at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:709 [inlined]
 [9] #open#271(::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::typeof(open), ::typeof(deserialize), ::String) at ./io.jl:298
 [10] open at ./io.jl:296 [inlined]
 [11] deserialize(::String) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Serialization/src/Serialization.jl:719
 [12] top-level scope at REPL[3]:1

IMO one should try to avoid serialization if possible since it "is not a stable format and it is possible that you cannot read a serialized object created with another Julia version. It really is only intended for process to process communication.", as this comment on discourse points out.

@jonasmac16
Copy link
Contributor Author

How can we save the Chain objects without setting save_state=false to avoid the above error?

@cpfiffer
Copy link
Member

cpfiffer commented Feb 6, 2020

Honestly the best way to do this is to not store the model at all, and refactor the resume and chain writing functionality. Serialization is a cheap trick that breaks a lot -- we should have an actual file backend like JSON, HDF5, or CSV that stores all the samples. You should be able to resume by giving a chain the model, so we don't need to rely on serializing functions.

For right now you should probably stick to using the save_state keyword that keeps you from having errors.

bors bot pushed a commit to TuringLang/DynamicPPL.jl that referenced this issue Jun 29, 2020
This PR removes `ModelGen` completely. The main motivation for it were the issues that @itsdfish and I experienced when working with multiple processes. The following MWE
```julia
using Distributed
addprocs(4)
@Everywhere using DynamicPPL

@Everywhere @model model() = begin end

pmap(x -> model(), 1:4)
```
fails intermittently

> if not all of these [`@model`] evaluations generate same evaluator and generator functions of the same name (i.e., these var"###evaluator#253" and var"###generator#254" functions). I assume one could maybe provoke the error by defining another model first on the main process before calling @Everywhere @model ....

(copied from the discussion on Slack)

With the changes in this PR, `@model model() = ...` generates only a single function `model` on all workers, and hence there are no issues anymore with the names of the generators and evaluators. The evaluator is created as a closure inside of `model`, which can be serialized and deserialized properly by Julia. So far I haven't been able to reproduce the issues above with this PR.

The only user-facing change of the removal of `ModelGen` (apart from that one never has to construct it, which simplifies the docs and the example @denainjs asked about) is that the `logprob` macro now requires to specify `model = m` where `m = model()` instead of `model = model` (since that's just a regular function from which the default arguments etc of the resulting model can't be extracted). It feels slightly weird that the evaluation is not based "exactly" on the specified `Model` instance but that the other parts of `logprob` modify it (which was the reason I guess for using the model generator before here), but on the other hand this weird behaviour already exists when specifying `logprob` with a `Chains` object. (BTW I'm not sure if we should actually use string macros here, maybe regular functions would be nicer.)

Additionally, I assume (though haven't tested it) that getting rid of the separate evaluator and generator functions will not only simplify serialization and deserialization when working with multiple processes but also when saving models and chains (see e.g. TuringLang/Turing.jl#1091).

Co-authored-by: David Widmann <[email protected]>
bors bot pushed a commit to TuringLang/DynamicPPL.jl that referenced this issue Jun 29, 2020
This PR removes `ModelGen` completely. The main motivation for it were the issues that @itsdfish and I experienced when working with multiple processes. The following MWE
```julia
using Distributed
addprocs(4)
@Everywhere using DynamicPPL

@Everywhere @model model() = begin end

pmap(x -> model(), 1:4)
```
fails intermittently

> if not all of these [`@model`] evaluations generate same evaluator and generator functions of the same name (i.e., these var"###evaluator#253" and var"###generator#254" functions). I assume one could maybe provoke the error by defining another model first on the main process before calling @Everywhere @model ....

(copied from the discussion on Slack)

With the changes in this PR, `@model model() = ...` generates only a single function `model` on all workers, and hence there are no issues anymore with the names of the generators and evaluators. The evaluator is created as a closure inside of `model`, which can be serialized and deserialized properly by Julia. So far I haven't been able to reproduce the issues above with this PR.

The only user-facing change of the removal of `ModelGen` (apart from that one never has to construct it, which simplifies the docs and the example @denainjs asked about) is that the `logprob` macro now requires to specify `model = m` where `m = model()` instead of `model = model` (since that's just a regular function from which the default arguments etc of the resulting model can't be extracted). It feels slightly weird that the evaluation is not based "exactly" on the specified `Model` instance but that the other parts of `logprob` modify it (which was the reason I guess for using the model generator before here), but on the other hand this weird behaviour already exists when specifying `logprob` with a `Chains` object. (BTW I'm not sure if we should actually use string macros here, maybe regular functions would be nicer.)

Additionally, I assume (though haven't tested it) that getting rid of the separate evaluator and generator functions will not only simplify serialization and deserialization when working with multiple processes but also when saving models and chains (see e.g. TuringLang/Turing.jl#1091).

Co-authored-by: David Widmann <[email protected]>
@yebai
Copy link
Member

yebai commented Jan 28, 2021

Fixed by #1105

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

4 participants