allow sorting via native oneDPL to support views with stride = 1

[fnrizzi]

Before this PR, the call to native sort in oneDPL was guarded with static asserts for views to have layoutRight and layoutLeft.
Since technically oneDPL can support stride = 1, this PR also allows for LayoutStride but adds a runtime check that stride ==1.

[crtrott]

We don't have a is_always_strided property like mdspan does, but basically that is what you need to check for here. LayoutTiled for example breaks this.

[crtrott]

This ignored LayoutTiled, and would compile with it

[fnrizzi]

ops, yes totally right, i actually had it that way in the first place and then simplified too much.
Thanks for catching it !

[nliber]

I agree with @crtrott. The old static_assert needs to be there.

[masterleinad]

We also need to match the checks in sort_onedpl above

          rank == 1 && 
          (std::is_same_v<typename ViewType::array_layout, LayoutRight> ||
           std::is_same_v<typename ViewType::array_layout, LayoutLeft> ||
           std::is_same_v<typename ViewType::array_layout, LayoutStride>),

here so we can fallback to copy_to_host_run_stdsort_copy_back in case of a different layout without running into the Kokkos::abort or static_assert.

[fnrizzi]

ok, so i changed this because it makes sense for this specific function. However, this is within the impl namespace and as far as usability, we have a static assert in the public API that alreay statically checks only for these layouts. so one cannot pass a view with a different layout anyway. makes sense?

[masterleinad]

we have a static assert in the public API that alreay statically checks only for these layouts.

That's not true for the overloads without comparator, see

kokkos/algorithms/src/sorting/Kokkos_SortPublicAPI.hpp

Lines 31 to 72 in d89140d

	template <class ExecutionSpace, class DataType, class... Properties>
	void sort([[maybe_unused]] const ExecutionSpace& exec,
	const Kokkos::View<DataType, Properties...>& view) {
	// constraints
	using ViewType = Kokkos::View<DataType, Properties...>;
	using MemSpace = typename ViewType::memory_space;
	static_assert(ViewType::rank == 1,
	"Kokkos::sort: currently only supports rank-1 Views.");
	static_assert(SpaceAccessibility<ExecutionSpace, MemSpace>::accessible,
	"Kokkos::sort: execution space instance is not able to access "
	"the memory space of the "
	"View argument!");
	if (view.extent(0) <= 1) {
	return;
	}
	if constexpr (Impl::better_off_calling_std_sort_v<ExecutionSpace>) {
	auto first = ::Kokkos::Experimental::begin(view);
	auto last = ::Kokkos::Experimental::end(view);
	std::sort(first, last);
	} else {
	Impl::sort_device_view_without_comparator(exec, view);
	}
	}
	template <class DataType, class... Properties>
	void sort(const Kokkos::View<DataType, Properties...>& view) {
	using ViewType = Kokkos::View<DataType, Properties...>;
	static_assert(ViewType::rank == 1,
	"Kokkos::sort: currently only supports rank-1 Views.");
	Kokkos::fence("Kokkos::sort: before");
	if (view.extent(0) <= 1) {
	return;
	}
	typename ViewType::execution_space exec;
	sort(exec, view);
	exec.fence("Kokkos::sort: fence after sorting");
	}

, but the ones with comparator have them

kokkos/algorithms/src/sorting/Kokkos_SortPublicAPI.hpp

Lines 77 to 131 in d89140d

	template <class ExecutionSpace, class ComparatorType, class DataType,
	class... Properties>
	void sort([[maybe_unused]] const ExecutionSpace& exec,
	const Kokkos::View<DataType, Properties...>& view,
	const ComparatorType& comparator) {
	// constraints
	using ViewType = Kokkos::View<DataType, Properties...>;
	using MemSpace = typename ViewType::memory_space;
	static_assert(
	ViewType::rank == 1 &&
	(std::is_same_v<typename ViewType::array_layout, LayoutRight> ||
	std::is_same_v<typename ViewType::array_layout, LayoutLeft> ||
	std::is_same_v<typename ViewType::array_layout, LayoutStride>),
	"Kokkos::sort with comparator: supports 1D Views with LayoutRight, "
	"LayoutLeft or LayoutStride.");
	static_assert(SpaceAccessibility<ExecutionSpace, MemSpace>::accessible,
	"Kokkos::sort: execution space instance is not able to access "
	"the memory space of the View argument!");
	if (view.extent(0) <= 1) {
	return;
	}
	if constexpr (Impl::better_off_calling_std_sort_v<ExecutionSpace>) {
	auto first = ::Kokkos::Experimental::begin(view);
	auto last = ::Kokkos::Experimental::end(view);
	std::sort(first, last, comparator);
	} else {
	Impl::sort_device_view_with_comparator(exec, view, comparator);
	}
	}
	template <class ComparatorType, class DataType, class... Properties>
	void sort(const Kokkos::View<DataType, Properties...>& view,
	const ComparatorType& comparator) {
	using ViewType = Kokkos::View<DataType, Properties...>;
	static_assert(
	ViewType::rank == 1 &&
	(std::is_same_v<typename ViewType::array_layout, LayoutRight> ||
	std::is_same_v<typename ViewType::array_layout, LayoutLeft> ||
	std::is_same_v<typename ViewType::array_layout, LayoutStride>),
	"Kokkos::sort with comparator: supports 1D Views with LayoutRight, "
	"LayoutLeft or LayoutStride.");
	Kokkos::fence("Kokkos::sort with comparator: before");
	if (view.extent(0) <= 1) {
	return;
	}
	typename ViewType::execution_space exec;
	sort(exec, view, comparator);
	exec.fence("Kokkos::sort with comparator: fence after sorting");
	}

.

[fnrizzi]

yes but the code you are requesting changes in the impl is inside sort_device_view_with_comparator so that is only called by the piublic API if a comparator is present

[masterleinad]

Then we need this check when calling the version without comparator (since it's also hitting sort_onedpl), right?

[fnrizzi]

yes you are right. I did not want touch the existing code, yes, but makes sense to add the constraints as much as possible into the public API rather than having them hidden. So i am changing this to also add these constraints to the public API of the sort without comparator.