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

Use const generics, try_from_fn for the Dirichlet distribution #1006

Open
newpavlov opened this issue Jul 30, 2020 · 3 comments
Open

Use const generics, try_from_fn for the Dirichlet distribution #1006

newpavlov opened this issue Jul 30, 2020 · 3 comments
Labels
B-API Breakage: API P-postpone Waiting on something else

Comments

@newpavlov
Copy link
Member

newpavlov commented Jul 30, 2020

Right now the Dirichlet distribution allocates inner state and sampled values, even though it can be naturally expressed in terms of arrays and AFAIK is usually used with fixed number of dimensions.

WeightedIndex also can be expressed in terms of arrays, but in practice number of weights can be not known at compile time. We could introduce a "fixed" variant in addition to the existing "dynamic" one.

@newpavlov newpavlov added B-API Breakage: API P-postpone Waiting on something else labels Jul 30, 2020
@vks
Copy link
Collaborator

vks commented Jul 30, 2020

This is blocked by the stabilization of minimal const generics.

@newpavlov
Copy link
Member Author

Note that we return an array with size dependent on a generic parameter, so I am not sure if minimal const generics will be enough for this. It may be blocked on rust-lang/rust#68436.

@dhardy
Copy link
Member

dhardy commented May 21, 2024

To copy what I wrote in #485:

We now use const generics: impl<F, const N: usize> Distribution<[F; N]> for Dirichlet<F, N> where ...

Caveat: we currently use Vec to construct some arrays in distribution constructors. Once core::array::try_from_fn is stable, we can use that instead.

Caveat: sometimes arrays of other lengths are required (DirichletFromBeta uses an internal array of length one less than the output length). This isn't really an issue, but currently uses an allocator (doing otherwise would require generic_const_exprs or an empty element).

Caveat: the above approach does not support run-time variable length N. We likely don't need to care about this.


Conclusion: we can improve our code once try_from_fn is stabilized, but probably not beyond that.

I tried simply copying the code behind try_from_fn as a utility fn; this is viable but requires rather more unsafe code than I'd like (besides which, we forbid unsafe from rand_distr).

@dhardy dhardy changed the title Use const generics for the Dirichlet distribution Use ~~const generics~~ try_from_fn for the Dirichlet distribution Jul 20, 2024
@dhardy dhardy changed the title Use ~~const generics~~ try_from_fn for the Dirichlet distribution Use const generics, try_from_fn for the Dirichlet distribution Jul 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-API Breakage: API P-postpone Waiting on something else
Projects
None yet
Development

No branches or pull requests

3 participants