Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove is_relationally_comparable for table device views #10342

Merged
Merged
7 changes: 3 additions & 4 deletions cpp/include/cudf/table/row_operators.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -355,12 +355,13 @@ class row_lexicographic_comparator {
* @brief Construct a function object for performing a lexicographic
* comparison between the rows of two tables.
*
* The caller should ensure the corresponding column types are comparable.
davidwendt marked this conversation as resolved.
Show resolved Hide resolved
*
* @throws cudf::logic_error if `lhs.num_columns() != rhs.num_columns()`
* @throws cudf::logic_error if column types of `lhs` and `rhs` are not comparable.
*
* @param has_nulls Indicates if either input table contains columns with nulls.
* @param lhs The first table
* @param rhs The second table (may be the same table as `lhs`)
* @param has_nulls Indicates if either input table contains columns with nulls.
* @param column_order Optional, device array the same length as a row that
* indicates the desired ascending/descending order of each column in a row.
* If `nullptr`, it is assumed all columns are sorted in ascending order.
Expand All @@ -381,8 +382,6 @@ class row_lexicographic_comparator {
_null_precedence{null_precedence}
{
CUDF_EXPECTS(_lhs.num_columns() == _rhs.num_columns(), "Mismatched number of columns.");
CUDF_EXPECTS(detail::is_relationally_comparable(_lhs, _rhs),
"Attempted to compare elements of uncomparable types.");
}

/**
Expand Down
8 changes: 1 addition & 7 deletions cpp/include/cudf/table/table_device_view.cuh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -150,10 +150,4 @@ auto contiguous_copy_column_device_views(HostTableView source_view, rmm::cuda_st
return std::make_tuple(std::move(descendant_storage), d_columns);
}

namespace detail {
extern template bool is_relationally_comparable<table_device_view>(table_device_view const& lhs,
table_device_view const& rhs);
extern template bool is_relationally_comparable<mutable_table_device_view>(
mutable_table_device_view const& lhs, mutable_table_device_view const& rhs);
} // namespace detail
} // namespace cudf
45 changes: 2 additions & 43 deletions cpp/src/table/table_device_view.cu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,8 +20,7 @@
#include <cudf/utilities/error.hpp>

#include <rmm/cuda_stream_view.hpp>

#include <thrust/logical.h>
#include <rmm/exec_policy.hpp>
davidwendt marked this conversation as resolved.
Show resolved Hide resolved

namespace cudf {
namespace detail {
Expand Down Expand Up @@ -54,45 +53,5 @@ template class table_device_view_base<column_device_view, table_view>;
// Explicit instantiation for a device table of mutable views
template class table_device_view_base<mutable_column_device_view, mutable_table_view>;

namespace {
struct is_relationally_comparable_functor {
template <typename T>
constexpr bool operator()()
{
return cudf::is_relationally_comparable<T, T>();
}
};
} // namespace

template <typename TableView>
bool is_relationally_comparable(TableView const& lhs, TableView const& rhs)
{
return thrust::all_of(thrust::counting_iterator<size_type>(0),
thrust::counting_iterator<size_type>(lhs.num_columns()),
[lhs, rhs] __device__(auto const i) {
// Simplified this for compile time. (Ideally use double_type_dispatcher)
// TODO: possible to implement without double type dispatcher.
return lhs.column(i).type() == rhs.column(i).type() and
type_dispatcher(lhs.column(i).type(),
is_relationally_comparable_functor{});
});
}

// Explicit extern template instantiation for a table of immutable views
extern template bool is_relationally_comparable<table_view>(table_view const& lhs,
table_view const& rhs);

// Explicit extern template instantiation for a table of mutable views
extern template bool is_relationally_comparable<mutable_table_view>(mutable_table_view const& lhs,
mutable_table_view const& rhs);

// Explicit extern template instantiation for a device table of immutable views
template bool is_relationally_comparable<table_device_view>(table_device_view const& lhs,
table_device_view const& rhs);

// Explicit extern template instantiation for a device table of mutable views
template bool is_relationally_comparable<mutable_table_device_view>(
mutable_table_device_view const& lhs, mutable_table_device_view const& rhs);

} // namespace detail
} // namespace cudf
34 changes: 33 additions & 1 deletion cpp/src/table/table_view.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018-2019, NVIDIA CORPORATION.
* Copyright (c) 2018-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -96,4 +96,36 @@ table_view scatter_columns(table_view const& source,
return table_view{updated_columns};
}

namespace detail {
namespace {
struct is_relationally_comparable_functor {
template <typename T>
constexpr bool operator()()
{
return cudf::is_relationally_comparable<T, T>();
}
};
} // namespace

template <typename TableView>
bool is_relationally_comparable(TableView const& lhs, TableView const& rhs)
{
return std::all_of(thrust::counting_iterator<size_type>(0),
thrust::counting_iterator<size_type>(lhs.num_columns()),
[lhs, rhs](auto const i) {
return lhs.column(i).type() == rhs.column(i).type() and
type_dispatcher(lhs.column(i).type(),
is_relationally_comparable_functor{});
});
}

// Explicit extern template instantiation for a table of immutable views
extern template bool is_relationally_comparable<table_view>(table_view const& lhs,
table_view const& rhs);

// Explicit extern template instantiation for a table of mutable views
extern template bool is_relationally_comparable<mutable_table_view>(mutable_table_view const& lhs,
Copy link
Contributor

@ttnghia ttnghia Mar 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extern should not be used here. As I found https://stackoverflow.com/questions/8130602/using-extern-template-c11):

You should only use extern template to force the compiler to not instantiate a template when you know that it will be instantiated somewhere else. It is used to reduce compile time and object file size.

mutable_table_view const& rhs);

} // namespace detail
} // namespace cudf