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

Fix thrust failure when transfering data from device_vector to host_vector with vectors of size 1 #7382

Merged
merged 9 commits into from
Feb 17, 2021
15 changes: 15 additions & 0 deletions cpp/include/cudf_test/column_utilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,21 @@ inline std::pair<thrust::host_vector<std::string>, std::vector<bitmask_type>> to
return {host_data, bitmask_to_host(c)};
}

/**
* @brief Print the different values between two columns
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
*
* @param differences contains indices of the different values
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
* @param lhs and rhs are the two input columns
* @param all_differences set to true will print all the different values and false will print only
* the first value
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
* @param depth the depth level of column data
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
*/
std::string differences_message(thrust::device_vector<int> const& differences,
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
column_view const& lhs,
column_view const& rhs,
bool all_differences,
int depth);

} // namespace test
} // namespace cudf

Expand Down
6 changes: 3 additions & 3 deletions cpp/include/cudf_test/column_wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ struct fixed_width_type_converter {
template <typename FromT = From,
typename ToT = To,
typename std::enable_if<std::is_same<FromT, ToT>::value, void>::type* = nullptr>
ToT operator()(FromT element) const
constexpr ToT operator()(FromT element) const
{
return element;
}
Expand All @@ -106,7 +106,7 @@ struct fixed_width_type_converter {
(cudf::is_convertible<FromT, ToT>::value ||
std::is_constructible<ToT, FromT>::value),
void>::type* = nullptr>
ToT operator()(FromT element) const
constexpr ToT operator()(FromT element) const
{
return static_cast<ToT>(element);
}
Expand All @@ -117,7 +117,7 @@ struct fixed_width_type_converter {
typename ToT = To,
typename std::enable_if<std::is_integral<FromT>::value && cudf::is_timestamp_t<ToT>::value,
void>::type* = nullptr>
ToT operator()(FromT element) const
constexpr ToT operator()(FromT element) const
{
return ToT{typename ToT::duration{element}};
}
Expand Down
17 changes: 17 additions & 0 deletions cpp/tests/reshape/byte_cast_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,23 @@ using namespace cudf::test;
class ByteCastTest : public cudf::test::BaseFixture {
};

// Previously, there was a bug that causes thrust to crash
// This test is to make sure that bug does not pop up again
// If this test fails, the program will crash
TEST_F(ByteCastTest, thurstTest)
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
{
cudf::test::fixed_width_column_wrapper<int> a{0, 1, 2, 3};
cudf::test::fixed_width_column_wrapper<int> b{0, 1, 2, 4};
cudf::test::fixed_width_column_wrapper<int> c{10, 11, 12, 13};

thrust::device_vector<int> diff_ab = std::vector<int>{3};
thrust::device_vector<int> diff_ac = std::vector<int>{0, 1, 2, 3};

// Prevent the compiler to optimize away the calls to differences_message
volatile auto str1 = cudf::test::differences_message(diff_ab, a, b, true, 0);
volatile auto str2 = cudf::test::differences_message(diff_ac, a, c, true, 0);
}

TEST_F(ByteCastTest, int16ValuesWithSplit)
{
using limits = std::numeric_limits<int16_t>;
Expand Down
78 changes: 39 additions & 39 deletions cpp/tests/utilities/column_utilities.cu
Original file line number Diff line number Diff line change
Expand Up @@ -160,45 +160,6 @@ class corresponding_rows_not_equivalent {
}
};

std::string differences_message(thrust::device_vector<int> const& differences,
column_view const& lhs,
column_view const& rhs,
bool all_differences,
int depth)
{
CUDF_EXPECTS(not differences.empty(), "Shouldn't enter this function if `differences` is empty");

std::string const depth_str = depth > 0 ? "depth " + std::to_string(depth) + '\n' : "";

if (all_differences) {
std::ostringstream buffer;
buffer << depth_str << "differences:" << std::endl;

auto source_table = cudf::table_view({lhs, rhs});
auto diff_column = fixed_width_column_wrapper<int32_t>(differences.begin(), differences.end());
auto diff_table = cudf::gather(source_table, diff_column);

// Need to pull back the differences
auto const h_left_strings = to_strings(diff_table->get_column(0));
auto const h_right_strings = to_strings(diff_table->get_column(1));

for (size_t i = 0; i < differences.size(); ++i)
buffer << depth_str << "lhs[" << differences[i] << "] = " << h_left_strings[i] << ", rhs["
<< differences[i] << "] = " << h_right_strings[i] << std::endl;

return buffer.str();
} else {
int index = differences[0]; // only stringify first difference

auto diff_lhs = cudf::detail::slice(lhs, index, index + 1);
auto diff_rhs = cudf::detail::slice(rhs, index, index + 1);

return depth_str + "first difference: " + "lhs[" + std::to_string(index) +
"] = " + to_string(diff_lhs, "") + ", rhs[" + std::to_string(index) +
"] = " + to_string(diff_rhs, "");
}
}

// non-nested column types
template <typename T, bool check_exact_equality>
struct column_comparator_impl {
Expand Down Expand Up @@ -373,6 +334,45 @@ struct column_comparator {

} // namespace

/**
* @copydoc cudf::test::differences_message
*/
std::string differences_message(thrust::device_vector<int> const& differences,
column_view const& lhs,
column_view const& rhs,
bool all_differences,
int depth)
{
CUDF_EXPECTS(not differences.empty(), "Shouldn't enter this function if `differences` is empty");
std::string const depth_str = depth > 0 ? "depth " + std::to_string(depth) + '\n' : "";
if (all_differences) {
std::ostringstream buffer;
buffer << depth_str << "differences:" << std::endl;

// thrust may crash if a device_vector is passed to fixed_width_column_wrapper,
// thus we construct fixed_width_column_wrapper from a host_vector instead
thrust::host_vector<int> h_differences(differences);
auto source_table = cudf::table_view({lhs, rhs});
auto diff_column =
fixed_width_column_wrapper<int32_t>(h_differences.begin(), h_differences.end());
auto diff_table = cudf::gather(source_table, diff_column);
// Need to pull back the differences
auto const h_left_strings = to_strings(diff_table->get_column(0));
auto const h_right_strings = to_strings(diff_table->get_column(1));
for (size_t i = 0; i < h_differences.size(); ++i)
buffer << depth_str << "lhs[" << h_differences[i] << "] = " << h_left_strings[i] << ", rhs["
<< h_differences[i] << "] = " << h_right_strings[i] << std::endl;
return buffer.str();
} else {
int index = differences[0]; // only stringify first difference
auto diff_lhs = cudf::detail::slice(lhs, index, index + 1);
auto diff_rhs = cudf::detail::slice(rhs, index, index + 1);
return depth_str + "first difference: " + "lhs[" + std::to_string(index) +
"] = " + to_string(diff_lhs, "") + ", rhs[" + std::to_string(index) +
"] = " + to_string(diff_rhs, "");
}
}

/**
* @copydoc cudf::test::expect_column_properties_equal
*/
Expand Down