Skip to content

Make StandardNormal and Exp1 distributions generic? #988

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
newpavlov opened this issue Jun 23, 2020 · 3 comments
Closed

Make StandardNormal and Exp1 distributions generic? #988

newpavlov opened this issue Jun 23, 2020 · 3 comments
Labels
E-question Participation: opinions wanted

Comments

@newpavlov
Copy link
Member

Right now only those two distributions are not generic over float type. For consistency it could be nice to make them generic as well, probably with an additional trait bound specific to Ziggurat.

@dhardy
Copy link
Member

dhardy commented Jun 25, 2020

Wouldn't this just make them a pain to use? For the parametrised distributions, the Float type can be inferred from arguments, but without arguments it must be explicit for these distributions. Of course the trade-off is that sampled values may have their type inferred.

@newpavlov
Copy link
Member Author

I was thinking about potential uses with custom float types (e.g. SIMD vectors), but looks like in the current form Float trait is not well suited for that. Feel free to close this issue.

@dhardy
Copy link
Member

dhardy commented Jun 25, 2020

At a minimum, SIMD support would require a redesign of the ziggurat function; this is currently internal; to allow external crates to implement SIMD would require making it a public trait. I'm not sure we want that.

The num-traits author makes a good point about expermental crates building on conservative ones, but in this case we'd have to significantly adjust rand_distr to allow this and expect the SIMD crate to implement a variant of ziggurat.

In my opinion it would be better to keep this within rand_distr (though potentially still with a trait, to allow impl<F: Ziggurat> Distribution<F> for Exp1).

@dhardy dhardy closed this as completed Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-question Participation: opinions wanted
Projects
None yet
Development

No branches or pull requests

2 participants