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

Unifying error messages. #747

Closed
trappmartin opened this issue Apr 4, 2019 · 3 comments
Closed

Unifying error messages. #747

trappmartin opened this issue Apr 4, 2019 · 3 comments

Comments

@trappmartin
Copy link
Member

At the moment the error messages are all over the place and highly inconsistent. For example the assume function in HMC uses three different types of error messages.

I think we should have a consistent way of doing this. Maybe we can have a simple function that generates those error messages, e.g.

tmsg(msg::AbstractString, scope:: AbstractString) = "Turing [$scope]: $msg"

which would be used as follows:

@assert length(dists) == 1 tmsg("Turing only support vectorizing i.i.d distribution", "HMC.assume")
@assert size(var) == size(rs) tmsg("Variable and random number dimension unmatched.", "HMC.assume")
error("Unsupported variable container", "HMC.assume")

I guess we could also write a macro for this but I'm not sure its worth the efforts.

@cpfiffer
Copy link
Member

cpfiffer commented Apr 9, 2019

What does everyone think about an additional keyword to sample or another global state function like turnprogress that would silence internal notifications?

We have a PR open at TuringTutorials where NUTS spits out a crazy amount of warnings due (I think) to a prior with a large variance. Here's a short example of this:

┌ Info: [Turing] looking for good initial eps...
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/support/hmc_core.jl:240
[NUTS{Turing.Core.ForwardDiffAD{40},Union{}}] found initial ϵ: 6.4
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/support/hmc_core.jl:235
┌ Warning: 120.93421495873957 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 172.5055540294431 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 156.60472556096468 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 445.7969834587392 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 1342.9204474442015 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 4165.919809816427 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 13271.612008129292 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 23600.035453047334 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 73338.99300485148 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96
┌ Warning: 224004.49369259353 exceeds 5.0; capped to 5.0 for numerical stability
└ @ Turing.Inference /home/saumya/.julia/packages/Turing/r03H1/src/inference/adapt/stepsize.jl:96

It's pretty annoying. I think we get a lot of errors that are maybe not super useful, and it'd be nice if we could do something here.

We might have internal error levels reported with a macro or a function, as Martin suggested. It'd be a pretty quick one to write --- it'll be very similar to the @turing_testset macro we have for the testing suite. I can imagine using warnings internally with differing levels of importance. If you want to know about tiny numerical errors, or every time MH rejects a sample or something, we can have a reporting level for that.

Here's how I see using that interally:

@twarn 6 "Everything is okay, just thought I should check in and see if you needed anything."
@twarn 0 "Your inference is horrible, the sampler's broken, everything is bad, and I'm quitting."

I might be able to specify the level of warnings I'm willing to see, like 3 or less, using Turing.setwarn(3), or in the sampler call sample(. . ., verbose = 3).

Thoughts? I agree that the warnings should be streamlined, because right now you usually get far too much information that is not particularly actionable.

@yebai
Copy link
Member

yebai commented Apr 23, 2019

The following code snippet is used in several places, it might be helpful to define a utility function for better modularity

alg_str = isa(alg, HMC) ? "HMC" :
isa(alg, HMCDA) ? "HMCDA" :
isa(alg, SGHMC) ? "SGHMC" :
isa(alg, SGLD) ? "SGLD" :
isa(alg, NUTS) ? "NUTS" : "Hamiltonian"

@yebai yebai added this to the 0.7 milestone May 15, 2019
@yebai yebai mentioned this issue Jul 4, 2019
56 tasks
@yebai yebai modified the milestones: 0.7, 0.8 Oct 3, 2019
@mohamed82008
Copy link
Member

I think this was solved in #965. Please re-open if you think more work is needed.

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