Swap strided template parameters

[Dan Bonachea]

The strided RMA function templates (after generalized completion) are currently declared as:

template < typename T, int Dim ,
typename Completions = decltype ( operation_cx :: as_future ()) >
RType rput_strided (
      T const * src_base ,
      std :: ptrdiff_t const * src_strides ,
      global_ptr <T> dest_base ,
      std :: ptrdiff_t const * dest_strides ,
      std :: size_t const * extents ,
      Completions cxs= Completions {});

However this order of template parameters creates a usability nuisance in practice, because T can always be deduced from the function arguments, but Dim cannot and must always be provided explicitly.

Dim should be moved to first in the template argument list, allowing code like the following:

  double src[10][20][30];
  double dst[10][20][30];
  std::ptrdiff_t srcstride[] = { 20*30*sizeof(double), 30*sizeof(double), sizeof(double) };
  std::ptrdiff_t dststride[] = { 20*30*sizeof(double), 30*sizeof(double), sizeof(double) };
  std::size_t extent[] = { 2, 3, 4 };
  auto f = rput_strided<3>( // note the lack of T here
     &src[1][1][1], srcstride,
     &dst[1][1][1], dststride,
     extent);

[Dan Bonachea]

On a related topic, why do we specify template parameter int Dim and a precondition "Dim must be non-negative"?

Wouldn't it be simpler (and more consistent with STL templates like std::array) to make the template parameter std::size_t Dim?

[Amir Kamil]

I agree with Dan's points, as well as his suggestion on Slack to use std::array instead of pointers. Here is what I suggest for rput_strided:

template<std::size_t Dim, typename T,
         typename Completions=decltype(operation_cx::as_future())>
RType rput_strided(
  T const *src_base,
  const std::array<std::ptrdiff_t, Dim> &src_strides,
  global_ptr<T> dest_base,
  const std::array<std::ptrdiff_t, Dim> &dest_strides,
  const std::array<std::size_t, Dim> &extents,
  Completions cxs=Completions{});

This allows the following code:

  double src[10][20][30];
  global_ptr<double> dst;
  std::array<std::ptrdiff_t, 3> srcstride = { 20*30*sizeof(double), 30*sizeof(double), sizeof(double) };
  std::array<std::ptrdiff_t, 3> dststride = { 20*30*sizeof(double), 30*sizeof(double), sizeof(double) };
  std::array<std::size_t, 3> extent = { 2, 3, 4 };
  auto f = rput_strided( // note the lack of T or Dim here
     &src[1][1][1], srcstride,
     dst, dststride,
     extent);
  auto g = rput_strided<3>( // note the lack of T here
     &src[1][1][1], { 20*30*sizeof(double), 30*sizeof(double), sizeof(double) },
     dst, { 20*30*sizeof(double), 30*sizeof(double), sizeof(double) },
     { 2, 3, 4 });

The second call requires the dimensionality, since it can't be inferred from an initializer list. So the dimensionality should be the first template parameter.

[Dan Bonachea]

@akamil : The main downside I see with your proposal of requiring const std::array<*, Dim> & array arguments in {rput,rget}_strided() is for programs where the strides/extents are already stored in a built-in array. Unless I'm missing something, there doesn't seem to be a convenient (and ideally efficient) means to convert a const std::size_t * to a const std::array<std::size_t, Dim>? On the other hand, the opposite conversion is easy via std::array<..>::data(), which seems to favor the current calling convention using pointers const std::size_t * as the more universal means to pass an array to a library function, although it unfortunately loses size information/checking/inference (which is also unpleasant, but seems the marginally lesser evil).

[Dan Bonachea]

This needs to be resolved prior to the March 18 STPM17-7 milestone deliverable:

Implement new “Expanded VIS Operations” APIs utilizing corresponding GASNet-EX feature in STPM17-6; write or update the corresponding specification; and provide corresponding tests and/or examples.

[Dan Bonachea]

This issue was discussed in the 1/10/18 meeting.

We agreed to swap the template parameters as suggested.

We also resolved to provide two overloads for each of {rput,rget}_strided - one overload takes all three array arguments as C arrays (as currently specified), and a new overload takes all three array arguments as std::array's.

[Dan Bonachea]

Proposed resolution in pull request #3.

Please comment and/or approve there.

[Dan Bonachea]

Merge pull request #3 'issue110-strided-api'

Resolves issue 110

• issue110-strided-api: issue 110: Improve Strided API signatures

→ <<cset 37368ca8629d>>