Improve `compile` function and some other improvements

[sunxd3]

@yebai an issue is that to compute these fields of BUGSModel https://github.com/TuringLang/JuliaBUGS.jl/blob/855f968dd037b62cecde633b3a4618f47f228802/src/model.jl#L17-L23 it is required to run the model at least once. But without initialization, it is possible that we may run into error because we are sampling from prior for initialization.

Do you think we should always require user to initialize before using the model? If that is the case, compile can just produce the graph and eval_env, and initialize will generate the BUGSModel. This also means that the output of compile cannot be evaluate!!ed.

[yebai]

Do you think we should always require user to initialize before using the model?

Yes, that could work.

[sunxd3]

@yebai I made some update to the interface.

In the current state of this PR, the original compile function is divided into three functions:

• compile now takes model_def and data and produce a BUGSGraph and eval_env
• BUGSModel constructor take BUGSGraph, eval_env (optionally inits, in the case where sampling from prior creates error)
• also user can initialize! a BUGSModel separately

thoughts?

[sunxd3]

@yebai thanks, all good suggestions.

About unflatten (the above unresolved comment), I thought it was more natural to name the function unflatten when the initial_params is a vector of reals. Now the name is changed to initialize!, so it is an overload. https://github.com/TuringLang/JuliaBUGS.jl/blob/03ca2ba14486ee5ba460343ad575a0599cc312d2/src/model.jl#L173-L187

[yebai]

In the current state of this PR, the original compile function is divided into three functions:

Thanks @sunxd3. I released that if we leave the values in eval_env/svi::SimpleVarInfo to a default value (e.g. 0 or undef), we can probably return BUGSModel from compile. This way, users won't need to call the BUGSModel constructor. The simplified workflow becomes:

• compile now takes model_def and data and produces a BUGSModel with default parameter values
• user can initialize! a BUGSModel separately

[sunxd3]

The issue is that we need to determine the dimension of the model and length of variables. These would require model evaluation.

And model evaluation can fail when simply sampling from prior. So we can't do it without having initializations.

[yebai]

And model evaluation can fail when simply sampling from prior. So we can't do it without having initializations.

This is strange -- we should always be able to sample from the prior of a generative model.

Can you provide an example where sampling from the prior fails?

[sunxd3]

An example can be

@bugs begin
    tau_b1 ~ dgamma(1.0E-3,1.0E-3) # equivalently, Distributions.Gamma{Float64}(α=0.001, θ=1000.0)
    b1 ~ dnorm(0.0, tau_b1) # equivalently, Normal(0, sqrt(1/tau_b1), 
    # tau_b1 is going to be very very small, so this prior became close to Normal(0, Inf)
    mu = exp(b1) # mu becomes either 0 or Inf
    y ~ dpois(mu) # error
end

this is a simplified version of https://www.multibugs.org/examples/latest/Epil.html.

This can be attributed to the different parametrization between BUGS and Distributions.jl, changing the priors would help a lot. Still, examples like this exist.

The current version in the PR will just sample from priors when no init_params given.

Default values (like 0 or undef) wouldn't work, as we do want to execute the program at least once to decide the dimensions of the model. Values like 0 just cause a lot problems.

[sunxd3]

On another thoughts, this would also be a problem generally for noninformative priors. Ideally, we would have some special treatment for dflat, normal with large variance, etc. Currently, it's probably just going to error without initializations.

[yebai]

In Turing, we actually define separate rand functions for robustly initialising the model for HMC. We could do similar tricks for uninformative distributions in BUGS.

[sunxd3]

It shouldn't be too difficult to adapt DPPL's SampleFromUniform pipeline, ref https://github.com/TuringLang/DynamicPPL.jl/blob/4cf395bd0ad182b217478a5f04b491b975f6cca5/src/utils.jl#L309-L344

[coveralls]

Pull Request Test Coverage Report for Build 9098806595

Details

• 43 of 46 (93.48%) changed or added relevant lines in 2 files are covered.
• 5 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.008%) to 82.571%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/model.jl	30	33	90.91%
<!--	Total:	43	46	93.48%	-->

Files with Coverage Reduction	New Missed Lines	%
src/JuliaBUGS.jl	5	81.71%
<!--	Total:	5		-->

Totals
Change from base Build 8936561024:	0.008%
Covered Lines:	1426
Relevant Lines:	1727

---

💛 - Coveralls