-
Notifications
You must be signed in to change notification settings - Fork 228
Set minimum of OrderedLogistic distribution to 1 #2603
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
Conversation
The minimum of the OrderedLogistic function is supposed to be 1, not 0. This is consistent with the documentation, and the behavior of the distribution when using rand().
That's great, thanks! Could I trouble you to add a test to |
I can try later today! |
We add the test that the support should have the right number of elements, and that the probabilities add up to 1.0.
Added a test now. |
Wonderful, thank you for the great work @Noricc! I'll just bump the patch so that we can release, then will merge once CI is happy. |
863acef
to
2a2bd6b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2603 +/- ##
=======================================
Coverage 85.57% 85.57%
=======================================
Files 22 22
Lines 1456 1456
=======================================
Hits 1246 1246
Misses 210 210 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 15899880826Details
💛 - Coveralls |
The minimum of the OrderedLogistic distribution is supposed to be 1, not 0. This is consistent with the documentation, and the behavior of the distribution when using rand().
Fixes bug #2602