Update and deprecate is_space::host_memory/execution/mirror_space #3973
Conversation
|
This PR does not pass because we are still using |
| #elif defined(KOKKOS_ENABLE_HIP) | ||
| || std::is_same<memory_space, | ||
| Kokkos::Experimental::HIPHostPinnedSpace>::value | ||
| #elif defined(KOKKOS_ENABLE_SYCL) | ||
| || std::is_same<memory_space, | ||
| Kokkos::Experimental::SYCLSharedUSMSpace>::value |
There was a problem hiding this comment.
This is OK but you must state clearly your fixing a defect in the description of this PR
There was a problem hiding this comment.
On second thought, why do you think we should fix if we are deprecating anyway?
There was a problem hiding this comment.
I prefer rather to not break users' code even if they are using deprecated functionality (especially if they turn off deprecation warnings).
There was a problem hiding this comment.
You are not breaking code. It is already broken.
There was a problem hiding this comment.
Sure, but people are unlikely to use the new backends (in particular, SYCL and OpenMPtarget already. So if they use the deprecated versions in their code and want to use, say SYCL, with a new release their code might fail unexpectedly.
In the end, I just don't say much of a reason not to do our best here. I also tried to just forward to the replacement but that is a header dependency nightmare.
|
Please say in the description what should be used instead. |
is_space remove from Impl and host_mirror_space type deprecated See kokkos/kokkos#3973
is_space remove from Impl and host_mirror_space type deprecated See kokkos/kokkos#3973
Cherry-picked from #3838. In particular, provide correct definitions for
SYCL,HIPandOpenMPTarget. Deprecated forHostMirror.