Added icdf for Rice and skewnormal distributions #7095
Conversation
|
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7095 +/- ##
==========================================
- Coverage 92.21% 92.12% -0.10%
==========================================
Files 101 101
Lines 16912 16934 +22
==========================================
+ Hits 15595 15600 +5
- Misses 1317 1334 +17
|
|
Hi The icdf must be written with PyTensor operations and tests are needed. Have a look at previous PRs that introduced icdf methods. |
There was a problem hiding this comment.
Thanks for your PR @ParamThakkar123. As @ricardoV94, can you provide corresponding tests for your icdf functions? See PR #6747 for a nice reference.
Also, I am not sure in the math of your icdf functions... Can you provide some justification? Passing tests would help a lot here, but some arguments as to what is happening would be beneficial
| @@ -2993,6 +2996,29 @@ def logp(value, mu, sigma, alpha): | |||
| tau > 0, | |||
| msg="tau > 0", | |||
| ) | |||
| def icdf(prob, mu, sigma, alpha, max_iter=100, tol=1e-8): | |||
There was a problem hiding this comment.
What justifies the icdf method having a different signature compared to the logp method mentioned above?
| x -= derivative | ||
|
|
||
| res = mu + sigma * x | ||
| res = check_icdf_value(res, prob) |
There was a problem hiding this comment.
Can you explain in words (or math) what is going on here?
There was a problem hiding this comment.
The provided code defines a function icdf that aims to find the inverse cumulative distribution function (ICDF) for a specific probability distribution.
cdf Function:
Calculates the cumulative distribution function (CDF) for a given input
x using a specific probability distribution formula.
The distribution appears to be a modified version, involving terms like pt.exp (likely indicating exponentiation), iv (modified Bessel function of the first kind), and ive (modified Bessel function of the second kind).
cdf_derivative Function:
Calculates the derivative of the CDF with respect to
x. This derivative is used in the Newton-Raphson method, which is an iterative numerical technique for finding roots (or zeros) of a function.
Newton-Raphson Iteration (newton):
Uses the Newton-Raphson method to iteratively find a value of x such that
cdf(x)−prob=0.
The fprime argument is provided with the derivative function (cdf_derivative) to guide the iteration.
check_icdf_value Function:
Performs some checks on the calculated ICDF value (res) and potentially modifies it.
Checks whether certain parameters, in this case,
σ, satisfy a condition (in this case, >0 σ>0).
If the condition is not met, it likely raises an error or handles the situation accordingly.
Overall Workflow:
The main function icdf combines these components to find the ICDF for a given probability (prob) using the Newton-Raphson method.
It includes checks to ensure the validity of the ICDF value and the parameters involved.
There was a problem hiding this comment.
Sorry to ask but are you a bot?
The code will clearly not work with PyTensor variables.
There was a problem hiding this comment.
I'll fix everything and make a new pull request
There was a problem hiding this comment.
Calculates the cumulative distribution function (CDF) for a given input x using a specific probability distribution formula. The distribution appears to be a modified version, involving terms like pt.exp (likely indicating exponentiation), iv (modified Bessel function of the first kind), and ive (modified Bessel function of the second kind). cdf_derivative Function:
Performs some checks on the calculated ICDF value (res) and potentially modifies it.
What do you mean by "likely indicating", "potentially modifies it"? This wording seems strange to me.
Again, none of this will work with PyTensor variables. If you are relying on AI to generate responses and code changes, we cannot accept them
| return ((1 - x**2 / sigma**2) * pt.exp(-x**2 / (2 * sigma**2)) * iv(0, x * nu / sigma**2) | ||
| + (x / sigma**2) * pt.exp(-x**2 / (2 * sigma**2)) * ive(0, x * nu / sigma**2)) * nu / sigma**2 | ||
|
|
||
| approx_icdf = newton(cdf, x0, fprime=cdf_derivative, tol=tol, maxiter=max_iter) |
There was a problem hiding this comment.
Will newton from scipy work with PyTensor variables?
| if pt.abs(diff) < tol: | ||
| return mu + sigma * x | ||
|
|
||
| derivative = norm.pdf(x) - 2 * alpha * norm.pdf(alpha * x) |
There was a problem hiding this comment.
does norm.pdf and norm.ppf work with PyTensor objects?
Description
Added icdf for Rice and SkewNormal distributions
Related Issue
Checklist
Type of change
📚 Documentation preview 📚: https://pymc--7095.org.readthedocs.build/en/7095/