-
Notifications
You must be signed in to change notification settings - Fork 2
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
adding PosteriorSample()
helper type.
#357
base: development
Are you sure you want to change the base?
Conversation
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.
Definitely an important addition. Added a few comments, but feel free to go ahead after you review.
pass | ||
|
||
|
||
class PosteriorSample(dist.Distribution): |
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 makes sense to me. I just want to make sure that "PosteriorSample" is the right name for this. Feels like it can also be called "PlaceholderSample" given it might have functionality that pushes the boundaries of what a Bayesian modeler might specifically consider belongs to the moniker "posterior."
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 am pretty green on all this Bayesian inference stuff so feel free to ignore if you think this is a dumb comment.
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.
yep I think @kokbent will have insight on this
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 do like PlaceholderSample
a little better since it doesn't need to be substituted by a posterior sample, e.g., initial state, or some other number that we substitute in run time for whatever reason... PosteriorSample
is kind of like a subset of PlaceholderSample
that might help readability, so I'm a bit torn 🤔
Often it is the case that modelers will want to run two separate models, one to fit to the data, and than another to project forward in time or run some scenarios. In these cases they need a way within their configuration files to mark that a particular parameter will depend on a set of posterior samples, usually from a previous run.
The
PosteriorSample
class offers an easy way for users to mark that a particular parameter's values will be deterministically picked from a set of posterior samples, while also quickly erroring if a user attempts to treat it as if it is a distribution they may randomly sample from.Here is a basic example of the functionality:
in the backend the
PosteriorSample
class is treated just like anumpyro.distributions.Distribution
object, however, when attempting to sample it outside of aPredictive()
ornumpyro.handlers.substitute
context, the error will be raised immediately.In this example we use a site name
x
to identify the sampled parameter, however as long as we programmatically pick site names based on the parameter name, and users do not modify theirdata
dictionary to mess with site names, this should properly match all parameters.FYSA @kokbent @tjhladish @cdc-ap66 feel free to start a discussion below.
CLOSES #355