[petsc-dev] using typedef for each different memory space the variable lives in

[Barry Smith]

   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 <jed at jedbrown.org> wrote:
> 
> Barry Smith <bsmith at petsc.dev> 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