Document issue with Kokkos::sort and comparators #504
Conversation
| that are forwarded to the TPL satisfy the sycl::is_device_copyable trait to avoid compiler errors. This holds true in particular | ||
| for comparators used with Kokkos::sort in Kokkos versions prior to 4.3. The best advice to give is to make sure the respective | ||
| parameters are trivially-copyable. If this isn't possible, sycl::is_device_copyable should be specialized and users should make | ||
| sure to use raw pointers instead of Kokkos::Views. |
There was a problem hiding this comment.
Can you add a code example and include the compile error so that it can be easily found by users?
There was a problem hiding this comment.
Kokkos::sort(exec, values, KOKKOS_LAMBDA(int i, int j) { return keys(i) < keys(j); }); // error blah with SYCL
docs/source/known-issues.rst
Outdated
| SYCL | ||
| ==== | ||
|
|
||
| - Multiple of the Kokkos algorithm functions use third-party libraries like oneDPL. |
There was a problem hiding this comment.
"Multiple of" sounds weird to me.
Maybe "several" or "a number of"? Maybe a native English speaker can advise here.
| /usr/bin/compiler/../../include/sycl/handler.hpp:2332:5: note: (skipping 7 contexts in backtrace; use -ftemplate-backtrace-limit=0 to see all) | ||
| 2332 | parallel_for_lambda_impl<KernelName, KernelType, 1, PropertiesT>( | ||
| | ^ | ||
| [...] |
There was a problem hiding this comment.
Please add a Functor definition followed by a sycl::is_device_copyable traits specialization to showcase what the user is expected to do.
There was a problem hiding this comment.
I meant also adding a definition for MyComparator and showing the actual Kokkos::sort invocation.
Do you think it is too obvious?
There was a problem hiding this comment.
That commit changes the example into
MyComparator my_comparator;
Kokkos::sort(exec, values, my_comparator);I find this already to be pretty verbose and don't think we need to provide details on the potential implementation of MyComparator.
There was a problem hiding this comment.
Oh I thought you'd transform the lambda into that comparator thing.
I suppose that is good enough.
There was a problem hiding this comment.
The error you posted was with a lambda though right? (Not necessarily asking you to update)
There was a problem hiding this comment.
Yes, it was with a lambda but I printed only those errors that are independent of the example. The lambda referenced in it comes from oneDPL.
Related to kokkos/kokkos#6801 (comment).