Skip to content

Replace cudf::detail::get_value() with cudf::get_element()#4358

Open
davidwendt wants to merge 4 commits intoNVIDIA:mainfrom
davidwendt:get-element-value
Open

Replace cudf::detail::get_value() with cudf::get_element()#4358
davidwendt wants to merge 4 commits intoNVIDIA:mainfrom
davidwendt:get-element-value

Conversation

@davidwendt
Copy link
Contributor

Replaces calls to libcudf internal cudf::detail::get_value() with public cudf::get_element().

Signed-off-by: David Wendt <dwendt@nvidia.com>
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR replaces two calls to the libcudf internal API cudf::detail::get_value() with the public cudf::get_element() API, removing the dependency on the private <cudf/detail/get_value.cuh> header.

Key changes:

  • bloom_filter.cu: Removes unused #include <cudf/detail/get_value.cuh> (no call sites existed in that file) and bumps the copyright year to 2026.
  • parse_uri.cu: Swaps the private header for <cudf/copying.hpp> (the correct public header for cudf::get_element()), and replaces the single call site with a properly typed cast through cudf::scalar_type_t<cudf::size_type>. Stream ordering is preserved end-to-end — stream is correctly passed to both cudf::get_element() and the subsequent .value(stream) call, matching the semantics of the original detail API.

Confidence Score: 5/5

  • This PR is safe to merge — it is a clean replacement of a private internal API with its public equivalent, with correct stream propagation throughout.
  • The change is minimal and mechanically correct. The offsets_column in parse_uri.cu is explicitly typed INT32 via type_to_id<size_type>(), making the static_cast downcast safe. stream is passed to both cudf::get_element() and .value(stream), preserving the original ordering semantics. The bloom_filter.cu change is a pure dead-include removal with no functional impact. No remaining uses of cudf::detail::get_value exist in the repository.
  • No files require special attention.

Important Files Changed

Filename Overview
src/main/cpp/src/bloom_filter.cu Removed unused #include <cudf/detail/get_value.cuh> (no calls to this API existed in the file) and updated copyright year to 2026; no functional changes.
src/main/cpp/src/parse_uri.cu Replaces cudf::detail::get_value<size_type>() with public cudf::get_element() + .value(stream). The offsets_column is typed INT32 (created via type_to_id<size_type>()), so the static_cast<cudf::numeric_scalar<int32_t> const&> downcast is safe. Stream is correctly propagated end-to-end to both get_element and .value().

Sequence Diagram

sequenceDiagram
    participant Caller
    participant parse_uri
    participant cudf_get_element as cudf::get_element()
    participant scalar as numeric_scalar<int32_t>
    participant Host

    Caller->>parse_uri: parse_uri(input, chunk, ..., stream, mr)
    parse_uri->>parse_uri: build offsets_column (INT32, stream)
    parse_uri->>parse_uri: exclusive_scan offsets (stream)
    parse_uri->>cudf_get_element: get_element(offsets_view, offset_count-1, stream)
    Note over cudf_get_element: copies last offset element<br/>to device scalar on `stream`
    cudf_get_element-->>parse_uri: unique_ptr<scalar>
    parse_uri->>scalar: static_cast<numeric_scalar<int32_t>>.value(stream)
    Note over scalar: syncs `stream`, copies<br/>value to host
    scalar-->>parse_uri: out_chars_bytes (host int32_t)
    parse_uri->>parse_uri: allocate d_out_chars(out_chars_bytes, stream, mr)
    parse_uri-->>Caller: strings column
Loading

Last reviewed commit: b5c0c95

Signed-off-by: David Wendt <dwendt@nvidia.com>
…s-jni into get-element-value

Signed-off-by: David Wendt <dwendt@nvidia.com>
@mythrocks
Copy link
Collaborator

Build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants