-
Notifications
You must be signed in to change notification settings - Fork 246
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
DOC Add Predictive examples #1084
Conversation
a9b6e90
to
65e858b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @MarcoGorelli! I think you can remove >>>
to skip doctest.
numpyro/infer/util.py
Outdated
Given a model, you can sample from the prior predictive: | ||
|
||
>>> predictive = Predictive(model, num_samples=num_samples) | ||
>>> samples = predictive(rng_key1, *model_args, **model_kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: rng_key1 -> rng_key
To make it clearer that we need to set data=None
, I think you can provide a signature for model
:
Given a model
def model(X, y=None):
...
return numpyro.sample("obs", likelihood, obs=y)
you can sample from the prior predictive:
predictive = Predictive(model, num_samples=1000)
y_pred = predictive(rng_key, X)["obs"]
If you also have posterior samples, you can sample from the posterior predictive:
predictive = Predictive(model, posterior_samples=posterior_samples)
y_pred = predictive(rng_key, X)["obs"]
numpyro/infer/svi.py
Outdated
@@ -101,6 +101,8 @@ class SVI(object): | |||
>>> svi = SVI(model, guide, optimizer, loss=Trace_ELBO()) | |||
>>> svi_result = svi.run(random.PRNGKey(0), 2000, data) | |||
>>> params = svi_result.params | |||
>>> predictive = Predictive(model, guide=guide, params=params, num_samples=1000) | |||
>>> samples = predictive(random.PRNGKey(0), data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code does not return any useful information unless you set data=None
(which will give you posterior predictive samples). If you want to get posterior samples, then you can move those codes to the end and add a comment
>>> # get posterior samples
>>> predictive = Predictive(guide, params=params, num_samples=1000)
>>> samples = predictive(random.PRNGKey(1), data)
numpyro/infer/mcmc.py
Outdated
>>> samples = mcmc.get_samples() | ||
>>> posterior_samples = mcmc.get_samples() | ||
>>> predictive = Predictive(model, posterior_samples=posterior_samples) | ||
>>> samples = predictive(rng_key1, *model_args, **model_kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have a model here. I think you can add a small code snippet to get_samples
method of MCMC
.
numpyro/infer/mcmc.py
Outdated
@@ -509,6 +511,8 @@ def run(self, rng_key, *args, extra_fields=(), init_params=None, **kwargs): | |||
""" | |||
Run the MCMC samplers and collect samples. | |||
|
|||
See :class:`~numpyro.infer.util.Predictive` for how to use them to collect posterior predictive samples. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think get_samples
is a better place for this. Also maybe changing them
to those samples
?
numpyro/infer/util.py
Outdated
|
||
Given a model, you can sample from the prior predictive: | ||
|
||
>>> predictive = Predictive(model, num_samples=num_samples) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit num_samples=1000
Thanks for your feedback! I'll address your comments at the weekend (won't have time this week before then unfortunately) |
Please take your time. We can always defer doc improvements to later releases. :) |
|
||
posterior_samples = mcmc.get_samples() | ||
predictive = Predictive(model, posterior_samples=posterior_samples) | ||
samples = predictive(rng_key1, *model_args, **model_kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need an empty string after this to make lint pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be the *
- will try making it a r
string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fixed, it just needed to be put into a code block, so with the previous line ending with ::
rather than :
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks @MarcoGorelli!
xref #1082 (comment)
I thought it might be clearer to slightly edit the existing examples in SVI and MCMCKernel, which already have runnable code, and then to refer to them in the Predictive kernel
Here's the part added to Predictive:
SVI:
MCMCKernel:
MCMC.run:
EDIT
some doctest failure...I'll fix this later, the example in
numpyro/infer/util.py
needs to just be code, but without doctest running on it