OpenMPTarget: Fix reduction bug in parallel_reduce for TeamPolicy.

[rgayatri23]

The PR fixes a bug in the OpenMPTarget backend for array reduction.

[dalg24]

So the issue was both declaration of the result as an array and copy back to the host?
@lucbv please confirm that this fixes #4410

[lucbv]

@rgayatri23 @dalg24
No there is still a similar issue in the implementation of execute_array (line 197 onward).

[lucbv]

Below is the piece of code that triggers it in Kokkos Kernels, for a reproducer I suggest adding void main() that creates a 2D view filled with increasing values in each columns so you can analytically compute the result of the nrm1 operation.

template <class RV, class XMV, class SizeType = typename XMV::size_type>
struct MV_Nrm1_Right_FunctorVector {
  typedef typename XMV::execution_space execution_space;
  typedef SizeType size_type;
  typedef typename XMV::non_const_value_type xvalue_type;
  typedef Kokkos::ArithTraits<xvalue_type> XAT;
  typedef Kokkos::ArithTraits<typename XAT::mag_type> MAT;
  typedef typename XAT::mag_type value_type[];

  size_type value_count;
  typename XMV::const_type m_x;

  MV_Nrm1_Right_FunctorVector(const XMV& x) : value_count(x.extent(1)), m_x(x) {
    static_assert(Kokkos::Impl::is_view<RV>::value,
                  "KokkosBlas::Impl::MV_Nrm1_Right_FunctorVector: "
                  "R is not a Kokkos::View.");
    static_assert(Kokkos::Impl::is_view<XMV>::value,
                  "KokkosBlas::Impl::MV_Nrm1_Right_FunctorVector: "
                  "X is not a Kokkos::View.");
    static_assert(std::is_same<typename RV::value_type,
                               typename RV::non_const_value_type>::value,
                  "KokkosBlas::Impl::MV_Nrm1_Right_FunctorVector: "
                  "R is const.  It must be nonconst, because it is an output "
                  "argument (we must be able to write to its entries).");
    static_assert(RV::rank == 1 && XMV::rank == 2,
                  "KokkosBlas::Impl::MV_Nrm1_Right_FunctorVector: "
                  "RV must have rank 1 and XMV must have rank 2.");
  }

  KOKKOS_INLINE_FUNCTION void operator()(const size_type i,
                                         value_type sum) const {
    const size_type numVecs = value_count;
#ifdef KOKKOS_ENABLE_PRAGMA_IVDEP
#pragma ivdep
#endif
#ifdef KOKKOS_ENABLE_PRAGMA_VECTOR
#pragma vector always
#endif
    for (size_type j = 0; j < numVecs; ++j) {
      xvalue_type val = m_x(i, j);
      sum[j] += MAT::abs(XAT::real(val)) + MAT::abs(XAT::imag(val));
    }
  }

  KOKKOS_INLINE_FUNCTION void join(volatile value_type update,
                                   const volatile value_type source) const {
    const size_type numVecs = value_count;
#ifdef KOKKOS_ENABLE_PRAGMA_IVDEP
#pragma ivdep
#endif
#ifdef KOKKOS_ENABLE_PRAGMA_VECTOR
#pragma vector always
#endif
    for (size_type j = 0; j < numVecs; ++j) {
      update[j] += source[j];
    }
  }

  KOKKOS_INLINE_FUNCTION void join(value_type update,
                                   const value_type source) const {
    const size_type numVecs = value_count;
#ifdef KOKKOS_ENABLE_PRAGMA_IVDEP
#pragma ivdep
#endif
#ifdef KOKKOS_ENABLE_PRAGMA_VECTOR
#pragma vector always
#endif
    for (size_type j = 0; j < numVecs; ++j) {
      update[j] += source[j];
    }
  }
};

template <class RV, class XMV, class SizeType>
void MV_Nrm1_Invoke(const RV& r, const XMV& X) {
  typedef typename XMV::execution_space execution_space;
  const SizeType numRows = static_cast<SizeType>(X.extent(0));
  Kokkos::RangePolicy<execution_space, SizeType> policy(0, numRows);

  // If the input multivector (2-D View) has only one column, invoke
  // the single-vector version of the kernel.
  if (X.extent(1) > 1) {
    typedef MV_Nrm1_Right_FunctorVector<RV, XMV, SizeType> functor_type;
    functor_type op(X);
    Kokkos::parallel_reduce("KokkosBlas1::Nrm1::S1", policy, op, r);
  }
}

[lucbv]

In general I would suggest looking at what tests are disabled in Kokkos Core when KOKKOS_ENABLE_OPENMPTARGET is defined. I am pretty sure that the reproducer I show above is already covered, it is just not enabled at this time.

Issue is not resolved and no new unit test is enabled

[rgayatri23]

@rgayatri23 @dalg24 No there is still a similar issue in the implementation of execute_array (line 197 onward).

Yes I need to do the similar changes for the RangePolicy version too. I am working on the unit tests now, enabling the one that tests these features.

[rgayatri23]

Fixed the bug in RangePolicy parallel_reduce for array reductions and enabled corresponding tests.

[dalg24]

@lucbv please review the PR update

[lucbv]

Thanks @rgayatri23 for the changes. It fixes some issues but I now have more build problem with the execute_init_join method. I have to comment it's implementation to build without failures in Kokkos Kernels. We can tackle that in another PR in my opinion.

[rgayatri23]

@lucbv What issues are you facing in the execute_init_join ? The only change there is that we now initialize zero length reductions to be consistent with Kokkos "should" do.

[lucbv]

@rgayatri23
I have opened another issue regarding these, see issue #4500