-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add layout_padded example #180
Conversation
dd60489
to
38b6481
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should talk about this, in case it becomes canonical. You are storing two extents objects here, even though only a single dimension is padded. I think we can simplify this. Also I don't particular like the storage order part. For example: why not simply take another layout policy as an argument, if we anyway store an inner layout? That is assuming that simply introducing layout_left_padded and layout_right_padded is not the right thing to do in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for the most part
@crtrott wrote:
An excellent point -- I had originally thought that we wanted to pad all but a single dimension; that would have justified the current design more in terms of code reuse. Padding just a single dimension suggests a different way to compute strides.
We had a nice chat about this offline -- thanks! |
Here is a summary of suggested revisions from this morning's offline discussion with Christian.
Here are the only differences between the current
This suggests that we only need two new layouts to express both As a size optimization, we only need to store the padded It would make sense for |
d2bc405
to
24df549
Compare
I've converted this to a draft, so I can finish implementing the new |
f204512
to
fcf7f5e
Compare
This is a redesign of the previous layout_padded design, based on a discussion with Christian Trott about what we might like to standardize.
1. layout_left_padded::mapping(extents_type) constructor is now only enabled if padding != dynamic_extent, using a pre-C++20 technique instead of `requires`. 2. This should hopefully fix the icpc build.
1. Implement layout_right_padded 2. Improve tests to reduce code duplication
1. Do not constrain layout mapping constructors that just take extents to static padding; just use no padding in that case. 2. Remove default padding value = 0 (not correct anyway) from the layout mapping constructors that take both extents and padding value.
is_nothrow_constructible_v requires C++17. Use is_nothrow_constructible::value instead.
Document what public constructors are missing from layout_{left,right}_padded. Bring exposition-only names in line with their counterparts in the proposal.
02e253c
to
fbb9f1c
Compare
I've handed off this work to a collaborator for now. |
This PR is superseded by PR #237. Thanks all! |
…s_impl [ready] kokkos impl of `matrix_{frob,one,inf}_norm` and `vector_sum_of_squares`
I added a
layout_padded
example. This is a strided layout that ensures a given overalignment of the stride-1 extent (either the leftmost, for column-major mode, or the rightmost, for row-major mode).I've tested this on a few different compilers, as one can see here: https://godbolt.org/z/3Kn833dj8
-std=c++2b
-std=c++20
-std=c++17
size_t
!) MSVC 19.32/std:c++latest
-std=c++14