From a36dacb66325e03d3264482d35a5cf7e0b6c7a37 Mon Sep 17 00:00:00 2001 From: Lawrence Mitchell Date: Thu, 25 Jul 2024 00:31:40 +0100 Subject: [PATCH] Make C++ compilation warning free after #16297 (#16379) In https://github.com/rapidsai/cudf/pull/16297, we deprecated the use of `to_arrow` in favour of `to_arrow_host` and `to_arrow_device`. However, the scalar detail overload of `to_arrow` used the public table overload. So we get a warning when compiling internal libcudf code. Fix this by using the detail API, and fix a bug along the way where we were not passing through the arrow memory resource. Authors: - Lawrence Mitchell (https://github.com/wence-) Approvers: - David Wendt (https://github.com/davidwendt) - Michael Schellenberger Costa (https://github.com/miscco) - Vyas Ramasubramani (https://github.com/vyasr) - Karthikeyan (https://github.com/karthikeyann) URL: https://github.com/rapidsai/cudf/pull/16379 --- cpp/include/cudf/interop.hpp | 13 ++++++++----- cpp/src/interop/to_arrow.cu | 2 +- cpp/tests/interop/from_arrow_test.cpp | 9 +++++++++ cpp/tests/interop/to_arrow_test.cpp | 10 ++++++++++ cpp/tests/streams/interop_test.cpp | 9 +++++++++ 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/cpp/include/cudf/interop.hpp b/cpp/include/cudf/interop.hpp index 61f7d72a467..73bc205a095 100644 --- a/cpp/include/cudf/interop.hpp +++ b/cpp/include/cudf/interop.hpp @@ -152,7 +152,7 @@ struct column_metadata { * 9 which is the maximum precision for 32-bit types. Similarly, numeric::decimal128 will be * converted to Arrow decimal128 of the precision 38. */ -[[deprecated]] std::shared_ptr to_arrow( +[[deprecated("Use cudf::to_arrow_host")]] std::shared_ptr to_arrow( table_view input, std::vector const& metadata = {}, rmm::cuda_stream_view stream = cudf::get_default_stream(), @@ -177,7 +177,7 @@ struct column_metadata { * 9 which is the maximum precision for 32-bit types. Similarly, numeric::decimal128 will be * converted to Arrow decimal128 of the precision 38. */ -[[deprecated]] std::shared_ptr to_arrow( +[[deprecated("Use cudf::to_arrow_host")]] std::shared_ptr to_arrow( cudf::scalar const& input, column_metadata const& metadata = {}, rmm::cuda_stream_view stream = cudf::get_default_stream(), @@ -395,7 +395,7 @@ unique_device_array_t to_arrow_host( * @param mr Device memory resource used to allocate `cudf::table` * @return cudf table generated from given arrow Table */ -[[deprecated]] std::unique_ptr from_arrow( +[[deprecated("Use cudf::from_arrow_host")]] std::unique_ptr
from_arrow( arrow::Table const& input, rmm::cuda_stream_view stream = cudf::get_default_stream(), rmm::device_async_resource_ref mr = rmm::mr::get_current_device_resource()); @@ -403,14 +403,17 @@ unique_device_array_t to_arrow_host( /** * @brief Create `cudf::scalar` from given arrow Scalar input * - * @deprecated Since 24.08. + * @deprecated Since 24.08. Use arrow's `MakeArrayFromScalar` on the + * input, followed by `ExportArray` to obtain something that can be + * consumed by `from_arrow_host`. Then use `cudf::get_element` to + * extract a device scalar from the column. * * @param input `arrow::Scalar` that needs to be converted to `cudf::scalar` * @param stream CUDA stream used for device memory operations and kernel launches * @param mr Device memory resource used to allocate `cudf::scalar` * @return cudf scalar generated from given arrow Scalar */ -[[deprecated]] std::unique_ptr from_arrow( +[[deprecated("See docstring for migration strategies")]] std::unique_ptr from_arrow( arrow::Scalar const& input, rmm::cuda_stream_view stream = cudf::get_default_stream(), rmm::device_async_resource_ref mr = rmm::mr::get_current_device_resource()); diff --git a/cpp/src/interop/to_arrow.cu b/cpp/src/interop/to_arrow.cu index e89ecedc218..6b163e3441e 100644 --- a/cpp/src/interop/to_arrow.cu +++ b/cpp/src/interop/to_arrow.cu @@ -458,7 +458,7 @@ std::shared_ptr to_arrow(cudf::scalar const& input, { auto const column = cudf::make_column_from_scalar(input, 1, stream); cudf::table_view const tv{{column->view()}}; - auto const arrow_table = cudf::to_arrow(tv, {metadata}, stream); + auto const arrow_table = detail::to_arrow(tv, {metadata}, stream, ar_mr); auto const ac = arrow_table->column(0); auto const maybe_scalar = ac->GetScalar(0); if (!maybe_scalar.ok()) { CUDF_FAIL("Failed to produce a scalar"); } diff --git a/cpp/tests/interop/from_arrow_test.cpp b/cpp/tests/interop/from_arrow_test.cpp index 6eaa1a07e08..733e5814425 100644 --- a/cpp/tests/interop/from_arrow_test.cpp +++ b/cpp/tests/interop/from_arrow_test.cpp @@ -14,6 +14,13 @@ * limitations under the License. */ +// These interop functions are deprecated. We keep the code in this +// test and will migrate the tests to export the arrow C data +// interface which we consume with from_arrow_host. For now, the tests +// are commented out. + +#if 0 + #include #include @@ -595,3 +602,5 @@ TEST_F(FromArrowStructScalarTest, Basic) CUDF_TEST_EXPECT_TABLES_EQUAL(lhs, cudf_struct_scalar->view()); } + +#endif diff --git a/cpp/tests/interop/to_arrow_test.cpp b/cpp/tests/interop/to_arrow_test.cpp index a1ece0ce0f1..328ba210a3f 100644 --- a/cpp/tests/interop/to_arrow_test.cpp +++ b/cpp/tests/interop/to_arrow_test.cpp @@ -14,6 +14,13 @@ * limitations under the License. */ +// These interop functions are deprecated. We keep the code in this +// test and will migrate the tests to export via the arrow C data +// interface with to_arrow_host which arrow can consume. For now, the +// test is commented out. + +#if 0 + #include #include @@ -196,6 +203,7 @@ TEST_F(ToArrowTest, DateTimeTable) std::vector> schema_vector({arrow::field("a", arr->type())}); auto schema = std::make_shared(schema_vector); + auto expected_arrow_table = arrow::Table::Make(schema, {arr}); auto got_arrow_table = cudf::to_arrow(input_view, {{"a"}}); @@ -685,3 +693,5 @@ TEST_F(ToArrowStructScalarTest, Basic) } CUDF_TEST_PROGRAM_MAIN() + +#endif diff --git a/cpp/tests/streams/interop_test.cpp b/cpp/tests/streams/interop_test.cpp index 9e4ee5a4a93..9ba862585d0 100644 --- a/cpp/tests/streams/interop_test.cpp +++ b/cpp/tests/streams/interop_test.cpp @@ -14,6 +14,13 @@ * limitations under the License. */ +// These interop functions are deprecated. We keep the code in this +// test and will migrate the tests to export via the arrow C data +// interface with to_arrow_host which arrow can consume. For now, the +// test is commented out. + +#if 0 + #include #include #include @@ -67,3 +74,5 @@ TEST_F(ArrowTest, FromArrowScalar) auto arrow_scalar = arrow::MakeScalar(value); cudf::from_arrow(*arrow_scalar, cudf::test::get_default_stream()); } + +#endif