API proposal: Add sample_all()

[twiecki]

All the good Bayesians who are following the proper Bayesian workflow are writing things like:

with model:
    idata = pm.sample()
    idata.extend(pm.sample_prior_predictive())
    idata.extend(pm.sample_posterior_predictive(idata))

these days. Besides this syntax not being that pretty because of the extend, it's also verbose. So how about instead we have a pm.sample_all() that returns an InferenceData with posterior, prior, prior predictive, and posterior predictive, all in one command?

I think that would also help enforce best practices and make it much simpler, especially for beginners to do The Right Thing.

[ricardoV94]

What is the point of sample_prior? Is it the same as prior_predictive without draws from the likelihood?

[twiecki]

@ricardoV94 Right, I don't think we need it here (removed it above).

[canyon289]

Im in favor of this, would be really nice syntactic sugar. It also will make downstream arviz usage eaiser for beginners since itll reduce the number of "you dont have prior/posterior predictive samples" which are needed for many plots and diagnostics

[michaelosthege]

I like it. Would make some backend things easier too.
But I'd prefer to have kwargs so one can choose to do only some of the steps. sample_all could still be added as a short-hand notation for pm.sample(prior_predictive=True, posterior_predictive=True).

Related proposal: Setting discard_tuned_samples=False by default? They are saved in separate idata groups already and it would help understanding, diagnosis and performance tweaking.

[fonnesbeck]

I'm not sure how much you would want to automate all of this. The Good Bayesian would want to look at the prior predictive output before they go ahead and fit the model, right? There is something to be said for deliberately conducting the workflow yourself, and adjusting as necessary along the way.

[ericmjl]

Crazy idea: what if we added the function, but kept its presence "for the experts' in the house" only?

I'm maybe a quarter serious about this idea.

There's value for expert practitioners who are familiar with their domain to automate routine things. I can see the reasons why @twiecki would want this automation.

At the same time, I also see @fonnesbeck's point about deliberately conducting the workflow.

I see no harm in it being present as a convenient shortcut that we don't have to write on our own. I would maybe only caution against using the shortcut in our examples, that's all.

[twiecki]

I think our prior and posterior predictive sampling is very fast now. What if we just, by default, when calling pm.sample() we also run predictive samples and return them? There would be an option to switch them off. In that case, if you follow the Good Bayesian workflow you still explicitly call sample_prior_predictive() but if you actually draw posterior samples, you always have a fully populated InferenceData object.

[tomicapretto]

I think this functionality can be useful in some controlled situations -- like a lesson, where independent of the quality of the prior predictive checks, you still want to fit the model. But I agree with @fonnesbeck that one should always encourage that in more realistic scenarios you first do prior predictive checks, and only after that is OK, you fit the model.

In summary, not against having it but would prefer not to advertise it as the new tool that simplifies bayesian workflow because it would be actually spreading a bad practice.

[ricardoV94]

I don't think we should have this at all. Just write the function yourself, there's nothing wrong with that. Why should PyMC have to offer (and test and document) this?

[twiecki]

Because PyMC is all about UX.

[ricardoV94]

This is bad UX IMO. Hiding three distinct methods under the hood to save a few keystrokes?

What about non-default parameters? Are we going to never allow them, or start trying to parse what belongs to what? (tune, draws, target_accept, var_names, random_seed, ...)

If users really needed this, I would expect we would be seeing it in shared gists/ discourse issues. If it's for teaching only, why can't it be offered as a separate package by whomever wants to use it in teaching?

[twiecki]

I wouldn't allow kwargs to be forwarded, then you need to call them individually.
We use

with model:
    idata = pm.sample()
    idata.extend(pm.sample_prior_predictive())
    idata.extend(pm.sample_posterior_predictive(idata))

all the time, but it's a bit cumbersome to have to type. But it's a valid question of whether others are using this pattern too. But just because they aren't doesn't mean they wouldn't like it if it existed. A fully populated idata with a single line sounds neat to me. And even if it's just for teaching, what's wrong with a little helper function that's literally 3 lines and has the simplest doc-string ever.