Skip to content

[FEA] Support self comparisons in cudf::experimental::row::lexicographic::two_table_comparator #13371

@divyegala

Description

@divyegala

As showcased in #13347, there's a need for {lhs, lhs} and {rhs, rhs} comparisons in an instance of two_table_comparator.

This can't simply be achieved by adding more overloads because left and right terminology is baked into the comparator when it's constructed at the host-side. In a device function, the strongly typed indices now work with the assumption that a comp(i, j) that is called in a device function operates on {lhs, rhs} or {rhs, lhs}.

We need to settle on a design that lets us refactor the row operators such that the assumption of working on two different tables can be removed.

Do we strongly type device_row_comparator::operator() over here such that we can decide which columns of which tables to pass along to the element_comparator over here?

I see the design looking something like this:

class device_row_comparator {
  class element_comparator {
    operator() (size_type lhs_index, size_type rhs_index);
  };
  dispatch_element_operator(lhs_col, rhs_col, lhs_index, rhs_index);

  // these call dispatch_element_operator with the correct columns and indices
  operator() (lhs_index_type lhs_index, rhs_index_type rhs_index);
  operator() (lhs_index_type lhs_index, lhs_index_type rhs_index);
  operator() (rhs_index_type lhs_index, rhs_index_type rhs_index);
};

// the template `Comparator` here and below will be an instance of `device_row_comparator`,
// such that the strongly type indices can be passed along directly
template <typename Comparator, weak_ordering... values>
class single_table_ordering {
  operator() (size_type lhs_index, size_type rhs_index) {
    return comparator(lhs_index_type{lhs_index}, lhs_index_type{rhs_index});
};

template <typename Comparator, weak_ordering... values>
class two_table_ordering {
// same as current version of strong_index_comparator with added overloads for {lhs, lhs} and {rhs, rhs}
};

class self_comparator {
  auto less() {
    return less_comparator{single_table_ordering{device_row_comparator{...}}};
  }
};

class two_table_comparator {
  auto less() {
    return less_comparator{two_table_ordering{device_row_comparator{...}}};
  }
};

Note: In this example, weak_ordering_comparator_impl will be removed and it's functionality will instead be baked into single_table_ordering and two_table_ordering. less_comparator will then be reworked with CRTP such that:

template <typename Comparator>
class less_comparator : Comparator<weak_ordering::LESS>

Metadata

Metadata

Assignees

No one assigned

    Labels

    1 - On DeckTo be worked on nextfeature requestNew feature or requestlibcudfAffects libcudf (C++/CUDA) code.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions