Skip to content
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

use improved Rhat to implement ESS-bulk and ESS-tail #3299

Open
mitzimorris opened this issue Jul 12, 2024 · 10 comments
Open

use improved Rhat to implement ESS-bulk and ESS-tail #3299

mitzimorris opened this issue Jul 12, 2024 · 10 comments
Labels

Comments

@mitzimorris
Copy link
Member

Summary:

PR #3266 implemented the rank-normalization and folding needed for Rhat-bulk and Rhat-tail.
We should also add logic for ESS-bulk and ESS-tail, per Vehtari et al https://arxiv.org/abs/1903.08008.

Description:

Bulk ESS and tail ESS are available in R, but not in CmdStan or CmdStanPy. We need to add this to Stan so that all interfaces are providing the same estimates given a sample.

Bulk ESS can be computed from the rank normalized draws, per section 4.1

We will use the term bulk effective sample size (bulk-ESS or bulk-Seff ) to refer to the effective sample size based on the rank normalized draws.

Tail ESS is defined in section 4.3

To get a better sense of the sampling efficiency in the distributions’ tails, we propose to compute the minimum of the effective sample sizes of the 5% and 95% quantiles, which we will call tail effective sample size (tail-ESS or tail-Seff ).

Current Version:

v2.35.0

@mitzimorris
Copy link
Member Author

@avehtari @jgabry do we also want to implement the MCSE described in section 4.4 ? has this already been done in R? which package and where?

@mitzimorris
Copy link
Member Author

also pings to @aleksgorica and @SteveBronder.

plugging this in to CmdStan's stansummary is trivial, cf: stan-dev/cmdstan@develop...feature/1263-new-rhat-summary

@jgabry
Copy link
Member

jgabry commented Jul 14, 2024

@avehtari @jgabry do we also want to implement the MCSE described in section 4.4 ? has this already been done in R? which package and where?

Yeah bulk and tail ESS are included in the standard diagnostics computed by the posterior package:

https://github.com/stan-dev/posterior/blob/master/R/convergence.R

@avehtari
Copy link
Collaborator

@avehtari @jgabry do we also want to implement the MCSE described in section 4.4 ? has this already been done in R? which package and where?

Yeah bulk and tail ESS are included in the standard diagnostics computed by the posterior package:

And the implementation for the MCSE for quantiles (Section 4.4 in the paper) is in .mcse_quantile function
https://github.com/stan-dev/posterior/blob/18c915e540e7578bb69830f3d544e1cfcae26b72/R/convergence.R#L418

@mitzimorris
Copy link
Member Author

mitzimorris commented Jul 16, 2024

is current ESS computation using autocorrelation? comment from @syclik, added 12 years ago remains:

// FIXME: reimplement using autocorrelation.

the answer is yes it is. comment should be removed.

@jgabry
Copy link
Member

jgabry commented Jul 16, 2024 via email

@avehtari
Copy link
Collaborator

I agree that that FIXME comment should be removed

@mitzimorris
Copy link
Member Author

mitzimorris commented Jul 24, 2024

since we don't know how much of the API in chains.hpp is being used elsewhere, I think that we should re-implement it as a new class chainset.hpp, which takes as its underlying data structure a std::vector<Eigen MatrixXd> to hold the chains, where all chains are the same size and shape, with matching column names, and do not contain any warmup draws. enforcing these constraints at object instantiation will allow us to get rid of a lot of repeated code across the various functions which do these checks. at some future point, we could deprecate chains.hpp.

further refactoring of the code in stan/analyze/mcmc: the rank-normalized split rhat added to compute_potential_scale_reduction.hpp should be split out into a separate files for rank-normalization, bulk and tail rhat, and bulk and tail ess.

@WardBrian, @SteveBronder thoughts?

@WardBrian
Copy link
Member

I'm still not sure we need something like chains.hpp at all. If we had analysis functions that took as an argument std::vector<Eigen::MatrixXd> chains, I suspect that is all we really need. Adding this object whose only job is to call the other functions seems like more code for little benefit, unless we use the object abstraction in some other way as well that I've missed

@mitzimorris
Copy link
Member Author

mitzimorris commented Jul 24, 2024

I'm still not sure we need something like chains.hpp at all. If we had analysis functions that took as an argument std::vector<Eigen::MatrixXd> chains, I suspect that is all we really need.

generally agree. the added functionality needed is consistency checking across chains, indexing into this object by column name, and ensuring that variance calculations are done on columns of finite, non-identical values. but yes, the analysis functions should take as arguments std::vector<Eigen::MatrixXd> chains plus column index/name and return the computed statistic/diagnostic for that column across all chains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants