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 of subviews and views with compile-time dimensions #237

Closed
gahansen opened this issue Apr 9, 2016 · 13 comments
Closed

Use of subviews and views with compile-time dimensions #237

gahansen opened this issue Apr 9, 2016 · 13 comments
Assignees
Labels
Enhancement Improve existing capability; will potentially require voting Feature Request Create new capability; will potentially require voting
Milestone

Comments

@gahansen
Copy link

gahansen commented Apr 9, 2016

I’m trying to figure out how to use subviews.

I have the following view types that I’d like to use:

typedef Kokkos::View< Scalar_[SpatialDim], Kokkos::LayoutStride, execution_space > geom_array_type ;
typedef Kokkos::View< Scalar_[SpatialDim][NumStates], Kokkos::LayoutStride, execution_space > geom_state_array_type ;

NumStates are = 2

Velocity is:

geom_state_array_type velocity;

I want to take the zeroith state of velocity and access it through a geom_array_type subview:

  geom_array_type vel_subview_state_0(subview(velocity, ALL (), ALL (), 0));

and the second through a different subview:

  geom_array_type vel_subview_state_1(subview(velocity, ALL (), ALL (), 1));

However, I get the errors:

/usr/local/trilinos/MPI_REL/include/KokkosExp_View.hpp:1121:7: fatal error: static_assert failed
"Incompatible View copy construction"
static_assert( Mapping::is_assignable , "Incompatible View copy construction" );
^ ~~~~~~~~~~~~~~~~~~~~~~
/Users/gahanse/Codes/ReconDriver/src/Fields.hpp:200:23: note: in instantiation of function template specialization
'Kokkos::Experimental::View<double *[3], Kokkos::LayoutStride, Kokkos::Serial>::View<double **, Kokkos::LayoutStride,
Kokkos::Device<Kokkos::Serial, Kokkos::HostSpace>, Kokkos::MemoryTraits<0> >' requested here
geom_array_type vel_subview_state_0(subview(this->velocity, ALL (), ALL (), 0));

I know if I made “geom_array_type” be a typedef like:

typedef Kokkos::View< Scalar**, Kokkos::LayoutStride, execution_space > geom_array_type ;

this would compile, but I have loads of other functions that are based on it being the typedef:

typedef Kokkos::View< Scalar*[SpatialDim], Kokkos::LayoutStride, execution_space > geom_array_type ;

Any thoughts on how I can assign a subview to the Scalar*[] version?

@hcedwar
Copy link
Contributor

hcedwar commented Apr 9, 2016

With the new View:

    auto vel_subview_state_0 = subview(velocity, ALL, ALL, 0);

    auto vel_subview_state_1 = subview(velocity, ALL, ALL, 1);

From: Glen Hansen <notifications@github.commailto:notifications@github.com>
Reply-To: kokkos/kokkos <reply@reply.github.commailto:reply@reply.github.com>
Date: Friday, April 8, 2016 at 7:49 PM
To: kokkos/kokkos <kokkos@noreply.github.commailto:kokkos@noreply.github.com>
Subject: [EXTERNAL] [kokkos/kokkos] Use of subviews and views with compile-time dimensions (#237)

I’m trying to figure out how to use subviews.

I have the following view types that I’d like to use:

typedef Kokkos::View< Scalar[SpatialDim], Kokkos::LayoutStride, execution_space > geom_array_type ;
typedef Kokkos::View< Scalar[SpatialDim][NumStates], Kokkos::LayoutStride, execution_space > geom_state_array_type ;

NumStates are = 2

Velocity is:

geom_state_array_type velocity;

I want to take the zeroith state of velocity and access it through a geom_array_type subview:

geom_array_type vel_subview_state_0(subview(velocity, ALL (), ALL (), 0));

and the second through a different subview:

geom_array_type vel_subview_state_1(subview(velocity, ALL (), ALL (), 1));

However, I get the errors:

/usr/local/trilinos/MPI_REL/include/KokkosExp_View.hpp:1121:7: fatal error: static_assert failed
"Incompatible View copy construction"
static_assert( Mapping::is_assignable , "Incompatible View copy construction" );
^ ~~~~~~~~~~~~~~~~~~~~~~
/Users/gahanse/Codes/ReconDriver/src/Fields.hpp:200:23: note: in instantiation of function template specialization
'Kokkos::Experimental::View::View Kokkos::Device, Kokkos::MemoryTraits<0> >' requested here
geom_array_type vel_subview_state_0(subview(this->velocity, ALL (), ALL (), 0));

I know if I made “geom_array_type” be a typedef like:

typedef Kokkos::View< Scalar**, Kokkos::LayoutStride, execution_space > geom_array_type ;

this would compile, but I have loads of other functions that are based on it being the typedef:

typedef Kokkos::View< Scalar*[SpatialDim], Kokkos::LayoutStride, execution_space > geom_array_type ;

Any thoughts on how I can assign a subview to the Scalar*[] version?


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHubhttps://github.com//issues/237

@crtrott
Copy link
Member

crtrott commented Apr 10, 2016

Carter, auto is not enough in this case. Glen explicitly wants subviews which retain the compile time dimensions, since that has performance implications. From what I can tell this does not currently happen, (i.e. he gets completely runtime views back).

@hcedwar
Copy link
Contributor

hcedwar commented Apr 11, 2016

Will require upgrade of the current simple subview mapping logic.

?


From: Christian Trott notifications@github.com
Sent: Sunday, April 10, 2016 10:06 AM
To: kokkos/kokkos
Cc: Edwards, Harold C
Subject: [EXTERNAL] Re: [kokkos/kokkos] Use of subviews and views with compile-time dimensions (#237)

Carter, auto is not enough in this case. Glen explicitly wants subviews which retain the compile time dimensions, since that has performance implications. From what I can tell this does not currently happen, (i.e. he gets completely runtime views back).

You are receiving this because you commented.
Reply to this email directly or view it on GitHubhttps://github.com//issues/237#issuecomment-208007196

@crtrott
Copy link
Member

crtrott commented Apr 11, 2016

Yeah I know :(, but I have been running into this in multiple cases now, even falling back to creating unmanaged views through extracting pointers. We need this for example for the BlockCSR capabilities. I believe this is a serious issue right now and we need to address it asap, since it bottlenecks performant implementations of milestone relevant things.

@crtrott crtrott self-assigned this Apr 11, 2016
@crtrott crtrott added this to the Spring 2016 milestone Apr 11, 2016
@crtrott
Copy link
Member

crtrott commented Apr 11, 2016

Scenarios which don't work:
Dynamic to Static Dimension:

Kokkos::View<int*> a("A",N0);
Kokkos::View<int[N0]> a1(a);

Similar issue:

Kokkos::View<int*[N1]> a("A",N0);
Kokkos::View<int[N1]> a1(a,0,Kokkos::ALL());

@crtrott crtrott added Enhancement Improve existing capability; will potentially require voting Feature Request Create new capability; will potentially require voting labels Apr 11, 2016
@crtrott
Copy link
Member

crtrott commented Apr 11, 2016

Oh I put the wrong commit message in. Anyway I think I have taken a first step in addressing this with commit 5df7378.

@crtrott
Copy link
Member

crtrott commented Apr 11, 2016

In addition to the first two scenarios the following works now:

// works for 
// LayoutSrc == LayoutRight && LayoutDst == LayoutRight
// LayoutSrc == LayoutRight && LayoutDst == LayoutStride
// LayoutSrc == LayoutLeft && LayoutDst == LayoutStride
// LayoutSrc == LayoutStride && LayoutDst == LayoutStride

Kokkos::View<int*[N1][N2], LayoutSrc> a(...);
Kokkos::View<int[N1][N2], LayoutDst> a_sub1(a,0,Kokkos::ALL(),Kokkos::ALL());
Kokkos::View<int[N1][N2], LayoutDst> a_sub2 = Kokkos::subview(a,0,Kokkos::ALL(),Kokkos::ALL());

Kokkos::View<int***, LayoutSrc> a(...);
Kokkos::View<int[N1][N2], LayoutDst> a_sub1(a,0,Kokkos::ALL(),Kokkos::ALL());
Kokkos::View<int[N1][N2], LayoutDst> a_sub2 = Kokkos::subview(a,0,Kokkos::ALL(),Kokkos::ALL());

@crtrott
Copy link
Member

crtrott commented Apr 11, 2016

Just a note: the constructor method is preferred for performance, it can avoid an intermediate assignment.

@crtrott
Copy link
Member

crtrott commented Apr 11, 2016

Besides more testing, another interesting usecase is assignment from LayoutStride to LayoutLeft/Right with runtime checks for higher dimensions.

crtrott added a commit that referenced this issue Apr 12, 2016
This allows in particular something like:

View<int***[N3][N4]> a("A",N0,N1,N2);
View<int[N2][N3][N4],LayoutStride> a_sub1(a,1,2,Kokkos::ALL(),Kokkos::ALL(),Kokkos::ALL());
View<int*[N3][N4],LayoutStride> a_sub2(a,1,2,Kokkos::ALL(),Kokkos::ALL(),Kokkos::ALL());
View<int***,LayoutStride> a_sub3(a,1,2,Kokkos::ALL(),Kokkos::ALL(),Kokkos::ALL());

this addresses more of issue #237
crtrott added a commit that referenced this issue Apr 12, 2016
This tests a set of new assignment/subview capabilities based on
issue #237
crtrott added a commit that referenced this issue Apr 12, 2016
@crtrott
Copy link
Member

crtrott commented Apr 13, 2016

The first batch of these is pushed to master and Trilinos

@crtrott
Copy link
Member

crtrott commented Apr 13, 2016

I am thinking about ideas how to test that illegal assignments actually fail. My current ideas are:

Compile Time Errors:
In the standalone Kokkos testing use scripting to test compilation, need to figure out how to combine that with Jenkins.

Runtime Errors:
For testing only replace "abort" with exceptions and test the assignment failure in serial.

Any thoughts?

@bmpersc
Copy link

bmpersc commented Apr 13, 2016

For runtime you might not have to conditionally replace the abort. It looks like abort just raises the signal SIGABRT, so you could trap it with a signal handler. This might not play well if you want to catch many aborts in a single executable though.

As for the compile time checks, there is no getting around that these have to build something. However, I think that we could embed that into the tests like you mentioned. Even in Trilinos if that is still desired. Basically we could make a CMakeLists.txt that has a sole purpose of testing these cases. Then the test in Trilinos would be defined to be run cmake on that CMakeLists.txt which would then create an output that we would check to ensure everything failed as expected. The CMake logic could also check for correctness and report a pass or fail if that is prefered.

@hcedwar
Copy link
Contributor

hcedwar commented Apr 27, 2016

Subview performance and assignment considerations are carried forward in issues #257 and #258.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve existing capability; will potentially require voting Feature Request Create new capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

4 participants