-
Notifications
You must be signed in to change notification settings - Fork 7
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
Added Banana shaped distribution example #68
Conversation
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
If this is interesting, for context https://discourse.julialang.org/t/mcmc-benchmark/63013 |
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
I like @mschauer's suggestion about the n-dimensional Rosenbrock distribution. Maybe it is a little complicated/involved for the quickstart page? I have an implementation for this if you are interested to see. Not sure if I should put it here or make a new PR? |
It's a good suggestion. I agree, it would be nice to have. RE the Quickstart, I think we only want to include it if it's not too complicated and multi-path Pathfinder more-or-less (or almost) works (we already have the funnel for which that isn't the case). Since you have an implementation, could you generate the plot of draws and the animation of fitting and drop them here in a comment? Then we can discuss whether we think they should go into the quickstart. |
Thanks! Hm, yeah these don't look terribly interesting. I'm surprised at how poor the samples look. It looks like it's initializing very close to the mode, while the scale is quite large. Could you use |
Sure!
|
Ah, okay, I see what the issue is. The marginal distribution of If we plot the contours of this density, it looks very different than the KDE, which overaggressively smooths it. It basically looks like a parabola. This is a very hard distribution to fit, and understandably, Pathfinder fails to produce draws over the whole distribution, though it does produce draws from the high-density part. Since we already have an example like this in the quickstart (failure due to high curvature/correlations), I think we should stick with the original banana function. |
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.
Just a few minor tweaks. Then I think it's good to go.
Codecov Report
@@ Coverage Diff @@
## main #68 +/- ##
=======================================
Coverage 92.89% 92.89%
=======================================
Files 13 13
Lines 521 521
=======================================
Hits 484 484
Misses 37 37 Continue to review full report at Codecov.
|
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
Co-authored-by: Seth Axen <[email protected]>
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.
accepted all the changes :)
Thanks @burtonjosh for the excellent contribution! |
as mentioned in #10, this PR adds an example of using the package on a 2D banana shaped distribution.