I put a &self receiver, like in Distribution @dhardy was there a specific reason why you proposed &mut self?

I am not sure if I want a different name, because it clashed with the one in Distribution and I would like structs to implement both if applicable. Making these things unambiguous feels often too cumbersome in Rust. And it can break existing code when doing use rand_distr::* or similar.

I put a &self receiver, like in Distribution @dhardy was there a specific reason why you proposed &mut self?

Sorry, it should be &self.

I am not sure if I want a different name, because it clashed with the one in Distribution and I would like structs to implement both if applicable. Making these things unambiguous feels often too cumbersome in Rust. And it can break existing code when doing use rand_distr::* or similar.

I'm not really sure of the answer to that. Any of these can work (from the POV of a dependency):

• use rand_distr::MultiDistribution; — Do we want the multi module to be pub? Likely yes, in which case supporting this path is redundant.
• use rand_distr::multi::Distribution; — Usable but potential for confusion and can make usage of both Distribution traits annoying
• use rand_distr::multi::MultiDistribution; — Redundant naming, but avoids the above issues so probably the best choice

I was talking about the naming of sample because its the same as in distribution and makes the method ambiguous when both traits are in scope.

I guess I also prefer the last of the 3 options for the name of the trait.

Also, should we do this in a v0.6 branch? I guess there will be breaking changes with respect to the Dirichlet at the end and possible some v0.5.* releases.

I was talking about the naming of sample because its the same as in distribution and makes the method ambiguous when both traits are in scope.

This shouldn't be an issue unless a type implements both Distribution and MultiDistribution — but I doubt we'd want that.

Except possibly a symmetric distribution like Dirichlet which could be sampled in one or multiple dimensions? Or if we wanted to transparently support uncorrelated multi-dimensional sampling of 1D distributions like Normal? No, both these are likely bad ideas.

I was talking about the naming of sample because its the same as in distribution and makes the method ambiguous when both traits are in scope.

This shouldn't be an issue unless a type implements both Distribution and MultiDistribution — but I doubt we'd want that.

Except possibly a symmetric distribution like Dirichlet which could be sampled in one or multiple dimensions? Or if we wanted to transparently support uncorrelated multi-dimensional sampling of 1D distributions like Normal? No, both these are likely bad ideas.

I was thinking about both having MultiDistribution to sample into a buffer and still Distribution which returns a Vec. If someone would anyway allocate for each sample or does not care about allocations, the latter is more convenient.
But I could understand if this would lead to more confusion than it benefits people.

The point of using const generics was to avoid needing to allocate. If allocation is necessary anyway, having both sample_to_buf and sample(...) -> Vec<_> doesn't add much. Moreover, if we are going to support both styles of method, it should be done under the same trait in my opinion — we can implement sample automatically, provided we know the expected sample length.

Which leads to another point: we may want a sample_len or just len method.

So:

pub trait MultiDistribution<T> {
    fn sample_len(&self) -> usize;
    fn sample_to_buf(&self, buf: &mut [T]);
    fn sample(&self) -> Vec<T> where T: Default {
        let mut buf = Vec::new();
        buf.resize_with(self.sample_len(), T::default());
        self.sample_to_buf(&mut buf);
    }
}

That requires T: Default to support sample, which I think is reasonable.

We should decide if we want to keep the possibility to have multidim sampling without allocations or not. If we do not need it, we can ditch the const generics and have less code and then your proposed Trait would make a lot of sense.

I am still leaning toward keeping it, because I had a usecase where Multinomial samples where extremely time critical and it helps to save the allocations, especially in multithread where there is synchronization with malloc. But this might also be a niche usecase. (I would only sample once per Multinomial)

Edit: Your Trait is also still implementable for a const generic version and you can avoid all allocations. So I would go with this approach.

I thought we did decide to drop the const-generics approach for rand_distr?

Your use-case sounds fairly specific. Maybe there are further optimisations available when sampling only once.

Would it be possible to have a non-const generic Dirichlet that can still be used without allocating? Without discussing Distribution traits for now, imagine the following API:

impl<F: Float> Dirichlet<F> {
    pub fn new(alpha: Vec<F>) -> Result<Dirichlet<F>, Error>;
    pub fn sample<R: Rng>(&self, rng: &mut R, output: &mut Vec<F>); // Users can re-use this vector to avoid allocations
    pub fn into_vec(self) -> Vec<F>; // Recovers the memory passed in with `new()`
}

This still requires the alloc feature, but so does the current (const generic) implementation, and afaict no one has presented such a use case yet.

For sample(), we could also generalize the output type to iter::Extend, if we want to allow collecting into other data structures.

Would it be possible to have a non-const generic Dirichlet that can still be used without allocating? Without discussing Distribution traits for now, imagine the following API:

impl<F: Float> Dirichlet<F> {
    pub fn new(alpha: Vec<F>) -> Result<Dirichlet<F>, Error>;
    pub fn sample<R: Rng>(&self, rng: &mut R, output: &mut Vec<F>); // Users can re-use this vector to avoid allocations
    pub fn into_vec(self) -> Vec<F>; // Recovers the memory passed in with `new()`
}

This still requires the alloc feature, but so does the current (const generic) implementation, and afaict no one has presented such a use case yet.

For sample(), we could also generalize the output type to iter::Extend, if we want to allow collecting into other data structures.

In the case of Dirichlet it needs an array of other distributions (Beta or Gamma), so this would not work. I would also say its a bit to complex of an API.

My current favorite would be something like this:

pub trait MultiDistribution<T> {
    /// Returns the length of one sample (dimension of the distribution)
    fn sample_len(&self) -> usize;
    /// Samples from the distribution and writes the result to `buf`
    fn sample_to_buf<R: Rng + ?Sized>(&self, rng: &mut R, buf: &mut [T]);
}

impl<T, D> Distribution<Vec<T>> for D
where
    D: MultiDistribution<T>,
    T: Clone + Default,
{
    fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> Vec<T> {
        let len = self.sample_len();
        let mut buf = vec![Default::default(); len];
        self.sample_to_buf(rng, &mut buf);
        buf
    }
}

The distributions would implement MultiDistribution and we get the Distribution<Vec<T>> implementation for free.

Unfortunately this does not work because of orphan rules. It also does not feel right to define the MultiDistribution in rand, so I am not sure if this can work.

We could just implement Distribution manually (we could then also distinguish between Vec<T> and [T;3] for example).

I think we can keep the MultiDistribution design. The implementation of Distribution is pretty ugly, but this is not a problem for the users and not too bad to handle for us.

Final question: Should we keep the const generic Dirichlet and add a non const generic one, or just replace it.

The implementation of Distribution is pretty ugly

This is fine IMO.

Should we keep the const generic Dirichlet and add a non const generic one, or just replace it.

That question shouldn't be directed here; can we adjust it in a new PR?

From what I remember, there didn't appear to be much use (or evidence for use) for the const-generic implementation.

So in this PR, we keep the original const generic Dirichlet, just implementing Multidistribution?
And then merging modified #15 later?

Needs cargo fmt.