Implement Pareto distribution #495
Conversation
|
Okay, but I'm starting to wonder if we shouldn't discuss #290 before continuing to add more distributions. Is there some specific motivation for this one? |
src/distributions/pareto.rs
Outdated
| /// | ||
| /// # Panics | ||
| /// | ||
| /// `scale` and `shape` have to be non-zero and positive. |
There was a problem hiding this comment.
May be worth mentioning common variable names such as k, x_m and α.
There was a problem hiding this comment.
I agree and added those to the docs.
|
As I understand it was primarily about adding new distributions. And no, we don't currently have any framework for choosing whether to add new distributions, this is just an arbitrary point to bring it up (also since you opened #290 originally). |
src/distributions/pareto.rs
Outdated
| impl Distribution<f64> for Pareto { | ||
| fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> f64 { | ||
| let u: f64 = rng.sample(OpenClosed01); | ||
| self.scale * u.powf(-1.0 / self.shape) |
There was a problem hiding this comment.
It should be equivalent to store -1 / shape as a field which should speed up sampling.
src/distributions/pareto.rs
Outdated
| let d = Pareto::new(1.0, 2.0); | ||
| let mut rng = ::test::rng(1); | ||
| for _ in 0..1000 { | ||
| d.sample(&mut rng); |
There was a problem hiding this comment.
In theory the sample must never be less than the shape parameter, so you should either test this or give some small allowance for rounding (but hopefully this isn't necessary).
There was a problem hiding this comment.
I assume you meant the scale parameter? Done.
It is as extensive as the testing of |
|
Looks good. Still missing a benchmark but not required. Any further comments before merging? If not I'll probably merge tomorrow. |
No description provided.