Yes, this simple model does not introduce any safety for the compiler, it 
just IMHO could make PETSc and user code much clearer and easier to follow.

For example

   struct {
      PetscReal *A;
      PetscReal *B:
   }

 is much more confusing than

   struct {
      PetscReal *A;
      PetscKokkosReal *B:
   }

 the user immediately knows which memory space each variable is associated 
with. And we have the confusing version all over PETSc now.

    Yes, it is generally for arrays, but also makes sense for local variables 
inside a kernel; just makes the code clearer. For example 

  Kokkos::parallel_for(Kokkos::RangePolicy<ExecutionSpace>(0,nx), KOKKOS_LAMBDA 
(const PetscInt &i)
    {
      PetscKokkosReal uvleft  = eval(U,0, i, 1, .5);
      PetscKokkosReal uvright = eval(U,0, i+1, 1, -.5);
      PetscKokkosReal uwleft  = eval(U,1, i, 1, .5);
      PetscKokkosReal uwright = eval(U,1, i+1, 1, -.5);
      flux(0,i, 1) = -.5*(uwleft+uwright)+.5*alpha_LF*(uvleft-uvright);
      flux(1,i, 1) = -.5*(uvleft+uvright)+.5*alpha_LF*(uwleft-uwright);
    });

vs

  Kokkos::parallel_for(Kokkos::RangePolicy<ExecutionSpace>(0,nx), KOKKOS_LAMBDA 
(const PetscInt &i)
    {
      PetscReal uvleft  = eval(U,0, i, 1, .5);
      PetscReal uvright = eval(U,0, i+1, 1, -.5);
      PetscReal uwleft  = eval(U,1, i, 1, .5);
      PetscReal uwright = eval(U,1, i+1, 1, -.5);
      flux(0,i, 1) = -.5*(uwleft+uwright)+.5*alpha_LF*(uvleft-uvright);
      flux(1,i, 1) = -.5*(uvleft+uvright)+.5*alpha_LF*(uwleft-uwright);
    });

  I don't think there is need for conversion between PetscKokkosReal and 
PetscReal. One uses the normal code for moving data between the memory spaces.
 It is different than PetscInt and PetscBLASInt. 




> On Dec 12, 2020, at 10:46 AM, Jed Brown <[email protected]> wrote:
> 
> Barry Smith <[email protected]> writes:
> 
>>   Currently we use PetscScalar and PetscScalar * to refer to variables that 
>> could be in any memory space. On the CPU, on the GPU, in Kokkos, etc.
>> 
>>   Would it make sense to use typedef to indicate at each location the true 
>> type of the memory location when possible? 
>> 
>>   typedef PetscReal PetscKokkosReal   means the variable is in the Kokkos 
>> memory space
>>   typedef PetscReal PetscCUDAReal
>>   typedef PetscReal PetscNVSHEMReal
> 
> There's no safety here. I suppose you could do a struct to prevent implicit 
> conversion
> 
> typdef struct {PetscReal val;} PetscCUDAReal;
> 
> but even this doesn't enforce the invariant.
> 
> I think the distinction only makes sense for boxed values/arrays. If you ever 
> have a value, it's in your memory space, but it matters when you dereference 
> a pointer.
> 
> A question is how you'd convert. Presumably it requires a cudaMemcpy, so 
> you'd want the array length to be part of the type and some light interfaces 
> to do the conversions.
> 
> Some mapping of values to device memory can be piggy-backed on kernel 
> launches so I'm not sure that requiring up-front conversion is the right 
> approach (effectively adding one or more cudaMemcpy to the critical path).
> 
>>   etc. 
>> 
>>  Then one could write things like 
>> 
>>  struct {
>>     ...
>>     PetscNVSHEMReal *values;
>>  }
>> 
>>  Similarly inside kernels one would use the type type associated with the 
>> kernel, cuda with cuda etc. 
>> 
>>  I think the resulting code will be much clearer and easier to dive into, 
>> then having to first figure out where each variable lives.
>> 
>>  I find the current code confusing because one cannot immediately see a 
>> variable declaration and know where it lives, even though it does live 
>> somewhere in particular..
>> 
>>  Barry

Reply via email to