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

Integrate min-const-generics. #820

Merged
merged 17 commits into from
Apr 12, 2021
Merged

Integrate min-const-generics. #820

merged 17 commits into from
Apr 12, 2021

Conversation

sebcrozet
Copy link
Member

@sebcrozet sebcrozet commented Jan 3, 2021

Fix #852, fix #858, fix #621, fix #590, fix #526, fix #373

This PR isn't intended to be merged right now. It won't be merged before min-const-generics is stabilized. Though I wanted to submit this to show that min-const-generics is powerful enough to let us replace generic-array by a regular array based on const-generics.

This PR does not do everything we can change when min-const-generics will stabilize, but it contains all the changes needed to make everything work. More changes can be made to make the API a bit nicer though (e.g. to allow the user to write Point<f32, 10> instead of Point<f32, Const<10>> for example).

The idea here is to replace the typenum numbers (and our own number types U1, U2, etc.) by a new struct wrapping an integer: struct Const<const T: usize>, and define an array storage like this:

struct ArrayStorage<N, const R: usize, const C: usize> {
    data: [[N; R]; C]
}

Then we have to adapt all the relevant trait implementations and voilà!

Limitation

There is one limitation here. Because min-const-generics does not allow operations on the const parameter, some method modifying the dimension of a matrix won't work. For example Vector<N, Const<2>>::push should return a Vector<N, Const<3>>, yet we can't write something like this:

impl<const R: usize> Vector<N, Const<R>> {
    fn push(&self) -> Vector<N, Const<{R + 1}>>;
}

because the {R + 1} isn't allowed yet. So we still need to use typenum as a workaround here. Basically, we have two traits ToTypenum and ToConst which we use to map Const<1> to typenum::U1, Const<2> to typenum::U2, etc. That way, we can type our push function like this:

impl<const R: usize> Vector<N, Const<R>> {
    fn push(&self) -> Vector<N, <Sum<<Const<R> as ToTypenum>::Output, typenum::U1> as ToConst>::Output
    where Const<R>: ToTypenum,
               <Const<R> as ToTypenum>::Output: Add<typenum::U1>,
               Sum<<Const<R> as ToTypenum>::Output, typenum::U1>: ToConst;
}

The code I use in nalgebra for this is actually much nicer because all the gory details can be hidden behind our DimSum, DimSub, etc. traits (which already existed before this PR).

So the limitation here is that methods like push that do some operations on the dimension of the input matrix will only work with matrices having dimensions with a type implementing ToTypenum. However we can only implement our ToTypenum and ToConst traits for a finite number of types. In this PR I implemented ToTypenum for Const<1> to Const<128>. We may add more if someone needs it. So right now methods like push will only work on statically-sized matrices with dimensions smaller than 128 (but it will continue to work on all dynamically-sized matrices). Methods that don't rely on this kind of type-level dimension addition or subtraction, will work with all dimensions (including these that don't implement ToTypenum).

@sebcrozet sebcrozet force-pushed the min_const_generics branch from 3d0f68f to cc4427e Compare April 11, 2021 12:07
@sebcrozet
Copy link
Member Author

sebcrozet commented Apr 12, 2021

I believe I have done everything I can think of regarding the integration of const-generics to nalgebra.
Here are some benefits of const-generics:

  • The DefaultAllocator: Allocator traits are no longer needed whenether only dimensions given as const integers are specified. For example the code related to the Point<T, const D: usize> no longer requires any allocator bounds.
  • Debugging vectors/matrices with small sizes becomes much nicer because the debugger gives a much nicer view of arrays as compared to GenericArray.
  • The new constructors of matrices, vectors, points, and translations, with a number of rows and columns smaller or equal to 6, are now const fn.
  • The Quaternion::new constructor is also a const fn.

Because Rust doesn't allow to write a type parameter to be either a const or a type, I had to make some changes to our aliases. In particular, we have:

  • OMatrix<T, R: Dim, C: Dim>: for owned matrices (formerly called MatrixMN).
  • SMatrix<T, const R: usize, const C: usize>: for statically-allocated matrices.
  • DMatrix<T>: for dynamically-allocated matrices.

The same applies to vectors: OVector, SVector, DVector.
The MatrixN alias has been deprecated for simplicity (just use OMatrix<T, D, D> instead).

I renamed the scalar type parameter from N to T because N wasn't idiomatic.

@sebcrozet sebcrozet merged commit ee2e0be into dev Apr 12, 2021
};

use std::ops;

// N.B.: Not a public trait!
// T.B.: Not a public trait!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I don't think this N was supposed to be renamed to T!

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