-
Notifications
You must be signed in to change notification settings - Fork 54
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
Consider whether exoplanet should use DensityDist rather than Potential #70
Comments
I think this would be cool, but it would probably be better to separate |
Ok. I will try to work on it. |
@mrtommyb: I implemented this on the flight to AAS. It's on the The syntax is now something like: with pm.Model():
kernel = xo.gp.terms....
gp = xo.gp.GP(kernel, x, yerr**2, ...)
gp.marginal("likelihood", observed=y) |
Hi @dfm, I'm working on a model which uses pm.Potential('spitzer_depth_potential', pm.Normal.dist(0, 300).logp(spitzer_depth)) where |
@bmorris3: There shouldn't be any issue with that syntax, but a simpler one might be: pm.Normal("spitzer_depth_prior", mu=spitzer_depth, sigma=300, observed=0.0) Which should work equivalently and I find somewhat more readable. (You can think of this "prior" as, instead, "data" from a previous experiment.) |
Thanks for this, that's definitely more readable! |
PyMC3 recommended way to include a custom likelihood function is with pm.DensityDist, e.g.
However in the exoplanet examples, the the GP likelihood is implemented as pm.Potential. The results are the same but it might be sensible to implement the samples using the standard PyMC3 way of doing it. Advantages include clear separating out of the observables. Additionally, some of the nice plotting functions like model_to_graphviz ignore potentials.
Here's an example of what it would look like
The text was updated successfully, but these errors were encountered: