Skip to content

Commit

Permalink
Fix scatter output size for structs. (#8155)
Browse files Browse the repository at this point in the history
Fixes #8150, which causes CUDF failures if the `scatter()` source struct column has a different size from that of the target. It turns out that the implementation was erroneously using the source column's size to construct the result, instead of the target's.

The following changes were made in this commit:

1. Scatter output's size was set to target's size instead of source.
2. Test case for the above.
3. Peripherally, `scatter_lists_test` and `scatter_struct_test` were moved from `.cu` to `.cpp`, for faster compile.

Authors:
  - MithunR (https://github.com/mythrocks)

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

URL: #8155
  • Loading branch information
mythrocks authored May 4, 2021
1 parent 53e1c66 commit a80dff2
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 5 deletions.
2 changes: 1 addition & 1 deletion cpp/include/cudf/detail/scatter.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ struct column_scatterer_impl<struct_view> {

// Need to put the result column in a vector to call `gather_bitmask`
std::vector<std::unique_ptr<column>> result;
result.emplace_back(cudf::make_structs_column(source.size(),
result.emplace_back(cudf::make_structs_column(target.size(),
std::move(output_struct_members),
0,
rmm::device_buffer{0, stream, mr},
Expand Down
4 changes: 2 additions & 2 deletions cpp/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ ConfigureTest(COPYING_TEST
copying/pack_tests.cu
copying/sample_tests.cpp
copying/scatter_tests.cpp
copying/scatter_list_tests.cu
copying/scatter_struct_tests.cu
copying/scatter_list_tests.cpp
copying/scatter_struct_tests.cpp
copying/segmented_gather_list_tests.cpp
copying/shift_tests.cpp
copying/slice_tests.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

#include <cudf/column/column_view.hpp>
#include <cudf/copying.hpp>
#include <cudf/detail/iterator.cuh>
#include <cudf/detail/scatter.cuh>
#include <cudf/lists/lists_column_view.hpp>
#include <cudf/table/table.hpp>
#include <cudf/table/table_view.hpp>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,3 +291,24 @@ TYPED_TEST(TypedStructScatterTest, ScatterStructOfListsTest)
auto const scatter_map = int32s_col{-3, -2, -1, 5, 4, 3, 2}.release();
test_scatter(structs_src, structs_tgt, structs_expected, scatter_map);
}

TYPED_TEST(TypedStructScatterTest, ScatterSourceSmallerThanTarget)
{
using namespace cudf;
using namespace cudf::test;

using fixed_width = fixed_width_column_wrapper<TypeParam, int32_t>;
using structs_col = structs_column_wrapper;
using scatter_map_col = fixed_width_column_wrapper<offset_type, int32_t>;

auto source_child = fixed_width{22, 55};
auto target_child = fixed_width{0, 1, 2, 3, 4, 5, 6};
auto expected_child = fixed_width{0, 1, 22, 3, 4, 55, 6};

auto const source = structs_col{{source_child}}.release();
auto const target = structs_col{{target_child}}.release();
auto const scatter_map = scatter_map_col{2, 5}.release();
auto const expected = structs_col{{expected_child}}.release();

test_scatter(source, target, expected, scatter_map);
}

0 comments on commit a80dff2

Please sign in to comment.