-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add pareto distribution #82
base: master
Are you sure you want to change the base?
Conversation
This is necessary to account for the jax.random.pareto function using the type II Pareto distribution
If we note While on the other hand PyMC3 implements What I would do is leave |
Sounds reasonable to me 👍 |
|
Also added Pareto distribution to mcx.distribution init
f8f3e6b
to
965f6dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking the time to add the Pareto distribution! It will be ready to merge once we have tests for the sampling shape and support correctness.
numerator = (scale ** 2) * shape | ||
denominator = ((shape - 1) ** 2) * (shape - 2) | ||
return numerator / denominator | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great addition! However, before I merge we'll need to add tests for the shape and the support! Would you mind adding those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed, was planning on it when I get some time, hopefully in the next few days!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there anything I can do to help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let you know when I get to it this weekend. Last 2.5 weeks I had a kidney stone which involved two surgeries and like 5 nights in hospitals, but things seem to be resolved now. 2021 has not been my year in terms of health. Nonetheless, I should finally have some time this weekend!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I found some time to add more tests - but I'm having one issue with a failing test for the variance in the case when the shape parameter is <= 2 and I'm not entirely sure what I've implemented wrong. Not being totally familiar with the Pareto distribution i've kind of just followed the information on https://en.wikipedia.org/wiki/Pareto_distribution which is stating that the variance should be infinite when the shape parameter is <= 2, however this is not the case in the current implementation. I'd appreciate some feedback!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extensive test suite, great job!
Remember that we defined shape = b
in this case. The variance should thus be theoretically infinite when
Then, if you measure the variance of samples drawn from the distribution, you should get a very large number but not strictly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I'll update the tests to reflect this. Thanks for the clarification!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry was away from this too long and am a bit confused because in your suggestion you are using
numerator = (scale ** 2) * shape
denominator = ((shape - 1) ** 2) * (shape - 2)
return numerator / denominator
I get that for that variance of the samples won't strictly be
Hi @tblazina Looks like the tests are not passing :( Are you still planning on working on this? |
Hi @rlouf - sorry about that, this fell by the wayside but I would plan on finishing this yes! I'll try to get to it in the next few days and if I don't think I can get around to doing it I will let you know! |
I'm not familiar really with this distribution and am a bit confused with all the different ways it is parameterized depending on which library you look at, for example Stan and PyMC3 both use a shape and scale parameter but the jax.scipy.stats implementation uses a parameter
b
as well as aloc
andscale
parameter. I guess the use of theb
parameter stems from thejax.random.pareto
function which I seems to be similar to the Numpy implementation where it is the "The Lomax or Pareto II distribution is a shifted Pareto distribution". I am not sure which would be preferable to use, some guidance/input would be appreciated. 🙏