Skip to content
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

Clarify definitions of inverse CDF functions #1814

Merged
merged 3 commits into from
Jan 3, 2024

Conversation

sethaxen
Copy link
Contributor

@sethaxen sethaxen commented Jan 1, 2024

Fixes #1813 by updating docstrings as proposed in #1813 (comment):

  • documents in what sense quantile and friends invert cdf and friends
  • documents that inputs to quantile and friends are probabilities in [0, 1] or log-probabilities in (-Inf, 0]. (inputs outside of this range would then lead to undefined behavior)

@codecov-commenter
Copy link

codecov-commenter commented Jan 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b2d7652) 85.98% compared to head (25c8c07) 85.93%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1814      +/-   ##
==========================================
- Coverage   85.98%   85.93%   -0.05%     
==========================================
  Files         144      144              
  Lines        8648     8655       +7     
==========================================
+ Hits         7436     7438       +2     
- Misses       1212     1217       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Evaluate the inverse cumulative distribution function at `q`.
Evaluate the generalized inverse cumulative distribution function at `q`.

For a given `0 ≤ q ≤ 1`, `quantile(d, q)` is the smallest value `x` in the support of `d`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I think it good to include the accurate definition, I feel like it would be helpful to state the simplified version before stating the precise but also more complicated definition. For most people, in most situations, the simple but slightly inaccurate definitions is probably more helpful. For the same reason, maybe put "generalized" in parentheses as many people might not know what it means. Same comment applies to the other changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@devmotion
Copy link
Member

Related: #1805

@devmotion devmotion merged commit 9e72f1f into JuliaStats:master Jan 3, 2024
10 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inv* functions are not really inverses
4 participants