Repository navigation
Signal each class of posterior warning once - #416
florence-bockting wants to merge 4 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #416 +/- ##
==========================================
+ Coverage 92.65% 92.71% +0.06%
==========================================
Files 31 31
Lines 3240 3268 +28
==========================================
+ Hits 3002 3030 +28
Misses 238 238 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 7c3f1f6 is merged into master:
Further explanation regarding interpretation and methodology can be found in the documentation. |
jgabry
left a comment
There was a problem hiding this comment.
This looks good. I made a few small suggestions.
Also one design question: before this PR users would see something like
There were 20 warnings (use warnings() to see them)"
and now with this PR they just get a single warning with no count. For something like ESS capping that's probably good, but for something like whether 1 observation or 900 observations get a Pareto k warning it could matter. Should we add something like "(n times)" to the message? I'm not sure. Maybe @avehtari and/or @VisruthSK have thoughts on this too.
I added the count. A warning that occurs more than once now ends with (n times), for example |
|
Totally missed this, I like how that looks. |
Description
This PR is connected to stan-dev/posterior#470 where we introduced warning classes.
These can now be extracted and collected through the new
.collect_warnings()function in loo.This collector ensures that the same warning is printed only once (and not multiple times when e.g. running some posterior function in a loop).
This PR is related to Issue #353 (but it does not solve it entirely).
TODOs
The PR was assisted by an LLM.