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

Add variant of subview that lets users add traits #134

Closed
mhoemmen opened this issue Nov 23, 2015 · 10 comments
Closed

Add variant of subview that lets users add traits #134

mhoemmen opened this issue Nov 23, 2015 · 10 comments
Assignees
Labels
Feature Request Create new capability; will potentially require voting

Comments

@mhoemmen
Copy link
Contributor

Add Kokkos::subview variant that lets users add traits (e.g., Atomic, MemoryUnmanaged, RandomRead) to the resulting type. This is particularly useful for creating an unmanaged subview of a managed View, since it would avoid the intermediate step of assigning from a managed to an unmanaged View (before taking the subview). It might look something like this:

auto X_unmanaged = Kokkos::subview_with_traits<MemoryUnmanaged> (X, std::make_pair (3, 10));

The advantage is that users would not need to know the exact return type (which is tricky given different layouts and slices!), yet could still add traits to it.

@crtrott crtrott added the Feature Request Create new capability; will potentially require voting label Nov 23, 2015
@hcedwar
Copy link
Contributor

hcedwar commented Nov 23, 2015

Likely do-able simply as: auto x = subview< Traits >( ... );

@hcedwar
Copy link
Contributor

hcedwar commented Nov 23, 2015

The refactored View will allow compatible subview construction as
view_type y( ... );
subview_type x( y , ...subview_args... );

@hcedwar hcedwar added this to the Pre Christmas Push milestone Nov 23, 2015
@hcedwar hcedwar self-assigned this Nov 23, 2015
@mhoemmen
Copy link
Contributor Author

When you say "likely do-able," does that mean it works now, or that it's easy to make that work?

@crtrott
Copy link
Member

crtrott commented Nov 23, 2015

That means we can add that without too much trouble.

@hcedwar
Copy link
Contributor

hcedwar commented Dec 9, 2015

Putting this feature in the experimental view.

View<...> primary( ...allocation_arguments... );
View<...> subview( primary , ...subview_arguments... );

The constructor for subview performs a static_assert on whether the subview type, primary view type, and subview arguments are compatible.

@crtrott
Copy link
Member

crtrott commented Dec 9, 2015

Ok that should be good enough for now. Btw your naming was a bit unfortunate took me a bit to get what you was saying better:

View<...> a_view(...allocation_arguments...);
View<...> a_sub_view(a_view, ... subview_arguments...);

Do we still want

auto a_sub_view = subview<Traits>(a_view, ...subview_arguments...);

In the first scheme I need to know what the data_type, layout and memory space are, in the second I don't.

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Dec 9, 2015

I've already hacked up a bit of the "add traits" functionality in Tpetra, for ensuring that I get unmanaged Views, no matter what type of input View comes in. It would be nice if Kokkos provided that :-)

@hcedwar
Copy link
Contributor

hcedwar commented Dec 9, 2015

Nearly trivial addition to experimental view:

auto a_sub_view = subview< MemoryTraits >( a_view , ...subview_arguments... );

Does a static_assert that MemoryTraits are really memory traits.

hcedwar added a commit that referenced this issue Dec 10, 2015
… that have different traits than the source view.

View<...> a( ...allocation... );
View<...> b( a , ...subview_arguments... );

satisfies feature request issue #134
hcedwar added a commit that referenced this issue Dec 10, 2015
…returns a subview with the specified memory traits.

Feature request issue #134
@hcedwar hcedwar closed this as completed Dec 10, 2015
@crtrott crtrott reopened this Dec 10, 2015
@crtrott
Copy link
Member

crtrott commented Dec 10, 2015

Don't close until its in master.

@crtrott
Copy link
Member

crtrott commented Dec 11, 2015

Pushed to master

@crtrott crtrott closed this as completed Dec 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Create new capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

3 participants