Skip to content

{Independent,}Sample trait for StandardNormal or rename to Ziggurat #97

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

Closed
SuperFluffy opened this issue Jan 24, 2016 · 8 comments
Closed
Labels
F-new-int Functionality: new, within Rand

Comments

@SuperFluffy
Copy link

All structs in rand::distributions implement the Sample and IndependentSample trait, which gives us the ability to call fn ind_sample<R: Rng>(&self, &mut R) -> Support, with Support the value of our random number.

However, StandardNormal, also in rand::distributions, does not implement the aforementioned traits, but Rand instead, which allows to call fn rand<R: Rng>(rng: &mut R) -> Self, where self will be StandardNormal(x), with x the value of our random number.

I believe this to be an inconsistency. Either StandardNormal should be moved out of rand::distributions to make it explicit that it behaves in a completely different way, or the Sample and IndependentSample traits should be implemented for it.

Alternatively, I propose StandardNormal to be renamed to ZIGNOR (or Ziggurat, I guess).

@bluss
Copy link
Contributor

bluss commented Mar 19, 2016

StandardNormal is indeed not a distribution, just a cute newtype for a float with a different Rand implementation. Just like Closed01.

@alexcrichton alexcrichton added the F-new-int Functionality: new, within Rand label Jun 14, 2017
@dhardy
Copy link
Member

dhardy commented Jul 24, 2017

Is there a good rationale for keeping the "standard distributions" like StandardNormal at all?

@pitdicker
Copy link
Contributor

I agree StandardNormal is a strange fit in distributions. It seems more like a part of the implementation of Normal and LogNormal that got accidentally exposed.

@dhardy changed it to a simple (private) function in dhardy@a10251b.

@pitdicker
Copy link
Contributor

@dhardy changed it to a simple (private) function in dhardy/rand@a10251b.

Now landed as part of #256.

@dhardy dhardy closed this as completed Mar 3, 2018
@pitdicker
Copy link
Contributor

Thanks for closing all the bugs 😄. Do you think I could get permission to also do so?

@dhardy
Copy link
Member

dhardy commented Mar 3, 2018

Thanks! I don't actually have permission to do that; @alexcrichton?

@alexcrichton
Copy link
Contributor

Sure! @dhardy I've made you an admin for the repo and invited @pitdicker as a collaborator

@pitdicker
Copy link
Contributor

@alexcrichton Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-new-int Functionality: new, within Rand
Projects
None yet
Development

No branches or pull requests

5 participants