-
-
Notifications
You must be signed in to change notification settings - Fork 417
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
change kde limits and hdi computation #1215
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1215 +/- ##
==========================================
- Coverage 93.14% 93.07% -0.07%
==========================================
Files 94 94
Lines 9497 9504 +7
==========================================
Hits 8846 8846
- Misses 651 658 +7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about not setting the range between 0 and 1.
Here is an example of bad behaviour I think this would introduce: imagine we have 1e6 observations whose 1e6 loo-pit values are uniformally distributed between .3 and .7. Old behaviour would set the kde limits to 0,1 and the resulting kde would have an inverted U shape whereas with this change the kde would be perfectly flat and it would be the fact that the range is not from 0 to 1 what should warn the user of the extremely bad loo pit result they actually have.
Is this ready to merge? |
Yes. |
This makes two important changes. compute the kde over the data range (not over 0, 1). This help to spots problems, as the KDE is "not adding data". Compare our example in the gallery, with the new version.
.
This also introduce a theoretical HDI, instead of estimating the HDI from the samples.
maybe we should enforce the zero y-bottom-limit.