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

Updating sympy.stats.crv_types for extreme value distributions #16528

Merged
merged 17 commits into from Jun 16, 2019

Conversation

Projects
None yet
6 participants
@czgdp1807
Copy link
Member

commented Apr 1, 2019

References

[1] https://en.wikipedia.org/wiki/Generalized_extreme_value_distribution#Link_to_Fr%C3%A9chet,_Weibull_and_Gumbel_families
[2] https://en.wikipedia.org/wiki/Gumbel_distribution
[3] Conitnued #16381
[4] https://prism.ucalgary.ca/bitstream/handle/11023/796/ucalgary_2013_ji_chaoqun.pdf?sequence=4&isAllowed=y

Brief description of what is fixed or changed

PDF for GumbelDistribution has been corrected with changes made at appropriate places(docs, tests, code)
CDF for GumbelDistribution has been added. Associated test have also been added

Other comments

N/A

Future Work

The following tasks will be done for 1.5, mentioned in the order of decreasing priority,

Adding GumbelMinimumDistribution, GumbelMaximumDistribution and modifying Gumbel function.

Release Notes

  • stats
    • Gumbel distribution(minimum and maximum) have been added to sympy.stats
Added CDF and corrected PDF of Gumbel Distribution
The PDF of Gumbel distribution has been corrected.
The CDF has also been added. Associated tests have
also been added.
@sympy-bot

This comment has been minimized.

Copy link

commented Apr 1, 2019

Hi, I am the SymPy bot (v147). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.5.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References
[1] https://en.wikipedia.org/wiki/Generalized_extreme_value_distribution#Link_to_Fr%C3%A9chet,_Weibull_and_Gumbel_families
[2] https://en.wikipedia.org/wiki/Gumbel_distribution
[3] Conitnued https://github.com/sympy/sympy/pull/16381
[4] https://prism.ucalgary.ca/bitstream/handle/11023/796/ucalgary_2013_ji_chaoqun.pdf?sequence=4&isAllowed=y

#### Brief description of what is fixed or changed
PDF for GumbelDistribution has been corrected with changes made at appropriate places(docs, tests, code)
CDF for GumbelDistribution has been added. Associated test have also been added


#### Other comments
N/A

#### Future Work
The following tasks will be done for `1.5`, mentioned in the order of decreasing priority,

Adding `GumbelMinimumDistribution`, `GumbelMaximumDistribution` and modifying `Gumbel` function.

#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* stats
  * Gumbel distribution(minimum and maximum) have been added to `sympy.stats`
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@sidhantnagpal and @sylee957 Please see this. I have closed #16381 due to git issues caused due to rebasing. Apologies for inconvenience caused.

@sidhantnagpal

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

The base branch should be 1.4.

@czgdp1807 czgdp1807 changed the title Added CDF and corrected PDF of Gumbel Distribution [WIP] Added CDF and corrected PDF of Gumbel Distribution Apr 1, 2019

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@sidhantnagpal Please read the future work section of the main comment of this PR. This is for 1.5.

@codecov

This comment has been minimized.

Copy link

commented Apr 1, 2019

Codecov Report

Merging #16528 into master will increase coverage by 0.276%.
The diff coverage is 81.25%.

@@              Coverage Diff              @@
##            master    #16528       +/-   ##
=============================================
+ Coverage   74.126%   74.403%   +0.276%     
=============================================
  Files          620       622        +2     
  Lines       160572    160785      +213     
  Branches     37677     37738       +61     
=============================================
+ Hits        119026    119629      +603     
+ Misses       36140     35836      -304     
+ Partials      5406      5320       -86
@sidhantnagpal

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

It looks like the Future Work section was added afterwards. By the way, since you opened this prior to #16529, the description here would be obsolete once 1.4 is merged into master (ie after the release). I'd recommend updating the description and title to bring it more in line with the exact changes you intend to make.

@sylee957

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

Technically, your commit hashes are different as you can see, probably because of your attempt to edit commit in one of the clones, in different way.

f1b8d1d 39dc1d6

Actually, I think that the github would automatically have closed this PR, if the merged one had the identical commit as this one, and wouldn't have shown some duplicate diffs, surprisingly.

@czgdp1807 czgdp1807 changed the title [WIP] Added CDF and corrected PDF of Gumbel Distribution [WIP] Updating sympy.stats.crv_types for extreme value distributions Apr 1, 2019

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@sidhantnagpal I have updated the description and the title.

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2019

@sidhantnagpal and @sylee957 Please review the changes I have made for adding GumbelDistributionMinimum and GumbelDistributionMaximum.
I also want to know what options to pass to latex() to correct the docstrings, if there are any problems with it.
If there are any suggestions to apply DRY in this PR, then please let me know.

czgdp1807 added some commits Apr 2, 2019

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2019

@sidhantnagpal This job is failing.
Please suggest some way to resolve it. I cannot understand if it's related to my changes.

@sylee957
Copy link
Member

left a comment

Warning, treated as error:
/home/travis/build/sympy/sympy/sympy/stats/crv_types.py:docstring of sympy.stats.Gumbel:9:Unexpected indentation.

It is a sphinx error while building your doc.
I see you have some inconsistent indentation, one for Parameters, and two others for math:: directive,
but I'm only sure about fixing parameters, and not very sure about correctly formatting math in multiple lines, yet.

If my suggestion doesn't fix your issue, you may also have to take a look at your math directives, as well.
Generally, you would have to strictly make indentations, 4 spaces, consistently for each paragraph.

Show resolved Hide resolved sympy/stats/crv_types.py Outdated
Update sympy/stats/crv_types.py
Co-Authored-By: czgdp1807 <singh.23@iitj.ac.in>
@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

@sylee957 Thanks for your suggestions. I have committed the same.
Any reviews from your side regarding the code that I have added for completing task 1 of future work mentioned in the description.

  1. Adding GumbelMinimumDistribution, GumbelMaximumDistribution and modifying Gumbel function.
@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

@jksuom @sidhantnagpal Any comments on this from your side?

Show resolved Hide resolved sympy/stats/crv_types.py Outdated

@czgdp1807 czgdp1807 changed the title [WIP] Updating sympy.stats.crv_types for extreme value distributions [WIP][GSoC] Updating sympy.stats.crv_types for extreme value distributions May 26, 2019

czgdp1807 added some commits May 26, 2019

@czgdp1807 czgdp1807 added the GSoC label Jun 9, 2019

@czgdp1807 czgdp1807 changed the title [WIP][GSoC] Updating sympy.stats.crv_types for extreme value distributions [WIP] Updating sympy.stats.crv_types for extreme value distributions Jun 9, 2019

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

Please check the computation by wolfram alpha for charateristic function of Minimum Gumbel Distribution.
I don't think there is any result for it. We should return unevaluated integrals as is done by wolfram alpha. What do you say?

@sylee957

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

In Mathematica, it evaluates into gamma function

In[93]:= PDF[ExtremeValueDistribution[\[Mu],\[Beta]],x]
Out[93]= E^(-E^(((-x+\[Mu])/\[Beta]))+(-x+\[Mu])/\[Beta])/\[Beta]

In[94]:= PDF[GumbelDistribution[\[Mu],\[Beta]],x]
Out[94]= E^(-E^(((x-\[Mu])/\[Beta]))+(x-\[Mu])/\[Beta])/\[Beta]

In[95]:= CharacteristicFunction[ExtremeValueDistribution[\[Mu],\[Beta]],x]
Out[95]= E^(I x \[Mu]) Gamma[1-I x \[Beta]]

In[96]:= CharacteristicFunction[GumbelDistribution[\[Mu],\[Beta]],x]
Out[96]= E^(I x \[Mu]) Gamma[1+I x \[Beta]]

Also, you may try alternative query than calling integral in Wolfram Alpha, like
https://www.wolframalpha.com/input/?i=characteristic+function+of+gumbel+distribution

I'm not sure if the fourier-type integral can succeed in SymPy or Mathematica, but if the integral fails in SymPy, you may also add some TODO note about it.

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

Thanks @sylee957

@sylee957

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

Besides, I see Wolfram defines minimal and maximal gumbel distribution as

https://reference.wolfram.com/language/ref/GumbelDistribution.html
and
https://reference.wolfram.com/language/ref/ExtremeValueDistribution.html

I don't know the behind-the-scenes idea about how they had named like that, though.

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

I am doubtful that they are using direct results. As the same system is unable to solve the integral. I don't know if I passed the incorrect formula to the integral. Can you verify that?

@sylee957

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

image
I think that the integral succeeds with assumptions specified, but it took more than a minute to compute those integrals, while calling CharacteristicFunction only took a second

So, with similar assumptions, does it succeed in SymPy?

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Jun 12, 2019

Code

from sympy.stats import Gumbel, density
from sympy import integrate, symbols, exp, I, S
mu, beta = symbols('mu, beta', positive=True)
t, x = symbols('t, x', real=True)
G = Gumbel('G', beta, mu)
f = density(G)(x)
z = integrate(f * exp(I*t*x), (x, S.Reals))
print(z)

Output

exp(mu/beta)*Integral(exp(-x/beta)*exp(-exp(mu/beta)*exp(-x/beta))*exp(I*t*x), (x, -oo, oo))/beta

@sylee957 Can you please verify. As far as I can figure out, it doesn't succeed to give closed form expression.

I think that the integral succeeds with assumptions specified, but it took more than a minute to compute those integrals, while calling CharacteristicFunction only took a second

The difference in speed suggests that they are also using pre-computed results. So, I think there is nothing wrong in using pre-computed results in SymPy.

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Jun 12, 2019

@Upabjojr @sidhantnagpal Please review the changes I have made, so that I can move on to task 2 of the future work.

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Jun 12, 2019

So now, I will proceed with other GEV distributions to have the min-max choice.

Edit - There is no need as there doesn't exist any min-max choice for other GEV distributions.

@czgdp1807

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

The work on this PR is complete. If possible please merge it.

@czgdp1807 czgdp1807 changed the title [WIP] Updating sympy.stats.crv_types for extreme value distributions Updating sympy.stats.crv_types for extreme value distributions Jun 13, 2019

Show resolved Hide resolved sympy/stats/crv_types.py Outdated
@Upabjojr

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2019

I don't see any flaw in the code. @sidhantnagpal , you can decide when to merge.

@sidhantnagpal

This comment has been minimized.

Copy link
Member

commented Jun 16, 2019

Looks good to me.

@sidhantnagpal sidhantnagpal merged commit ffa2a22 into sympy:master Jun 16, 2019

3 checks passed

codecov/project 74.403% (target 0%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
sympy-bot/release-notes The release notes look OK
Details

@czgdp1807 czgdp1807 deleted the czgdp1807:stats_simple branch Jun 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.