TuringLang / MCMCChains.jl

Types and utility functions for summarizing Markov chain Monte Carlo simulations
https://turinglang.org/MCMCChains.jl/
Other
266 stars 29 forks source link

Add more methods for basic summary stats for Chains #421

Open ElOceanografo opened 1 year ago

ElOceanografo commented 1 year ago

I was recently surprised to find that calling std() on a Chains threw an error. Looking into it, it appears that only a few individual summary statistics (mean, cor, and quantile) are defined for Chains objects. Is there any reason not to implement methods for others, like cov, var, and std? If not I'm willing to make a PR.

cpfiffer commented 1 year ago

I don't think there's a good reason why they're not defined, I'd love a PR!

On Fri, May 19, 2023, at 10:34 AM, Sam wrote:

I was recently surprised to find that calling std() on a Chains threw an error. Looking into it, it appears that only a few individual summary statistics (mean, cor, and quantile) are defined for Chains objects. Is there any reason not to implement methods for others, like cov, var, and std? If not I'm willing to make a PR.

— Reply to this email directly, view it on GitHub https://github.com/TuringLang/MCMCChains.jl/issues/421, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADHITUKFCDWVAHQNIVGPGLXG6VLXANCNFSM6AAAAAAYIBVNIU. You are receiving this because you are subscribed to this thread.Message ID: @.***>

ElOceanografo commented 1 year ago

I've started working on a PR and was wondering about what the return type for cov(chns) should be. A 2D AxisArray would seem to be a good choice. However, that's inconsistent with the ChainDataFrame returned by mean(chns). Is there a particular reason why mean returns a (somewhat) complicated structure like that, rather than a plain vector or categorical AxisArray?

cpfiffer commented 1 year ago

I think the reason we defined it what way was to override a bunch of pretty-printing options. In the case of cov, I agree that it's maybe a little weird to have a ChainDataFrame since it's a square matrix. I'd be okay this time with it returning an AxisArray, but we should at some point do something a little more thoughtful about the ChainDataFrame type. That was code I added in. While it is pretty, it is a little bit of a pain in the ass to work with.