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

How to migrate away from Rand trait (Upgrading 0.4 -> 0.6) #694

Closed
dignifiedquire opened this issue Jan 15, 2019 · 11 comments
Closed

How to migrate away from Rand trait (Upgrading 0.4 -> 0.6) #694

dignifiedquire opened this issue Jan 15, 2019 · 11 comments

Comments

@dignifiedquire
Copy link

dignifiedquire commented Jan 15, 2019

I am trying to upgrade a dependency of mine which has a trait that looks like this:

pub MyCoolTrait: Sized + Eq + Copy + Clone + 'static + rand::Rand {
    // lots of trait methods
}


// and then later for the implementations
pub struct SomeCoolType {}

impl MyCoolTrait for SomeCoolType { /* ... */ }

impl Rand for SomeCoolType {
    fn rand<R: Rng>(rng: &mut R) -> Self { /* ... */ }
}

Now I understand that Rand was effectively replaced by Standard: Distribution<T>. The issue though is that I can not use this to easily add this is into this trait, as only adding Distribution<Self> does not get satisfied if I only implement Distribution<T> for Standard.

So the question is how can I migrate this code, without breaking all of the dependent crates?

@dhardy
Copy link
Member

dhardy commented Jan 15, 2019

IIRC Rand 0.5 had a transitional implementation of Rand where the Distribution requirement was met, but this was deleted for 0.6 (we don't like to keep the backwards compatibility stuff around for too long). But since you appear to have skipped 0.5 that doesn't help (and still wouldn't help for 0.4 users since we didn't add that compatibility layer).

In theory we could add optional compatibility wrappers for 0.4 over 0.5 and 0.5 over 0.6, then let you use 0.6 for your library while users can still use older Rand releases, but that's a lot of work.

Effectively this means that migrating from Rand 0.4 to 0.6 is a breaking release for your library, hence you must bump the version (including items from an unstable crate into the API of your crate makes it impossible to promise stability in your API).

Alternatively you can copy the Rand trait into your library.

As for coding the equivalent type restriction using Rand 0.6, you can't without making a similar trait (implemented for T where Standard: Distribution<T>) anyway. When investigating removing the Rand trait we looked for existing usage but didn't find much (only some half-functional fuzz-testing code), so I'm curious what you are doing here that uses rand::Rand

@dignifiedquire
Copy link
Author

It would be fine to break to a certain degree, but the functionality really should be all kept in this single trait ideally.

This is the code I am trying to migrate: https://github.com/zkcrypto/ff/blob/master/src/lib.rs#L19 maybe you have some ideas on how this could be done in a good way.

@dhardy
Copy link
Member

dhardy commented Jan 15, 2019

What do you use this for? Obviously to sample a random instance, but semantically what does this mean and why does it happen (only testing, or perhaps IV generation)?

@dignifiedquire
Copy link
Author

This is used for mainly two purposes

@dignifiedquire
Copy link
Author

Also these are implemented for "higher level fields" like here: https://github.com/zkcrypto/pairing/blob/master/src/bls12_381/fq2.rs#L59-L66 in the sense that these are combinations of the simpler forms, that rely on the coordinates being generated through this

@dhardy
Copy link
Member

dhardy commented Jan 15, 2019

If you want to stay with the same style, then I would suggest making your own version of the Rand trait (you can either use a blanket implementation over our Standard or use more specific implementations for the types you use).

The Rand trait name did not distinguish between random variables and random generators, and also did not specify the distribution — it sounds like you are talking about uniform random variables.

I would suggest you do something like the following (or possibly remove the method and use it only as a marker trait):

trait UniformRandom {
    fn sample<R: Rng + ?Sized>(rng: R) -> Self;
}
impl<T> UniformRandom for T where Standard: Distribution<T> {
    fn sample<R: Rng + ?Sized>(rng: R) -> Self {
        rand::distribution::Standard.sample(rng)
    }
}

@dignifiedquire
Copy link
Author

This makes sense, and yes I think random variables is right for the usage here. I will try this out and report back if this covers all the use cases.

@dhardy
Copy link
Member

dhardy commented Jan 25, 2019

@dignifiedquire I'll assume that solved your problem?

@dhardy dhardy closed this as completed Jan 25, 2019
@dignifiedquire
Copy link
Author

dignifiedquire commented Jan 26, 2019 via email

@burdges
Copy link
Contributor

burdges commented Mar 20, 2019

Did you resolve this? It's clear impl Rand for SomeCoolType { .. } becomes impl Distribution<E> for Standard { } but yeah one cannot write trait Foo where Standard: Distribution<Self> { .. }.

We could introduce a specific trait like @dhardy suggests, except probably use standard in the name.

trait StandardRandom {
    fn standard_sample<R: Rng + ?Sized>(rng: R) -> Self;
}
impl<T> StandardRandom for T where Standard: Distribution<T> {
    fn standard_sample<R: Rng + ?Sized>(rng: R) -> Self {
        rand::distribution::Standard.sample(rng)
    }
}

I'm though curious why rand lacks some more general trait to serve this purpose, like say

trait Sampler<D> {
    fn sampler<R: Rng + ?Sized>(rng: R, dist: D) -> Self;
    fn sampler_iter<'a, R>(rng: &'a mut R, dist: D) -> DistIter<'a, Self, R, T>
    where D: Sized, R: Rng;
}
impl<T, D> Sampler<D> for T where D: Distribution<T> {
    fn sampler<R: Rng + ?Sized>(rng: R, dist: D) -> Self {
        dist.sample(rng)
    }

    fn sampler_iter<'a, R>(rng: &'a mut R, dist: D) -> DistIter<'a, T, R, D>
    where D: Sized, R: Rng,
    {
        dist.sample_iter(rng)
    }
}

It'd play the same roll for Distribution as the Into trait plays for From, no? Am I missing something @dhardy ?

@dignifiedquire
Copy link
Author

I haven’t looked into it again, but it is on my todo list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants