From 2c073d984794a3c643e597016f4736e432b02767 Mon Sep 17 00:00:00 2001 From: Lawrence Mitchell Date: Wed, 24 Jul 2024 11:31:53 +0000 Subject: [PATCH 1/5] Avoid deprecation warning in internal to_arrow code In #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. --- cpp/src/interop/to_arrow.cu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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"); } From 1c71a0e41aa3e9f3934b7789d3addbba49565815 Mon Sep 17 00:00:00 2001 From: Lawrence Mitchell Date: Wed, 24 Jul 2024 11:46:33 +0000 Subject: [PATCH 2/5] Remove deprecation on scalar from_arrow overload There is no replacement for this function, so the deprecation is not actionable. --- cpp/include/cudf/interop.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cpp/include/cudf/interop.hpp b/cpp/include/cudf/interop.hpp index 61f7d72a467..632afd2cedb 100644 --- a/cpp/include/cudf/interop.hpp +++ b/cpp/include/cudf/interop.hpp @@ -403,14 +403,12 @@ unique_device_array_t to_arrow_host( /** * @brief Create `cudf::scalar` from given arrow Scalar input * - * @deprecated Since 24.08. - * * @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( +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()); From 2ef531125fef76df36e692484c3040d87f8824a1 Mon Sep 17 00:00:00 2001 From: Lawrence Mitchell Date: Wed, 24 Jul 2024 11:47:44 +0000 Subject: [PATCH 3/5] Provide pointers in the deprecation messages for arrow interop --- cpp/include/cudf/interop.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/include/cudf/interop.hpp b/cpp/include/cudf/interop.hpp index 632afd2cedb..ee32b613c47 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()); From fb6a661b5004cd437c4259ddc789146b51825c1a Mon Sep 17 00:00:00 2001 From: Lawrence Mitchell Date: Wed, 24 Jul 2024 17:27:17 +0000 Subject: [PATCH 4/5] Reintroduce deprecation for scalar overload, and add migration --- cpp/include/cudf/interop.hpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cpp/include/cudf/interop.hpp b/cpp/include/cudf/interop.hpp index ee32b613c47..73bc205a095 100644 --- a/cpp/include/cudf/interop.hpp +++ b/cpp/include/cudf/interop.hpp @@ -403,12 +403,17 @@ unique_device_array_t to_arrow_host( /** * @brief Create `cudf::scalar` from given arrow Scalar input * + * @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 */ -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()); From 770a7535f6924ec06338d06f57f73d86fafbec54 Mon Sep 17 00:00:00 2001 From: Lawrence Mitchell Date: Wed, 24 Jul 2024 17:27:47 +0000 Subject: [PATCH 5/5] Comment out tests with use deprecated functionality We will migrate these to test the arrow C interchange API, hence leaving the code in place. --- cpp/tests/interop/from_arrow_test.cpp | 9 +++++++++ cpp/tests/interop/to_arrow_test.cpp | 10 ++++++++++ cpp/tests/streams/interop_test.cpp | 9 +++++++++ 3 files changed, 28 insertions(+) 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