posterior_predictive MCMC samplers don't check for proper "r" generation function

[danielturek]

Neither the posterior_predictive nor the posterior_predictive_branch samplers check for the existence of functional "r" random generation functions. This causes a problem in the case of user-defined distributions, when the "r" function is not provided, and thus we auto-generate a placeholder "r" function, which itself generates an error if called. When nodes following such a distribution are assigned either form of the posterior_predictive MCMC sampler, the posterior_predictive sampler will call the "r" function thus resulting in an error.

Bad news, this is an instance of our default MCMC sampler assignment producing an error at MCMC runtime. Good news, this produces a hard-stop error, and there's no chance of any "silent failure".

@perrydv @paciorek I'm not sure the best approach for fixing this. I don't have many great ideas:

• Augment the registerDistributions function (and data structure associated with distributions) with an additional (boolean-valued) "rFunctionExists" field, or similar. For all of our distributions, this is TRUE. When a user writes a distribution, this is FALSE is we auto-generate the "r" placeholder. Then, logic would be added to the assignment of the posterior_predictive samplers to check for that this attribute is TRUE.
• (I don't like this solution) Code could be added to the posterior_predictive sampler, to inspect the necessary "r" function(s) and determine if they were auto-generated.
• Other ideas.... ?

[paciorek]

@danielturek Hmm, is a run-time error so bad given that the error message is quite clear: Error: user-defined distribution dZIP provided without random generation function.

Perhaps there is some elegance in detecting this at configuration or build time, but given other priorities, my initial reaction is that maybe we don't do anything.

Per another discussion, I am planning to add a note about user-defined 'r' being needed for initialization and posterior_predictive sampling in the manual and the user-defined distribution example.

Curious what @perrydv thinks.

[danielturek]

Yes, this is a legitimate concern @paciorek . Maybe it would be best if we inserted some checking into the R code which does sampler assignments. If a posterior_predictive_branch sampler is under consideration, then before assignment the necessary r functions are checked for (in the enclosing environment? in the global environment? exactly how to handle this would be a question for me). The sampler is then only assigned if all necessary r functions are found.

Does that sound reasonable?

[paciorek]

@danielturek I don't feel strongly. If you think we should do this and want to do it, that's fine with me. Given other release work I'm doing, I probably wouldn't do it myself for the upcoming release.

[danielturek]

@paciorek Agreed and understood. I'll put it on my "get to it when I can" list, which won't be for this upcoming release.

[paciorek]

@danielturek given post. pred. revamp, could you check if this can be closed?

[danielturek]

@paciorek This case is still not handled by the posterior_predictive sampler, but I'm ok with that for now. It feels like a fairly rare case to me.

If we agree this is something to prioritize, then I'm happy to address it, but I'd prefer if that wasn't for this upcoming release.

[paciorek]

Fine by me.