[Feature]: adds support for Gumbel Distribution

[soumyasen1809]

Solves #189

Added support for Gumbel distribution

[YeungOnion]

Looks good! Could you mention where you got the test values from in this PR?

[soumyasen1809]

Looks good! Could you mention where you got the test values from in this PR?

Hi @YeungOnion , I got the values from Wolfram alpha https://www.wolframalpha.com/input/?i=gumbel+distribution
Basically, I used the formula in the Math Input section and added the input values to get the results.

[YeungOnion]

Good reference. "As accurate as Wolfram" is absolutely acceptable.

Would you be able to reference some test suites in other libraries to find tail behavior we should test? My concern is because it is doubly exponential.

-- below added when edited
A different and one I'm more certain of for precision loss, one should consider implementing sf with f64::exp_m1 instead of the direct subtraction. Perhaps this is the only thing that would be uncovered by tests that's not far into the tail.

[YeungOnion]

additionally, two admin-esque things

1. Would you write up a conventional commit as a feat? (You can squash or not) But this will help in generating a changelog
2. Would you make this PR up to but not including the merge? Linear history is nice where we can have it.

[soumyasen1809]

Hi @YeungOnion , some unclear points:

1. Would you write up a conventional commit as a feat? (You can squash or not) But this will help in generating a changelog

I am not sure what you mean by "write up a conventional commit as a feat". Do you mean the title of the PR? It already says - [Feature]: adds support for Gumbel Distribution.
Or do you mean I should git commit --amend the last commit and add the [Feature] tag?
I don't mind keeping the commits unsquashed.

2. Would you make this PR up to but not including the merge? Linear history is nice where we can have it.
   Do you mean to do a rebase to the current commit? I just did it.

[soumyasen1809]

Would you be able to reference some test suites in other libraries to find tail behavior we should test? My concern is because it is doubly exponential.

I am not sure of any other Rust statistics lib that does Gumbel distribution. Sorry :(

[YeungOnion]

Oh I can clear those up

Or do you mean I should git commit --amend the last commit and add the [Feature] tag?

Yes, if just the first of these commits could instead be something like "feat: Add support for Gumbel..." then the tooling I use for the changelog will pick it up.

Thanks for the rebase!

I am not sure of any other Rust statistics lib that does Gumbel distribution. Sorry :(

That's okay, I don't either. Just wanting to include potential extreme values that are included in other implementations (language agnostic) of Gumbel. I didn't find values tested against in the scripy suite, just a comparison to the Kappa4 distribution.

Alternatively, you can choose some values that we should expect to be extreme to test each of the functions against. Motivating how large "extreme" is would be up to you. Floating point math is mostly magic to me.

[soumyasen1809]

Hi @YeungOnion thanks for the reply.

Yes, if just the first of these commits could instead be something like "feat: Add support for Gumbel..." then the tooling I use for the changelog will pick it up.

Ok, got it... Pushed now feat: Add support for Gumbel distribution

Alternatively, you can choose some values that we should expect to be extreme to test each of the functions against. Motivating how large "extreme" is would be up to you. Floating point math is mostly magic to me.

I have to check that. I am not sure if/how I can find it. Since most of the values were matching till at least 1e-12. I guess this analysis can be done later, after the PR is merged.

[codecov]

Codecov Report

Attention: Patch coverage is 97.77778% with 6 lines in your changes missing coverage. Please review.

Project coverage is 94.03%. Comparing base (a40ba07) to head (3dd22f4).
Report is 13 commits behind head on master.

Files with missing lines	Patch %	Lines
src/distribution/gumbel.rs	97.77%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
+ Coverage   93.94%   94.03%   +0.08%     
==========================================
  Files          52       53       +1     
  Lines       11783    12053     +270     
==========================================
+ Hits        11070    11334     +264     
- Misses        713      719       +6     

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

[YeungOnion]

Agreed, it's hard to know hard far into the tail users will need to evaluate it.

[YeungOnion]

Our CI caught a need for conditional compile on the "rand" feature, this was just added in #275 so no worries that you missed it.

Rust conditional compilations allows for including/excluding code based on some predicates, there's a crate feature that allows rand to be a dependency, so when CI ran the job excluding the feature, there was no dependency named rand and it failed.

Would you add this #cfg[...] to the implementation of the rand::distributions::Distribution trait so statrs can compile with default features off?

[soumyasen1809]

Would you add this #cfg[...] to the implementation of the rand::distributions::Distribution trait so statrs can compile with default features off?

Ok @YeungOnion , so I added

#[cfg(feature = "rand")]
impl ::rand::distributions::Distribution<f64> for Gumbel {...}

[YeungOnion]

We now have another distribution,
It looks good.
I thank you for your contribution!