Skip to content

Commit

Permalink
Remove the mr from the base fixture (#14057)
Browse files Browse the repository at this point in the history 
This mr is just an alias for the current memory resource, so we don't really need it. This came up in #14010 (comment). This PR removes all uses of it, but does not actually remove the mr yet. That will be done in a follow-up (see #14057 (comment)).

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #14057
  • Loading branch information
@vyasr
vyasr authored Sep 9, 2023
1 parent 36ee11a commit 886e189
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 140 deletions.
137 changes: 38 additions & 99 deletions cpp/tests/column/factories_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class ColumnFactoryTest : public cudf::test::BaseFixture {

public:
cudf::size_type size() { return _size; }
rmm::cuda_stream_view stream() { return cudf::get_default_stream(); }
};

template <typename T>
Expand All @@ -47,11 +46,8 @@ TYPED_TEST_SUITE(NumericFactoryTest, cudf::test::NumericTypes);

TYPED_TEST(NumericFactoryTest, EmptyNoMask)
{
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
0,
cudf::mask_state::UNALLOCATED,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, 0, cudf::mask_state::UNALLOCATED);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), 0);
EXPECT_EQ(0, column->null_count());
Expand All @@ -62,11 +58,8 @@ TYPED_TEST(NumericFactoryTest, EmptyNoMask)

TYPED_TEST(NumericFactoryTest, EmptyAllValidMask)
{
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
0,
cudf::mask_state::ALL_VALID,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, 0, cudf::mask_state::ALL_VALID);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), 0);
EXPECT_EQ(0, column->null_count());
Expand All @@ -77,11 +70,8 @@ TYPED_TEST(NumericFactoryTest, EmptyAllValidMask)

TYPED_TEST(NumericFactoryTest, EmptyAllNullMask)
{
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
0,
cudf::mask_state::ALL_NULL,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, 0, cudf::mask_state::ALL_NULL);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), 0);
EXPECT_EQ(0, column->null_count());
Expand All @@ -92,11 +82,8 @@ TYPED_TEST(NumericFactoryTest, EmptyAllNullMask)

TYPED_TEST(NumericFactoryTest, NoMask)
{
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
cudf::mask_state::UNALLOCATED,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), cudf::mask_state::UNALLOCATED);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(0, column->null_count());
Expand All @@ -107,11 +94,8 @@ TYPED_TEST(NumericFactoryTest, NoMask)

TYPED_TEST(NumericFactoryTest, UnitializedMask)
{
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
cudf::mask_state::UNINITIALIZED,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), cudf::mask_state::UNINITIALIZED);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_TRUE(column->nullable());
Expand All @@ -120,11 +104,8 @@ TYPED_TEST(NumericFactoryTest, UnitializedMask)

TYPED_TEST(NumericFactoryTest, AllValidMask)
{
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
cudf::mask_state::ALL_VALID,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), cudf::mask_state::ALL_VALID);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(0, column->null_count());
Expand All @@ -135,11 +116,8 @@ TYPED_TEST(NumericFactoryTest, AllValidMask)

TYPED_TEST(NumericFactoryTest, AllNullMask)
{
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
cudf::mask_state::ALL_NULL,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), cudf::mask_state::ALL_NULL);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(this->size(), column->null_count());
Expand All @@ -154,9 +132,7 @@ TYPED_TEST(NumericFactoryTest, NullMaskAsParm)
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
std::move(null_mask),
this->size(),
this->stream(),
this->mr());
this->size());
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(this->size(), column->null_count());
Expand All @@ -167,12 +143,8 @@ TYPED_TEST(NumericFactoryTest, NullMaskAsParm)

TYPED_TEST(NumericFactoryTest, NullMaskAsEmptyParm)
{
auto column = cudf::make_numeric_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
rmm::device_buffer{},
0,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), rmm::device_buffer{}, 0);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(0, column->null_count());
Expand All @@ -188,11 +160,8 @@ class NonNumericFactoryTest : public ColumnFactoryTest,
TEST_P(NonNumericFactoryTest, NonNumericThrow)
{
auto construct = [this]() {
auto column = cudf::make_numeric_column(cudf::data_type{GetParam()},
this->size(),
cudf::mask_state::UNALLOCATED,
this->stream(),
this->mr());
auto column = cudf::make_numeric_column(
cudf::data_type{GetParam()}, this->size(), cudf::mask_state::UNALLOCATED);
};
EXPECT_THROW(construct(), cudf::logic_error);
}
Expand All @@ -208,11 +177,8 @@ TYPED_TEST_SUITE(FixedWidthFactoryTest, cudf::test::FixedWidthTypes);

TYPED_TEST(FixedWidthFactoryTest, EmptyNoMask)
{
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
0,
cudf::mask_state::UNALLOCATED,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, 0, cudf::mask_state::UNALLOCATED);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
}

Expand All @@ -235,11 +201,8 @@ TYPED_TEST(EmptyFactoryTest, Empty)

TYPED_TEST(FixedWidthFactoryTest, EmptyAllValidMask)
{
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
0,
cudf::mask_state::ALL_VALID,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, 0, cudf::mask_state::ALL_VALID);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), 0);
EXPECT_EQ(0, column->null_count());
Expand All @@ -250,11 +213,8 @@ TYPED_TEST(FixedWidthFactoryTest, EmptyAllValidMask)

TYPED_TEST(FixedWidthFactoryTest, EmptyAllNullMask)
{
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
0,
cudf::mask_state::ALL_NULL,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, 0, cudf::mask_state::ALL_NULL);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), 0);
EXPECT_EQ(0, column->null_count());
Expand All @@ -265,11 +225,8 @@ TYPED_TEST(FixedWidthFactoryTest, EmptyAllNullMask)

TYPED_TEST(FixedWidthFactoryTest, NoMask)
{
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
cudf::mask_state::UNALLOCATED,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), cudf::mask_state::UNALLOCATED);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(0, column->null_count());
Expand All @@ -280,11 +237,8 @@ TYPED_TEST(FixedWidthFactoryTest, NoMask)

TYPED_TEST(FixedWidthFactoryTest, UnitializedMask)
{
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
cudf::mask_state::UNINITIALIZED,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), cudf::mask_state::UNINITIALIZED);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_TRUE(column->nullable());
Expand All @@ -293,11 +247,8 @@ TYPED_TEST(FixedWidthFactoryTest, UnitializedMask)

TYPED_TEST(FixedWidthFactoryTest, AllValidMask)
{
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
cudf::mask_state::ALL_VALID,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), cudf::mask_state::ALL_VALID);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(0, column->null_count());
Expand All @@ -308,11 +259,8 @@ TYPED_TEST(FixedWidthFactoryTest, AllValidMask)

TYPED_TEST(FixedWidthFactoryTest, AllNullMask)
{
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
cudf::mask_state::ALL_NULL,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), cudf::mask_state::ALL_NULL);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(this->size(), column->null_count());
Expand All @@ -327,9 +275,7 @@ TYPED_TEST(FixedWidthFactoryTest, NullMaskAsParm)
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
std::move(null_mask),
this->size(),
this->stream(),
this->mr());
this->size());
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(this->size(), column->null_count());
Expand All @@ -340,12 +286,8 @@ TYPED_TEST(FixedWidthFactoryTest, NullMaskAsParm)

TYPED_TEST(FixedWidthFactoryTest, NullMaskAsEmptyParm)
{
auto column = cudf::make_fixed_width_column(cudf::data_type{cudf::type_to_id<TypeParam>()},
this->size(),
rmm::device_buffer{},
0,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{cudf::type_to_id<TypeParam>()}, this->size(), rmm::device_buffer{}, 0);
EXPECT_EQ(column->type(), cudf::data_type{cudf::type_to_id<TypeParam>()});
EXPECT_EQ(column->size(), this->size());
EXPECT_EQ(0, column->null_count());
Expand All @@ -361,11 +303,8 @@ class NonFixedWidthFactoryTest : public ColumnFactoryTest,
TEST_P(NonFixedWidthFactoryTest, NonFixedWidthThrow)
{
auto construct = [this]() {
auto column = cudf::make_fixed_width_column(cudf::data_type{GetParam()},
this->size(),
cudf::mask_state::UNALLOCATED,
this->stream(),
this->mr());
auto column = cudf::make_fixed_width_column(
cudf::data_type{GetParam()}, this->size(), cudf::mask_state::UNALLOCATED);
};
EXPECT_THROW(construct(), cudf::logic_error);
}
Expand Down
17 changes: 10 additions & 7 deletions cpp/tests/copying/split_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2304,13 +2304,14 @@ TEST_F(ContiguousSplitTableCornerCases, OutBufferToSmall)
{
// internally, contiguous split chunks GPU work in 1MB contiguous copies
// so the output buffer must be 1MB or larger.
EXPECT_THROW(cudf::chunked_pack::create({}, 1 * 1024, mr()), cudf::logic_error);
EXPECT_THROW(cudf::chunked_pack::create({}, 1 * 1024), cudf::logic_error);
}

TEST_F(ContiguousSplitTableCornerCases, ChunkSpanTooSmall)
{
auto chunked_pack = cudf::chunked_pack::create({}, 1 * 1024 * 1024, mr());
rmm::device_buffer buff(1 * 1024, cudf::get_default_stream(), mr());
auto chunked_pack = cudf::chunked_pack::create({}, 1 * 1024 * 1024);
rmm::device_buffer buff(
1 * 1024, cudf::test::get_default_stream(), rmm::mr::get_current_device_resource());
cudf::device_span<uint8_t> too_small(static_cast<uint8_t*>(buff.data()), buff.size());
std::size_t copied = 0;
// throws because we created chunked_contig_split with 1MB, but we are giving
Expand All @@ -2321,8 +2322,9 @@ TEST_F(ContiguousSplitTableCornerCases, ChunkSpanTooSmall)

TEST_F(ContiguousSplitTableCornerCases, EmptyTableHasNextFalse)
{
auto chunked_pack = cudf::chunked_pack::create({}, 1 * 1024 * 1024, mr());
rmm::device_buffer buff(1 * 1024 * 1024, cudf::get_default_stream(), mr());
auto chunked_pack = cudf::chunked_pack::create({}, 1 * 1024 * 1024);
rmm::device_buffer buff(
1 * 1024 * 1024, cudf::test::get_default_stream(), rmm::mr::get_current_device_resource());
cudf::device_span<uint8_t> bounce_buff(static_cast<uint8_t*>(buff.data()), buff.size());
EXPECT_EQ(chunked_pack->has_next(), false); // empty input table
std::size_t copied = 0;
Expand All @@ -2334,9 +2336,10 @@ TEST_F(ContiguousSplitTableCornerCases, ExhaustedHasNextFalse)
{
cudf::test::strings_column_wrapper a{"abc", "def", "ghi", "jkl", "mno", "", "st", "uvwx"};
cudf::table_view t({a});
rmm::device_buffer buff(1 * 1024 * 1024, cudf::get_default_stream(), mr());
rmm::device_buffer buff(
1 * 1024 * 1024, cudf::test::get_default_stream(), rmm::mr::get_current_device_resource());
cudf::device_span<uint8_t> bounce_buff(static_cast<uint8_t*>(buff.data()), buff.size());
auto chunked_pack = cudf::chunked_pack::create(t, buff.size(), mr());
auto chunked_pack = cudf::chunked_pack::create(t, buff.size());
EXPECT_EQ(chunked_pack->has_next(), true);
std::size_t copied = chunked_pack->next(bounce_buff);
EXPECT_EQ(copied, chunked_pack->get_total_contiguous_size());
Expand Down
Loading

0 comments on commit 886e189

Please sign in to comment.