Update mcse_sd calculation to not use normality assumption. #2167
Conversation
|
It looks like tests are running against posterior 0.1.2, can this be updated to latest posterior? |
arviz/stats/diagnostics.py
Outdated
| @@ -732,8 +732,8 @@ def _ess_sd(ary, relative=False): | |||
| ary = np.asarray(ary) | |||
| if _not_valid(ary, shape_kwargs=dict(min_draws=4, min_chains=1)): | |||
| return np.nan | |||
| ary = _split_chains(ary) | |||
| return min(_ess(ary, relative=relative), _ess(ary**2, relative=relative)) | |||
| ary = abs(ary - ary.mean()) | |||
There was a problem hiding this comment.
Based on benchmarks, posterior just switched to this form (see stan-dev/posterior#266):
| ary = abs(ary - ary.mean()) | |
| ary = (ary - ary.mean())**2 |
There was a problem hiding this comment.
Thanks, I will add this. Need to go through loo to make sure we have the latest changes
There was a problem hiding this comment.
By fixing the tests I realized that change should have been done to mcse_sd not to ess_sd. With the changes I just pushed we are back to 100% match with posterior
There was a problem hiding this comment.
Based on the slack discussion, posterior should also be using the change in ess_sd (see stan-dev/posterior#388), so I suggest we revert that change.
OriolAbril
force-pushed
the
feature/updated_mcsd
branch
from
ff971b8 to
2a286cd
Compare
OriolAbril
dismissed stale reviews from sethaxen and aloctavodia
Review no longer applies
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2167 +/- ##
=======================================
Coverage 86.72% 86.72%
=======================================
Files 124 124
Lines 12908 12914 +6
=======================================
+ Hits 11194 11200 +6
Misses 1714 1714 ☔ View full report in Codecov by Sentry. |
Fixes #2005
See https://github.com/stan-dev/posterior/pull/233/files
Description
Checklist