Return weak orderings from device_row_comparator.

[rwlee]

This PR changes the experimental device_row_comparator to return weak_ordering instead of bool.

Originally part of PR #9452. Aids PR #10730, which builds strongly-typed two table comparators and should return a weak_ordering.

[codecov]

Codecov Report

Merging #10793 (08092fe) into branch-22.06 (8d861ce) will decrease coverage by 0.10%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           branch-22.06   #10793      +/-   ##
================================================
- Coverage         86.40%   86.29%   -0.11%     
================================================
  Files               143      143              
  Lines             22448    22597     +149     
================================================
+ Hits              19396    19501     +105     
- Misses             3052     3096      +44     

Impacted Files	Coverage Δ
...thon/dask_cudf/dask_cudf/tests/test_distributed.py	18.86% <0.00%> (-67.93%)	⬇️
python/cudf/cudf/io/parquet.py	90.47% <0.00%> (-2.23%)	⬇️
python/cudf/cudf/core/index.py	92.06% <0.00%> (-0.25%)	⬇️
python/cudf/cudf/core/_compat.py	100.00% <0.00%> (ø)
python/cudf/cudf/core/indexed_frame.py	91.70% <0.00%> (ø)
python/cudf/cudf/core/series.py	95.17% <0.00%> (+<0.01%)	⬆️
python/cudf/cudf/testing/_utils.py	94.05% <0.00%> (+0.06%)	⬆️
python/cudf/cudf/core/dataframe.py	93.77% <0.00%> (+0.08%)	⬆️
python/cudf/cudf/core/column/string.py	89.21% <0.00%> (+0.12%)	⬆️
python/cudf/cudf/core/groupby/groupby.py	91.79% <0.00%> (+0.22%)	⬆️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3a39b3...08092fe. Read the comment docs.

[jrhemstad]

I don't know where or when this precisely needs to be done, but I'd like some words written about the equality comparator vs this comparator with weak_ordering::equivalent. The former should always be preferred in operations where you're only concerned with equality (like groupby/join/etc). This is because it will be more efficient than using this comparator with the weak_ordering::equivalent which would require doing two operator< vs a single operator==, which is important for something like strings.

[jrhemstad]

I don't know where or when this precisely needs to be done, but I'd like some words written about the equality comparator vs this comparator with weak_ordering::equivalent. The former should always be preferred in operations where you're only concerned with equality (like groupby/join/etc). This is because it will be more efficient than using this comparator with the weak_ordering::equivalent which would require doing two operator< vs a single operator==, which is important for something like strings.

In other words, I don't want us to ever define a:

using equivalent_comparator = weak_ordering_comparator_impl<Comparator, weak_ordering::equivalent>;

[bdice]

This is surprisingly clean with the use of a parameter pack fold expression. Awesome suggestion @jrhemstad, and nice work implementing it @rwlee.

@rwlee showed me benchmarks offline (which I think he is going to paste here soon) and these changes didn't seem to have any negative performance impact.

With the documentation suggestion below, I approve.

[rwlee]

@rwlee showed me benchmarks offline (which I think he is going to paste here soon) and these changes didn't seem to have any negative performance impact.

Benchmarks were posted above #10793 (comment) -- linking down here so they can be found after the review comment it is resolved.

[bdice]

@rwlee I edited the PR title and description to make it clearer what this is doing -- see these previous comments:

• Add struct utility functions. #10776 (comment)
• Use column_views instead of column_device_views in binary operations. #10780 (comment)

[ttnghia]

It would be useful to have aliases for the remaining comparator too (i.e., > and >=).

[jrhemstad]

YAGNI

[ttnghia]

Then how about less_equivalent_comparator? We are not using it now, right?

[jrhemstad]

#9452 uses less_equivalent_comparator.

[ttnghia]

Suggested change

	namespace cudf {
	namespace experimental {
	namespace cudf::experimental {

[bdice]

This file has several layers of nested namespaces. For consistency, I would recommend leaving this unchanged.

[bdice]

I discussed this PR and #9452 with @rwlee offline. I'm merging this to unblock the next steps for #10730 and #9452.

[bdice]

@gpucibot merge