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

DOC/ENH: Explain limitations of stats.levy_stable more explicitly #8289

Merged
merged 5 commits into from
Feb 20, 2022

Conversation

jcalbert
Copy link
Contributor

In response to #8263 - makes the documentation of stats.levy_stable more descriptive.

@rgommers rgommers added scipy.stats enhancement A new feature or improvement labels Jan 13, 2018
@ev-br
Copy link
Member

ev-br commented Jan 14, 2018

Cross-ref #7374

Copy link
Member

@chrisb83 chrisb83 left a comment

Choose a reason for hiding this comment

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

Hi,
having these comments is definitely an improvement. I added a few remarks...


The distribution for `levy_stable` is parametrized by
:math:`0 < a \leq 2` and :math:`-1 \leq b \leq 1`. Its probability
density function cannot be expressed analytically, but is described by
Copy link
Member

Choose a reason for hiding this comment

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

"function cannot be expressed analytically" --> "function cannot be expressed analytically (except in some special cases)"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree

scipy/stats/_continuous_distns.py Outdated Show resolved Hide resolved
scipy/stats/_continuous_distns.py Outdated Show resolved Hide resolved
@chrisb83
Copy link
Member

#7374 hast been merged in the meantime. The table with the special cases would still be a useful addition. @jcalbert do you want to rebase and update your changes?

@jcalbert
Copy link
Contributor Author

Sure, I'll do that early next week. About #7374 - is the new _pdf() sufficient for all purposes, or is it still preferable to use other distributions (e.g. cauchy for alpha=1, beta=0) when possible?

@chrisb83
Copy link
Member

Thanks for the update. I don't know the implementation of 7374, my guess is that the special cases use simpler methods.

@jcalbert
Copy link
Contributor Author

OK, I think it's good but my rebase skills are shaky so we'll see.

@chrisb83
Copy link
Member

Once the rebase is successful in your local repo, you need to force the push. https://stackoverflow.com/questions/5509543/how-do-i-properly-force-a-git-push

@jcalbert
Copy link
Contributor Author

OK I'm pretty sure I've got it now.

scipy/stats/_continuous_distns.py Outdated Show resolved Hide resolved
scipy/stats/_continuous_distns.py Outdated Show resolved Hide resolved
Joseph Albert and others added 2 commits August 20, 2018 13:57
ENH: On failure, suggest using an equivalent distribution if possible.

Minor PEP8 fix.

Corrections suggested by @chrisb83

I think I'm undoing something?
@jcalbert
Copy link
Contributor Author

jcalbert commented May 4, 2019

It's been a while but I think this still applies.

@mdhaber
Copy link
Contributor

mdhaber commented Feb 20, 2022

@steppi @bsdz @ragibson Ran across this and thought you might have an opinion. Shall I move this over to stats/_levy_stable/__init__.py? Where in the notes would it fit?

[skip azp] [skip actions]
@jcalbert jcalbert requested a review from ilayn as a code owner February 20, 2022 06:10
@mdhaber mdhaber changed the base branch from main to maintenance/0.5.2.x February 20, 2022 06:12
@mdhaber mdhaber changed the base branch from maintenance/0.5.2.x to main February 20, 2022 06:12
@mdhaber
Copy link
Contributor

mdhaber commented Feb 20, 2022

Miraculously, Sphinx rendered the table exactly as I would have hoped.
image

Here's a direct link to the artifact.
https://43418-1460385-gh.circle-artifacts.com/0/html-scipyorg/reference/generated/scipy.stats.levy_stable.html#scipy.stats.levy_stable

@bsdz
Copy link
Contributor

bsdz commented Feb 20, 2022

Miraculously, Sphinx rendered the table exactly as I would have hoped. image

Here's a direct link to the artifact. https://43418-1460385-gh.circle-artifacts.com/0/html-scipyorg/reference/generated/scipy.stats.levy_stable.html#scipy.stats.levy_stable

We do specialize for most of those cases. E.g.

if alpha == 2.0:
# normal
return _norm_pdf(x0 / np.sqrt(2)) / np.sqrt(2)
elif alpha == 0.5 and beta == 1.0:
# levy
# since S(1/2, 1, gamma, delta; <x>) ==
# S(1/2, 1, gamma, gamma + delta; <x0>).
_x = x0 + 1
if _x <= 0:
return 0
return 1 / np.sqrt(2 * np.pi * _x) / _x * np.exp(-1 / (2 * _x))
elif alpha == 0.5 and beta == 0.0 and x0 != 0:
# analytical solution [HO]
S, C = sc.fresnel([1 / np.sqrt(2 * np.pi * np.abs(x0))])
arg = 1 / (4 * np.abs(x0))
return (
np.sin(arg) * (0.5 - S[0]) + np.cos(arg) * (0.5 - C[0])
) / np.sqrt(2 * np.pi * np.abs(x0) ** 3)
elif alpha == 1.0 and beta == 0.0:
# cauchy
return 1 / (1 + x0 ** 2) / np.pi

That said, I feel it wise to recommend the equivalent distributions where possible. There might optimisations, better consideration of numerics etc. That table makes it clear.

@mdhaber
Copy link
Contributor

mdhaber commented Feb 20, 2022

Thanks. If that looks like a good place for it, I'll merge.

@ragibson
Copy link
Contributor

I also agree that this information is useful and could be added to the documentation.

Wikipedia has a few more special cases (Landau and Holtsmark distributions) and analytic cases (using Bessel functions, hypergeometric functions, Fresnel integrals, etc.) for any particularly interested readers.
https://en.wikipedia.org/wiki/Stable_distribution#Special_cases
https://en.wikipedia.org/wiki/Stable_distribution#Other_analytic_cases

@ragibson
Copy link
Contributor

I also think the current location in the notes is reasonable -- it essentially comes at the end of the distribution overview and right before the more technical implementation details.

Copy link
Contributor

@steppi steppi left a comment

Choose a reason for hiding this comment

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

This looks good to me. The information is useful and I also agree with its placement in the notes.

@mdhaber
Copy link
Contributor

mdhaber commented Feb 20, 2022

OK. I'll go ahead and merge as-is, and if we want to add additional information later, we can. Thanks @jcalbert and all!

@mdhaber mdhaber merged commit 7818647 into scipy:main Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.stats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants