Prefer Kokkos::Experimental::sort_by_key() and drop rocThrust/oneDPL dependencies

[dalg24]

Defer to Kokkos and get rid of all the churn, both on the cmake-side to handle dependencies and the code path specializations.
I kept Details::KokkosExt::sortByKey() because I don't think Kokkos pushes a profiling region.

[dalg24]

I am not sure how much we care about the ARBORX_ENABLE_{ONEDPL,ROCTHRUST} exported CMake variables and macros. I just dropped them. We could technically map them to their Kokkos counterparts but I don't think it is worth the trouble.

[dalg24]

In file included from /opt/kokkos/include/Kokkos_Sort.hpp:25:
In file included from /opt/kokkos/include/sorting/Kokkos_SortPublicAPI.hpp:20:
/opt/kokkos/include/sorting/./impl/Kokkos_SortImpl.hpp:73:10: fatal error: 'oneapi/dpl/execution' file not found
#include <oneapi/dpl/execution>
         ^~~~~~~~~~~~~~~~~~~~~~

[dalg24]

The CUDA-12.0.1-NVCC-CUDA-AWARE-MPI build failure is unrelated and denotes an issue with the testing machine (fetnat_06)

nvlink fatal   : Could not open input file '/usr/lib/x86_64-linux-gnu/libcuda.so'

[aprokop]

I tried the same in #1191. I remember seeing some performance regression. You would need to run a lot of experiments to make sure there's none.

[dalg24]

I tried the same in #1191. I remember seeing some performance regression. You would need to run a lot of experiments to make sure there's none.

Should I assume you tested with the performance fix on random access iterators kokkos/kokkos#7304 ?

[aprokop]

Should I assume you tested with the performance fix on random access iterators kokkos/kokkos#7304 ?

Don't know. Regardless, we depend on Kokkos 4.5, and that patch appeared only in 4.6.01.

[dalg24]

we depend on Kokkos 4.5, and that patch appeared only in 4.6.01.

Kokkos 5.0 is a couple weeks out. Are we still committed to only supporting the last 2 Kokkos minor releases or is that something you were wanting to change.

[aprokop]

Kokkos 5.0 is a couple weeks out. Are we still committed to only supporting the last 2 Kokkos minor releases or is that something you were wanting to change.

I had bad experience of keeping with the last two releases, as propagating the new features into the projects that depend on Trilinos is so problematic.

[aprokop]

Trilinos 16.2 was released with Kokkos 4.7.1.

[dalg24]

Is this PR still blocked by anything now that Trilinos has had a few releases (17+) that are based on Kokkos 5?

[aprokop]

Is this PR still blocked by anything now that Trilinos has had a few releases (17+) that are based on Kokkos 5?

iirc it had a performance regression due to Kokkos iterators, but it was flaky, and I don't have a solid consistent data

[dalg24]

Is this PR still blocked by anything now that Trilinos has had a few releases (17+) that are based on Kokkos 5?

iirc it had a performance regression due to Kokkos iterators, but it was flaky, and I don't have a solid consistent data

Was that not resolved in Kokkos 4.6?
https://github.com/kokkos/kokkos/blob/dfa6599eb2d3277815cc2f406cc56cf66e02f85c/CHANGELOG.md?plain=1#L474

[aprokop]

Was that not resolved in Kokkos 4.6?

From my recollection, no. I think the problem was passing iterators to thrust sort. Going back through my communication, I see

Just FYI, Kokkos sort_by_key is slower than native Thrust
number of points          : 1000000000
 === unsigned [CUDA] ===
Time [Kokkos]         :   0.054
Time [Thrust]         :   0.039
 === int [CUDA] ===
Time [Kokkos]         :   0.060
Time [Thrust]         :   0.039
 === long long [CUDA] ===
Time [Kokkos]         :   0.108
Time [Thrust]         :   0.083
 === float [CUDA] ===
Time [Kokkos]         :   0.054
Time [Thrust]         :   0.040
 === double [CUDA] ===
Time [Kokkos]         :   0.109
Time [Thrust]         :   0.119

This is purely due to using iterators. Doing a quick patch in Kokkos (I know it won’t work for all cases):

  // thrust::sort_by_key(policy, keys_first, keys_last, values_first,
                      // std::forward<MaybeComparator>(maybeComparator)...);
  thrust::sort_by_key(policy, keys.data(), keys.data() + keys.size(), values.data(),
                      std::forward<MaybeComparator>(maybeComparator)...);
fixes it:
number of points          : 1000000000
 === unsigned [CUDA] ===
Time [Kokkos]         :   0.040
Time [Thrust]         :   0.039
 === int [CUDA] ===
Time [Kokkos]         :   0.043
Time [Thrust]         :   0.040
 === long long [CUDA] ===
Time [Kokkos]         :   0.087
Time [Thrust]         :   0.082
 === float [CUDA] ===
Time [Kokkos]         :   0.046
Time [Thrust]         :   0.040
 === double [CUDA] ===
Time [Kokkos]         :   0.090
Time [Thrust]         :   0.092

I am not sure why double timings are shorter, but at least they are consistent.

But I also recall having inconsistent numbers on the machine I ran it on.

[dalg24]

Would you be willing to contribute a fix upstream in Kokkos Core?

[aprokop]

Would you be willing to contribute a fix upstream in Kokkos Core?

Yes. I will need to find some time for it, though.

[aprokop]

This PR is on hold until we require Kokkos 5.3.