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

Allow (require) execution space for 1st arg of VerifyExecutionCanAccessMemorySpace #1192

Closed
mhoemmen opened this issue Oct 27, 2017 · 6 comments
Assignees
Labels
Documentation Update when users, developers do not understand the theory and/or implementation Enhancement Improve existing capability; will potentially require voting
Milestone

Comments

@mhoemmen
Copy link
Contributor

This is a copy of #178 , that got closed without being finished. We moved trilinos/Trilinos#1697 from Trilinos to Kokkos today.

@crtrott
Copy link
Member

crtrott commented Oct 27, 2017

And yes I agree with you this has been bugging me out of my mind for the last three years. Not sure why I didn't do anything about it. Also there should be a public class for this. How about just designing a new one:

template<class ExecSpace, class MemSpace>
SpaceAccessibility {
   enum readwrite;   // Can ExecSpace read and write to MemSpace
   enum write;       // Can ExecSpace write to MemSpace
   enum read;        // Can ExecSpace read from MemSpace
   enum allocatable; // Can ExecSpace allocate in MemSpace
   enum default;     // Is MemSpace the default memory space of ExecSpace
   enum fast;        // Is MemSpace the preferred fast persistent memory of ExecSpace
   enum capacity;    // Is MemSpace the preferred capacity memory of ExecSpace
};

For Example KNL with HBWSpace enabled:

SpaceAccessibility<OpenMP,HostSpace> {
   enum readwrite = 1;
   enum write = 1;
   enum read = 1;
   enum allocatable = 0; // Might be a 1 if we make our allocator threadsafe; 
   enum default = 0; 
   enum fast = 0;
   enum capacity 1 ;
};
SpaceAccessibility<OpenMP,HBWSpace> {
   enum readwrite = 1;
   enum write = 1;
   enum read = 1;
   enum allocatable = 0; // Might be a 1 if we make our allocator threadsafe; 
   enum default = 1; 
   enum fast = 1;
   enum capacity = 0;
};

Note it is not true that at least one of fast or capacity must be true. For CUDA we have CudaSpace, UVMSpace and HostPinnedSpace where CudaSpace would be fast, HostPinned capacity and UVM neither.

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Oct 27, 2017

Alas, default is a C++11 keyword. Not sure what would be better there.

I vote for "native" or "local" instead of "fast." Fast could mean different things (latency? bandwidth? good atomics but slow non-atomic access? accelerated vector scatter / gathers but slow scalar access?) depending on context.

Does "allocatable" mean "can allocate memory from that memory space inside a parallel execution region from that execution space"?

@ibaned ibaned added this to the 2017 December milestone Oct 31, 2017
@hcedwar hcedwar self-assigned this Nov 1, 2017
@hcedwar hcedwar added the Enhancement Improve existing capability; will potentially require voting label Nov 1, 2017
@hcedwar
Copy link
Contributor

hcedwar commented Nov 30, 2017

There is the currently undocumented public meta-function-class
Kokkos::SpaceAccessibility

template< typename AccessSpace , typename MemorySpace >
struct SpaceAccessibility {
  enum { accessible };  // AccessSpace can access MemorySpace
  enum { assignable };  // Can assign View<...,AccessSpace,...> = View<...,MemorySpace,...>
  enum { deep_copy };  // Can deep copy to AccessSpace::memory_space from MemorySpace
};

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Nov 30, 2017

That's an improvement, yes. It's still annoying to me that the interface requires allowing AccessSpace to be either a memory space or an execution space (otherwise assignable and deep_copy wouldn't work). However, this should at least allow AccessSpace to be an execution space, and the name isn't so wrong ;-) . Thanks!

@mhoemmen
Copy link
Contributor Author

@hcedwar May we consider SpaceAccessibility part of the public interface of Kokkos?

@hcedwar
Copy link
Contributor

hcedwar commented Nov 30, 2017

@mhoemmen Yep - it is in the Kokkos namespace.

Holding this issue open until wiki user guide is updated.

@hcedwar hcedwar added the Documentation Update when users, developers do not understand the theory and/or implementation label Nov 30, 2017
@hcedwar hcedwar modified the milestones: 2017 December, 2018 February Nov 30, 2017
@crtrott crtrott assigned crtrott and unassigned hcedwar Dec 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Update when users, developers do not understand the theory and/or implementation Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

5 participants