Skip to content

Commit

Permalink
Make C++ compilation warning free after #16297 (#16379)
Browse files Browse the repository at this point in the history
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.

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: #16379
  • Loading branch information
wence- authored Jul 24, 2024
1 parent ae4c7e3 commit a36dacb
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 6 deletions.
13 changes: 8 additions & 5 deletions cpp/include/cudf/interop.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<arrow::Table> to_arrow(
[[deprecated("Use cudf::to_arrow_host")]] std::shared_ptr<arrow::Table> to_arrow(
table_view input,
std::vector<column_metadata> const& metadata = {},
rmm::cuda_stream_view stream = cudf::get_default_stream(),
Expand All @@ -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<arrow::Scalar> to_arrow(
[[deprecated("Use cudf::to_arrow_host")]] std::shared_ptr<arrow::Scalar> to_arrow(
cudf::scalar const& input,
column_metadata const& metadata = {},
rmm::cuda_stream_view stream = cudf::get_default_stream(),
Expand Down Expand Up @@ -395,22 +395,25 @@ 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<table> from_arrow(
[[deprecated("Use cudf::from_arrow_host")]] std::unique_ptr<table> 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());

/**
* @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<cudf::scalar> from_arrow(
[[deprecated("See docstring for migration strategies")]] std::unique_ptr<cudf::scalar> 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());
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/interop/to_arrow.cu
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ std::shared_ptr<arrow::Scalar> 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"); }
Expand Down
9 changes: 9 additions & 0 deletions cpp/tests/interop/from_arrow_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <tests/interop/arrow_utils.hpp>

#include <cudf_test/base_fixture.hpp>
Expand Down Expand Up @@ -595,3 +602,5 @@ TEST_F(FromArrowStructScalarTest, Basic)

CUDF_TEST_EXPECT_TABLES_EQUAL(lhs, cudf_struct_scalar->view());
}

#endif
10 changes: 10 additions & 0 deletions cpp/tests/interop/to_arrow_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <tests/interop/arrow_utils.hpp>

#include <cudf_test/base_fixture.hpp>
Expand Down Expand Up @@ -196,6 +203,7 @@ TEST_F(ToArrowTest, DateTimeTable)
std::vector<std::shared_ptr<arrow::Field>> schema_vector({arrow::field("a", arr->type())});
auto schema = std::make_shared<arrow::Schema>(schema_vector);


auto expected_arrow_table = arrow::Table::Make(schema, {arr});

auto got_arrow_table = cudf::to_arrow(input_view, {{"a"}});
Expand Down Expand Up @@ -685,3 +693,5 @@ TEST_F(ToArrowStructScalarTest, Basic)
}

CUDF_TEST_PROGRAM_MAIN()

#endif
9 changes: 9 additions & 0 deletions cpp/tests/streams/interop_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <cudf_test/base_fixture.hpp>
#include <cudf_test/column_wrapper.hpp>
#include <cudf_test/default_stream.hpp>
Expand Down Expand Up @@ -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

0 comments on commit a36dacb

Please sign in to comment.