Analysis API: compute effective sample size

[roualdes]

Summary:

Offer interfaces a unified calculation of effective sample size.

Description:

This issue is solely for the calculation of ess for a single parameter across multiple chains.

Many details were discussed on the Discourse: Analysis API thread. Design details:

• route goes in stan-dev/stan:src/stan/analyze/mcmc/compute_ess.hpp
• signature is compute_effective_sample_size(std::vector<double*> draws, std::vector<size_t> sizes) where each chain's draws are stored in a contiguous block of memory
• convenience function compute_effective_sample_size(std::vector<double*> draws, size_t size)
• calculation and I/O handled separately. Only calculation, here.
• minimize copying
• stan-dev/stan is the appropriate place for ess calculation (and other diagnostics) as it is more closely related to sampling algorithms than the stan-dev/math library
• start with ess, integrate it all the way through, then build out other diagnostics
• long term goal to remove ess from chains class as OO structure is not necessary

Current Version:

v2.17.1

[roualdes]

Looking at effective_sample_size within the chains class, will n_kept_samples ever be different from n_samples? If so, do we need to accept n_kept_samples as an argument to compute_effective_sample_size?

[bob-carpenter]

Not sure if this answers your question, but the number of draws before thinning isn't relevant for n_eff calculations. We should be using "draws" not "samples" in the code if you want to fix that while you're at it---we'd like to use "sample" to mean a sequence of draws.

[riddell-stan]

It seems like `draws` is the preferred label. Do we want to change the parameter num_samples (CmdStan) to num_draws? Or at least add a note in the documentation that "num_samples" uses deprecated terminology?

…

On 07/06/2018 12:05 AM, Bob Carpenter wrote: Not sure if this answers your question, but the number of draws before thinning isn't relevant for n_eff calculations. We should be using "draws" not "samples" in the code if you want to fix that while you're at it---we'd like to use "sample" to mean a sequence of draws. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2569 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AmFA1oPCkVQlqcF4O4oJ6M7aLnrbxVznks5uDuIdgaJpZM4VEgDy>.

[roualdes]

I believe the answer to my question is, no, n_kept_samples and n_samples will never be different. My best efforts indicate that develop:effective_sample_size is only ever given kept draws, as @bob-carpenter suggested.

PR #2575 is now posted. I took some liberties on the code, and tried to justify them in the post.

Apologies for jumping this conversation across three different pages: Discourse, this issue, and the above PR.

[kedartal]

I hope this is the right place for this issue, apologies in advance if not.
I get a "duplicate symbols" linker error for the functions in .../stan/analyze/mcmc/compute_effective_sample_size.hpp when compiling against Stan (commit 11b1c9c); adding "inline" to the signatures solves the issue. Would it make sense to solve it this way, or alternatively, move the code into a cpp file?

[bob-carpenter]

@kedartal: This is not the right place. This is a specific issue. I don't want to hijack this issue with a discussion of inline.

If you have a reproducible example of a bug, you can create a new issue. Otherwise, we have a forum for general discussion if you have a question:

http://discourse.mc-stan.org

[roualdes]

Can this be closed now?

[avehtari]

It seems this should be closed because of PR #2575