Add Bayes Factor plot via Savage-Dickey density ratio #2037
Conversation
orduek
changed the title
|
Is it of interest to the audience? |
|
Hi, sorry for the very slow response. It looks like it is something that is probably useful to ArviZ users, but I had never heard about this plot before the issues was opened and haven't found the time to review the references (I am also quite slow to review PRs about things I do know about). I can leave some general comments for now and questions. Thanks for your patience. |
|
OK, I've fixed the issues you've raised.
|
|
ok, I added logging. Let me know if you think it's ok now. |
|
Ok, I went back to the issue conversation with @drbenvincent , the main concern was around bounded distributions (not categorical), but I'm not sure we'll have an easy solution to this one. We need some more expertise here. |
|
I don't think we should have a family argument. Instead, we should be able to handle all kinds of distributions without user intervention. I see two simple solutions
|
|
I lean toward starting with the current option. Later on, I think it's worth testing & develop the second option, maybe then we'll be able to allow a "method" input to the function. What do you think? |
|
yes we can leave it like this for this PR, and in the future think of a better method, not sure we will need to support more than one method. |
|
ok, great. |
arviz/plots/bf_plot.py
Outdated
| print("Posterior distribution has {post.ndim} dimensions") | ||
| # generate vector | ||
| if xlim is None: | ||
| x = np.linspace(np.min(prior), np.max(prior),prior.shape[0]) |
There was a problem hiding this comment.
probably better to just have a "high" number of points, like 5000, 10000?
There was a problem hiding this comment.
It will probably be very similar, but why not use the prior dimensions? to save resources?
There was a problem hiding this comment.
We are trying to fill the plot evenly with points and this method either needs many points or some density based sampling.
Common images are happy with 1000+ points
There could be problems with 100k points or if prior point count is low and concentrated in one location with huge tails
|
ok, I've kept two things open:
|
|
We already have some logic to handle variables names see for example https://github.com/arviz-devs/arviz/blob/main/arviz/plots/autocorrplot.py#L120. Something like 5000 points for the kde should be enough. |
|
I've changed the size to 5000 and tried to incorporate the var_names method. |
|
After those changes I think this is ready to merge. |
Co-authored-by: Osvaldo A Martin <[email protected]>
Co-authored-by: Osvaldo A Martin <[email protected]>
|
Ok, I've added some more words (what the function returns). |
aloctavodia
changed the title
|
For now, in order to use the extract function, I am using |
|
yes, try with a relative import. |
|
Done. Thanks! |
|
thanks @orduek! |
Add Bayes Factor plot via Savage-Dickey density ratio (arviz-devs#2037)
https://build.opensuse.org/request/show/1073712 by user dgarcia + dimstar_suse - skip python 3.11 and python 3.8 because some dependencies doesn't support that python versions - Update to 0.15.1 - Fix memory usage and improve efficiency in `from_emcee` ([2215](arviz-devs/arviz#2215)) - Lower pandas version needed ([2217](arviz-devs/arviz#2217)) - 0.15.0 - Adds Savage-Dickey density ratio plot for Bayes factor approximation. ([2037](arviz-devs/arviz#2037), [2152](arviz-devs/arviz#2152)) - Add `CmdStanPySamplingWrapper` and `PyMCSamplingWrapper` classes ([2158](arviz-devs/arviz#2158)) - Changed dependency on netcdf4-python to h5netcdf ([2122](arviz-devs/arviz#2122)) - Fix `reloo` outdate
Description
As requested in issue #689 - this is an attempt to add plot_bf function, that will take prior, idata (trace) and reference value, and output a Bayes Factor + plot (please see issue #689 for details and examples)