Skip to content
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

Rename Uniform? #359

Closed
pitdicker opened this issue Mar 30, 2018 · 11 comments
Closed

Rename Uniform? #359

pitdicker opened this issue Mar 30, 2018 · 11 comments
Labels
P-high Priority: high

Comments

@pitdicker
Copy link
Contributor

pitdicker commented Mar 30, 2018

Previous discussion around the names is in dhardy#81, and a little on the first RFC discussion thread.

Uniform is the distribution that is going to replace the Rand trait. All code that now implements Rand should eventually switch over. We should make sure we are happy with the name, I don't think we make people happy breaking the same code another time...

@pitdicker
Copy link
Contributor Author

pitdicker commented Mar 30, 2018

Comment on Reddit:

This trait seems to not have an expressible rule about what values it generates, and even if there is a rule, it's not uniform: it doesn't give all possible values of the type (see the floating point types), and it's not even a uniform distribution across some subset of the values of the type (Option).

Lining up with statistics, like the rest of the distributions, it'd probably make more sense to rename Range to Uniform (I think I was the one who chose the name Range, and I now don't think it was a good choice) and call what's currently Uniform something else.

Personally I still slightly prefer the name Basic over Uniform. Because Basic makes sense for all sorts of types, Uniform potentially not. We had the name Default before, but that clashes a bit unfortunate with the Default trait in the standard library. And Normal and Standard are statistical distributions, or close to that.

If you look at the name Uniform from the statistics side (what is what we are asking for with Distribution as trait name...) it is a little unfortunate. I would definitely not rename Range to Uniform as the comment suggest though, Range is just too common a term. We may make it an alias though.

What I can imagine as a nice direction is to implement sample on Range directly, and let it live outside the distribution trait. It does not feel like a distribution to me, and it would slightly simplify imports in user code. And then make Uniform the equivalent implementing the Distribution trait.

But to focus on the important part first: should we do a last-minute (last-week or something) rename of Uniform to something like Basic?

@pitdicker pitdicker added the P-high Priority: high label Mar 30, 2018
@pitdicker pitdicker changed the title Rename Uniform Rename Uniform? Mar 30, 2018
@vks
Copy link
Collaborator

vks commented Mar 30, 2018

We had the name Default before, but that clashes a bit unfortunate with the Default trait in the standard library.

Why is that a problem? It is always an option to use it qualifiedly like distributions::Default. I think Default makes a lot more sense than Basic.

@burdges
Copy link
Contributor

burdges commented Mar 30, 2018

It's true Unifom is the wrong name. We could maybe build a true Uniform with const generics too, giving even more reason to change. I think Basic and Default give no intuition either though, so they suck too.

Instead, I'd suggest more meaningful names like Derived or Structured or StructuredUnifom or TypeHierarchicalUnifom or UnifomByType or InductiveUnifom.

If trait objects of distributions get used, then maybe one could eventually do clever ZST trait object stunts like:

impl<T> Default for dyn Distribution<T> {
    fn default() -> dyn Distribution<T> { Uniform }
}

@pitdicker
Copy link
Contributor Author

Uh-oh I have started a bikeshed and everyone has a different opinion 😄.

Personally I think Uniform can be okay, but as said in the first post we should make sure.

When would it be used?

If people continue to use Rng::gen() not all that much. Maybe, if someone wants to be explicit Rng::sample(Uniform).

Then there is implementing the distribution for user types:

use rand::distributions::{Distribution, Uniform}
impl Distribution<MyType> for Uniform {
    /* ... */
}

And it can be used in trait bounds instead of the current Rand trait (maybe not that common?). An example from mod.rs:

impl<T> Distribution<Option<T>> for Uniform where Uniform: Distribution<T> {
    /* ... */
}

Let's try it out

Uniform: a distribution that generates uniformly distributed variables for this type.

use rand::distributions::{Distribution, Uniform};

rng.sample(Uniform);

impl<T> Distribution<MyType<T>> for Uniform
    where Uniform: Distribution<T> { /* ... */ }

Default: generate variables according to the default distribution for this type.

use rand::distributions;
use rand::distributions::Distribution;

rng.sample(distributions::Default);

impl<T> Distribution<MyType<T>> for distributions::Default
    where distributions::Default: Distribution<T> { /* ... */ }

Basic: generate some basic random variables for this type.

use rand::distributions::{Distribution, Basic};

rng.sample(Basic);

impl<T> Distribution<MyType<T>> for Basic
    where Basic: Distribution<T> { /* ... */ }

Derived: generate random variables, derived for this type.

use rand::distributions::{Distribution, Derived};

rng.sample(Derived);

impl<T> Distribution<MyType<T>> for Derived
    where Derived: Distribution<T> { /* ... */ }

Structured: (not sure)

use rand::distributions::{Distribution, Structured};

rng.sample(Structured);

impl<T> Distribution<MyType<T>> for Structured
    where Structured: Distribution<T> { /* ... */ }

...
Maybe one more idea: the very generic Random

use rand::distributions::{Distribution, Random};

rng.sample(Random);

impl<T> Distribution<MyType<T>> for Random
    where Random: Distribution<T> { /* ... */ }

@dhardy
Copy link
Member

dhardy commented Mar 31, 2018

Why is that a problem? It is always an option to use it qualifiedly like distributions::Default. I think Default makes a lot more sense than Basic.

In my experience there is only one real problem: when forgetting to use rand::distributions::Default; you get horrible error messages because std's Default is in the prelude. Not a problem when you know what's going on but really not a nice experience for new Rust coders.

@dhardy
Copy link
Member

dhardy commented Mar 31, 2018

If we can't use Default then how about Standard?

The defining characteristic of this distribution is that it is what you get without any parameterisation. That makes a name like Standard appropriate (like how the Standard Normal is just Normal with standardised parameters).

@pitdicker
Copy link
Contributor Author

👍 to Standard.

@burdges
Copy link
Contributor

burdges commented Apr 1, 2018

I agree Standard kinda works but its "the standard distribution defined through type induction", not anything connected to standard normal. I'd even worry that standard normal distributions might be called Standard eventually. If you like Standard then Common captures its meaning while afaik avoiding any statistics terms.

I'm missing why "distribution is .. what you get without any parameterisation". It's true a uniform distribution has no parameters, and this distribution has no run time parameters, but..

Actually None has weight 50% in an Option<u32>, so, as a distribution, the type itself should be viewed as compile time parameter. I omitted several suggestions up thread because they failed to reference the type. If you want short name then Typed and ByType work, maybe better than Derived, Structured, Inductive, or Hierarchical. Your text might read:

ByType: a distribution derived via type induction with common std types given uniform distributions.

@dhardy
Copy link
Member

dhardy commented Apr 1, 2018

All of the distributions should be typed though, unless they are only implemented for a single type (like the current statistical ones only having f64 impl), so I think Typed or Derived are just weird names. Yes, you could view the type as having compile-time-parameters, but they are not ones the user can adjust. 50% for Option::None is completely arbitrary, as is the 0..1 range for FP types.

A true uniform distribution does have parameters: the minimum and maximum values (and closed/open nature on reals), which is why we have a suggestion to rename RangeUniform.

Common is okay... not my favourite.

There is such a thing as a Standard Exponential Distribution. Calling the Standard Normal just Standard would be wrong IMO; there's also not much need for it to have a short name, so I think Standard is okay for this.

@burdges
Copy link
Contributor

burdges commented Apr 1, 2018

I think Standard sounds fine so long as people do not expect any confusion with statistics terms.

@dhardy
Copy link
Member

dhardy commented Apr 4, 2018

#369 renames to Standard, so I think this is done.

@dhardy dhardy closed this as completed Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high Priority: high
Projects
None yet
Development

No branches or pull requests

4 participants