Add transition_theta to outer inference code #71
Conversation
| stddev2 = std::get<3>(hypers[i]); | ||
| } | ||
|
|
||
| void Skellam::init_theta(std::mt19937* prng) { |
There was a problem hiding this comment.
Out of curiosity, is there a need for Skellam to be initialized randomly vs. statically like the other distributions? Or do we see other distributions initializing their parameters from their hyperprior? Mainly asking since it seems a little odd that Skellam's path is different from other distributions.
There was a problem hiding this comment.
Yeah, all the NonconjugateDistributions will need random initializations. The reason is that the NonconjugateDistributions can't efficiently marginalize over the latent parameters, so instead they store current values of their latents, which evolve whenever transition_theta is called. And I believe that that overall learning procedure works better when the latents are randomly initialized, but I guess I could be wrong about that. I personally always use random initialization when doing Metropolis-Hastings, but I guess some people always use the origin or the mean of the sampling distribution or something like that.
Anyway, we could definitely raise this issue on the slack channel if you like, but the above is my current understanding. Oh, and I guess another thing is that the GenDB doc says that for "distributions that explicitly represent their latents", "the code to initialize the model state will need to include code for sampling initial parameters from the parameter prior". So this is that.
Also, add Skellam as an HIRM distribution.
Also (and this is actually the majority of this change), call init_theta when creating a non-conjugate distribution. init_theta needs a random number generator, so a bunch of relation and irm methods now need to take one of those when they didn't before.