Skip to content

Commit

Permalink
Default to equal NaNs in make_merge_sets_aggregation. (#11952)
Browse files Browse the repository at this point in the history
Partially resolves #11329. This helps to align our default behaviors for null and NaN equality across APIs, specifically for `make_merge_sets_aggregation` in this PR. All functions should default to treating null values as equal to one another and NaN values as equal to one another.

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - David Wendt (https://github.com/davidwendt)

URL: #11952
  • Loading branch information
bdice authored Oct 21, 2022
1 parent f1ab5e9 commit 5c2150e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
5 changes: 3 additions & 2 deletions cpp/include/cudf/aggregation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,9 @@ std::unique_ptr<Base> make_merge_lists_aggregation();
* @return A MERGE_SETS aggregation object
*/
template <typename Base = aggregation>
std::unique_ptr<Base> make_merge_sets_aggregation(null_equality nulls_equal = null_equality::EQUAL,
nan_equality nans_equal = nan_equality::UNEQUAL);
std::unique_ptr<Base> make_merge_sets_aggregation(
null_equality nulls_equal = null_equality::EQUAL,
nan_equality nans_equal = nan_equality::ALL_EQUAL);

/**
* @brief Factory to create a MERGE_M2 aggregation
Expand Down
9 changes: 6 additions & 3 deletions cpp/tests/reductions/collect_ops_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,17 @@ TEST_F(CollectTest, MergeSetsWithNaN)

// nan unequal with null equal
fp_wrapper expected1{{-2.3e-5f, 1.0f, 2.3e5f, -NAN, NAN, NAN, 0.0f}, {1, 1, 1, 1, 1, 1, 0}};
auto const ret1 = collect_set(col, make_merge_sets_aggregation<reduce_aggregation>());
auto const ret1 = collect_set(
col,
make_merge_sets_aggregation<reduce_aggregation>(null_equality::EQUAL, nan_equality::UNEQUAL));
CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected1, dynamic_cast<list_scalar*>(ret1.get())->view());

// nan unequal with null unequal
fp_wrapper expected2{{-2.3e-5f, 1.0f, 2.3e5f, -NAN, NAN, NAN, 0.0f, 0.0f, 0.0f},
{1, 1, 1, 1, 1, 1, 0, 0, 0}};
auto const ret2 =
collect_set(col, make_merge_sets_aggregation<reduce_aggregation>(null_equality::UNEQUAL));
auto const ret2 = collect_set(
col,
make_merge_sets_aggregation<reduce_aggregation>(null_equality::UNEQUAL, nan_equality::UNEQUAL));
CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected2, dynamic_cast<list_scalar*>(ret2.get())->view());

// nan equal with null equal
Expand Down

0 comments on commit 5c2150e

Please sign in to comment.