sorting an empty view should exit early and not fail

[fnrizzi]

sort

The current impl of sort does not handle nor test the special case of an empty view.
Passing an empty view to sort should not fail.
Currently, the code fails because if we look here:

kokkos/algorithms/src/Kokkos_Sort.hpp

Lines 607 to 640 in 3f602b6

	sort(const ExecutionSpace& exec,
	const Kokkos::View<DataType, Properties...>& view) {
	using ViewType = Kokkos::View<DataType, Properties...>;
	using CompType = BinOp1D<ViewType>;
	Kokkos::MinMaxScalar<typename ViewType::non_const_value_type> result;
	Kokkos::MinMax<typename ViewType::non_const_value_type> reducer(result);
	parallel_reduce("Kokkos::Sort::FindExtent",
	Kokkos::RangePolicy<typename ViewType::execution_space>(
	exec, 0, view.extent(0)),
	Impl::min_max_functor<ViewType>(view), reducer);
	if (result.min_val == result.max_val) return;
	// For integral types the number of bins may be larger than the range
	// in which case we can exactly have one unique value per bin
	// and then don't need to sort bins.
	bool sort_in_bins = true;
	// TODO: figure out better max_bins then this ...
	int64_t max_bins = view.extent(0) / 2;
	if (std::is_integral<typename ViewType::non_const_value_type>::value) {
	// Cast to double to avoid possible overflow when using integer
	auto const max_val = static_cast<double>(result.max_val);
	auto const min_val = static_cast<double>(result.min_val);
	// using 10M as the cutoff for special behavior (roughly 40MB for the count
	// array)
	if ((max_val - min_val) < 10000000) {
	max_bins = max_val - min_val + 1;
	sort_in_bins = false;
	}
	}
	if (std::is_floating_point<typename ViewType::non_const_value_type>::value) {
	KOKKOS_ASSERT(std::isfinite(static_cast<double>(result.max_val) -
	static_cast<double>(result.min_val)));
	}

when view is empty, the reduction does not happen and so result contains the reduction_identity values.
Since those values are not equal, the following if is not satisfied:

if (result.min_val == result.max_val) return;

and the code fails because it hits this:

if (std::is_floating_point<typename ViewType::non_const_value_type>::value) {
    KOKKOS_ASSERT(std::isfinite(static_cast<double>(result.max_val) -
                                static_cast<double>(result.min_val)));
  }

BinSort

For BinSort there is a similar problem here:

kokkos/algorithms/src/Kokkos_Sort.hpp

Lines 327 to 331 in 3f602b6

	// Sort a subset of a view with respect to the first dimension using the
	// permutation array
	template <class ExecutionSpace, class ValuesViewType>
	void sort(const ExecutionSpace& exec, ValuesViewType const& values,
	int values_range_begin, int values_range_end) const {

Comments

Note that an empty view is not a precondition violation.
Also, the std lib behaves similarly: if we try to call std::sort on an empty vector the code does not crash.

When merging, need to be careful with #6081

[masterleinad]

What about BinSort::sort?

[fnrizzi]

What about BinSort::sort?

yes that too, i am working on it... have not pushed yet. I was unsure whether to make a separate PR just for that. I will jsut add it here

[dalg24]

What did you change?

[fnrizzi]

What did you change?

change what? I only rebased on develop

[dalg24]

What did you change?

change what? I only rebased on develop

Why bother? Anyway too late. I plan on merging after the CI completes

[fnrizzi]

What did you change?

change what? I only rebased on develop

Why bother? Anyway too late. I plan on merging after the CI completes

habit of keeping things updated so that no surprises come up after merging (maybe in this case does not matter, but in general does)

[dalg24]

So you know we test your PR merged into develop unless there is conflict in which we can't merge anyway.

[fnrizzi]

So you know we test your PR merged into develop unless there is conflict in which we can't merge anyway.

Now I know :) thanks!