Skip to content

Commit

Permalink
Remove default parameters for cudf::strings::detail functions (#12003)
Browse files Browse the repository at this point in the history
Removes default parameters from the `cudf::strings::detail` functions. Most of these were unintentional the rest were for allowing for the default memory-resource which was easily fixed. Most of the detail functions are not used outside of strings and the default parameters were not actually necessary there.

Hopefully this will help with #11967

Authors:
  - David Wendt (https://github.com/davidwendt)
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Yunsong Wang (https://github.com/PointKernel)

URL: #12003
  • Loading branch information
davidwendt authored Nov 4, 2022
1 parent 1d6931a commit 0278485
Show file tree
Hide file tree
Showing 47 changed files with 426 additions and 501 deletions.
4 changes: 2 additions & 2 deletions cpp/benchmarks/string/json.cu
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ auto build_json_string_column(int desired_bytes, int num_rows)
auto d_store_order = cudf::column_device_view::create(float_2bool_columns->get_column(2));
json_benchmark_row_builder jb{
desired_bytes, num_rows, {*d_books, *d_bicycles}, *d_book_pct, *d_misc_order, *d_store_order};
auto children =
cudf::strings::detail::make_strings_children(jb, num_rows, cudf::get_default_stream());
auto children = cudf::strings::detail::make_strings_children(
jb, num_rows, cudf::get_default_stream(), rmm::mr::get_current_device_resource());
return cudf::make_strings_column(
num_rows, std::move(children.first), std::move(children.second), 0, {});
}
Expand Down
25 changes: 11 additions & 14 deletions cpp/include/cudf/strings/detail/combine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,24 @@ namespace detail {
*
* @param stream CUDA stream used for device memory operations and kernel launches.
*/
std::unique_ptr<column> concatenate(
table_view const& strings_columns,
string_scalar const& separator,
string_scalar const& narep,
separator_on_nulls separate_nulls = separator_on_nulls::YES,
// Move before separate_nulls?
rmm::cuda_stream_view stream = cudf::get_default_stream(),
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> concatenate(table_view const& strings_columns,
string_scalar const& separator,
string_scalar const& narep,
separator_on_nulls separate_nulls,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

/**
* @copydoc join_strings(table_view const&,string_scalar const&,string_scalar
* const&,rmm::mr::device_memory_resource*)
*
* @param stream CUDA stream used for device memory operations and kernel launches.
*/
std::unique_ptr<column> join_strings(
strings_column_view const& strings,
string_scalar const& separator,
string_scalar const& narep,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> join_strings(strings_column_view const& strings,
string_scalar const& separator,
string_scalar const& narep,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

/**
* @copydoc join_list_elements(table_view const&,string_scalar const&,string_scalar
Expand Down
7 changes: 3 additions & 4 deletions cpp/include/cudf/strings/detail/concatenate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ namespace detail {
* @param mr Device memory resource used to allocate the returned column's device memory.
* @return New column with concatenated results.
*/
std::unique_ptr<column> concatenate(
host_span<column_view const> columns,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> concatenate(host_span<column_view const> columns,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

} // namespace detail
} // namespace strings
Expand Down
13 changes: 6 additions & 7 deletions cpp/include/cudf/strings/detail/copy_if_else.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,12 @@ namespace detail {
* @return New strings column.
*/
template <typename StringIterLeft, typename StringIterRight, typename Filter>
std::unique_ptr<cudf::column> copy_if_else(
StringIterLeft lhs_begin,
StringIterLeft lhs_end,
StringIterRight rhs_begin,
Filter filter_fn,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
std::unique_ptr<cudf::column> copy_if_else(StringIterLeft lhs_begin,
StringIterLeft lhs_end,
StringIterRight rhs_begin,
Filter filter_fn,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
auto strings_count = std::distance(lhs_begin, lhs_end);
if (strings_count == 0) return make_empty_column(type_id::STRING);
Expand Down
15 changes: 7 additions & 8 deletions cpp/include/cudf/strings/detail/copy_range.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,13 @@ namespace detail {
* @return std::unique_ptr<column> The result target column
*/
template <typename SourceValueIterator, typename SourceValidityIterator>
std::unique_ptr<column> copy_range(
SourceValueIterator source_value_begin,
SourceValidityIterator source_validity_begin,
strings_column_view const& target,
size_type target_begin,
size_type target_end,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
std::unique_ptr<column> copy_range(SourceValueIterator source_value_begin,
SourceValidityIterator source_validity_begin,
strings_column_view const& target,
size_type target_begin,
size_type target_end,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
CUDF_EXPECTS(
(target_begin >= 0) && (target_begin < target.size()) && (target_end <= target.size()),
Expand Down
12 changes: 5 additions & 7 deletions cpp/include/cudf/strings/detail/copying.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,11 @@ namespace detail {
* @param mr Device memory resource used to allocate the returned column's device memory.
* @return New strings column of size (end-start)/step.
*/
std::unique_ptr<cudf::column> copy_slice(
strings_column_view const& strings,
size_type start,
size_type end = -1,
// Move before end?
rmm::cuda_stream_view stream = cudf::get_default_stream(),
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<cudf::column> copy_slice(strings_column_view const& strings,
size_type start,
size_type end,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

/**
* @brief Returns a new strings column created by shifting the rows by a specified offset.
Expand Down
13 changes: 6 additions & 7 deletions cpp/include/cudf/strings/detail/fill.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,12 @@ namespace detail {
* @param mr Device memory resource used to allocate the returned column's device memory.
* @return New strings column.
*/
std::unique_ptr<column> fill(
strings_column_view const& strings,
size_type begin,
size_type end,
string_scalar const& value,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> fill(strings_column_view const& strings,
size_type begin,
size_type end,
string_scalar const& value,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

} // namespace detail
} // namespace strings
Expand Down
24 changes: 11 additions & 13 deletions cpp/include/cudf/strings/detail/gather.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,11 @@ std::unique_ptr<cudf::column> gather_chars(StringIterator strings_begin,
* @return New strings column containing the gathered strings.
*/
template <bool NullifyOutOfBounds, typename MapIterator>
std::unique_ptr<cudf::column> gather(
strings_column_view const& strings,
MapIterator begin,
MapIterator end,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
std::unique_ptr<cudf::column> gather(strings_column_view const& strings,
MapIterator begin,
MapIterator end,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
auto const output_count = std::distance(begin, end);
auto const strings_count = strings.size();
Expand Down Expand Up @@ -372,13 +371,12 @@ std::unique_ptr<cudf::column> gather(
* @return New strings column containing the gathered strings.
*/
template <typename MapIterator>
std::unique_ptr<cudf::column> gather(
strings_column_view const& strings,
MapIterator begin,
MapIterator end,
bool nullify_out_of_bounds,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
std::unique_ptr<cudf::column> gather(strings_column_view const& strings,
MapIterator begin,
MapIterator end,
bool nullify_out_of_bounds,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
if (nullify_out_of_bounds) return gather<true>(strings, begin, end, stream, mr);
return gather<false>(strings, begin, end, stream, mr);
Expand Down
13 changes: 7 additions & 6 deletions cpp/include/cudf/strings/detail/json.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

#pragma once

#include <cudf/scalar/scalar.hpp>
#include <cudf/strings/json.hpp>
#include <cudf/strings/strings_column_view.hpp>
#include <cudf/utilities/default_stream.hpp>

Expand All @@ -30,12 +32,11 @@ namespace detail {
*
* @param stream CUDA stream used for device memory operations and kernel launches
*/
std::unique_ptr<cudf::column> get_json_object(
cudf::strings_column_view const& col,
cudf::string_scalar const& json_path,
get_json_object_options options,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<cudf::column> get_json_object(cudf::strings_column_view const& col,
cudf::string_scalar const& json_path,
cudf::strings::get_json_object_options options,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

} // namespace detail
} // namespace strings
Expand Down
49 changes: 21 additions & 28 deletions cpp/include/cudf/strings/detail/replace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,42 +43,37 @@ enum class replace_algorithm {
* @param[in] stream CUDA stream used for device memory operations and kernel launches.
*/
template <replace_algorithm alg = replace_algorithm::AUTO>
std::unique_ptr<column> replace(
strings_column_view const& strings,
string_scalar const& target,
string_scalar const& repl,
int32_t maxrepl = -1,
// Move before maxrepl?
rmm::cuda_stream_view stream = cudf::get_default_stream(),
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> replace(strings_column_view const& strings,
string_scalar const& target,
string_scalar const& repl,
int32_t maxrepl,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

/**
* @copydoc cudf::strings::replace_slice(strings_column_view const&, string_scalar const&,
* size_type. size_type, rmm::mr::device_memory_resource*)
*
* @param[in] stream CUDA stream used for device memory operations and kernel launches.
*/
std::unique_ptr<column> replace_slice(
strings_column_view const& strings,
string_scalar const& repl = string_scalar(""),
size_type start = 0,
size_type stop = -1,
// Move before repl?
rmm::cuda_stream_view stream = cudf::get_default_stream(),
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> replace_slice(strings_column_view const& strings,
string_scalar const& repl,
size_type start,
size_type stop,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

/**
* @copydoc cudf::strings::replace(strings_column_view const&, strings_column_view const&,
* strings_column_view const&, rmm::mr::device_memory_resource*)
*
* @param[in] stream CUDA stream used for device memory operations and kernel launches.
*/
std::unique_ptr<column> replace(
strings_column_view const& strings,
strings_column_view const& targets,
strings_column_view const& repls,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> replace(strings_column_view const& strings,
strings_column_view const& targets,
strings_column_view const& repls,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

/**
* @brief Replaces any null string entries with the given string.
Expand All @@ -98,12 +93,10 @@ std::unique_ptr<column> replace(
* @param mr Device memory resource used to allocate the returned column's device memory.
* @return New strings column.
*/
std::unique_ptr<column> replace_nulls(
strings_column_view const& strings,
string_scalar const& repl = string_scalar(""),
// Move before repl?
rmm::cuda_stream_view stream = cudf::get_default_stream(),
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> replace_nulls(strings_column_view const& strings,
string_scalar const& repl,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

} // namespace detail
} // namespace strings
Expand Down
16 changes: 8 additions & 8 deletions cpp/include/cudf/strings/detail/scatter.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,18 @@ namespace detail {
* @return New strings column.
*/
template <typename SourceIterator, typename MapIterator>
std::unique_ptr<column> scatter(
SourceIterator begin,
SourceIterator end,
MapIterator scatter_map,
strings_column_view const& target,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
std::unique_ptr<column> scatter(SourceIterator begin,
SourceIterator end,
MapIterator scatter_map,
strings_column_view const& target,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
if (target.is_empty()) return make_empty_column(type_id::STRING);

// create vector of string_view's to scatter into
rmm::device_uvector<string_view> target_vector = create_string_vector_from_column(target, stream);
rmm::device_uvector<string_view> target_vector =
create_string_vector_from_column(target, stream, rmm::mr::get_current_device_resource());

// this ensures empty strings are not mapped to nulls in the make_strings_column function
auto const size = thrust::distance(begin, end);
Expand Down
29 changes: 13 additions & 16 deletions cpp/include/cudf/strings/detail/utilities.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,10 @@ namespace detail {
* @return offsets child column for strings column
*/
template <typename InputIterator>
std::unique_ptr<column> make_offsets_child_column(
InputIterator begin,
InputIterator end,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
std::unique_ptr<column> make_offsets_child_column(InputIterator begin,
InputIterator end,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
CUDF_EXPECTS(begin < end, "Invalid iterator range");
auto count = thrust::distance(begin, end);
Expand Down Expand Up @@ -117,12 +116,11 @@ __device__ inline char* copy_string(char* buffer, const string_view& d_string)
* @return offsets child column and chars child column for a strings column
*/
template <typename SizeAndExecuteFunction>
auto make_strings_children(
SizeAndExecuteFunction size_and_exec_fn,
size_type exec_size,
size_type strings_count,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
auto make_strings_children(SizeAndExecuteFunction size_and_exec_fn,
size_type exec_size,
size_type strings_count,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
auto offsets_column = make_numeric_column(
data_type{type_id::INT32}, strings_count + 1, mask_state::UNALLOCATED, stream, mr);
Expand Down Expand Up @@ -175,11 +173,10 @@ auto make_strings_children(
* @return offsets child column and chars child column for a strings column
*/
template <typename SizeAndExecuteFunction>
auto make_strings_children(
SizeAndExecuteFunction size_and_exec_fn,
size_type strings_count,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
auto make_strings_children(SizeAndExecuteFunction size_and_exec_fn,
size_type strings_count,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
return make_strings_children(size_and_exec_fn, strings_count, strings_count, stream, mr);
}
Expand Down
9 changes: 4 additions & 5 deletions cpp/include/cudf/strings/detail/utilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ namespace detail {
* @param mr Device memory resource used to allocate the returned column's device memory.
* @return The chars child column for a strings column.
*/
std::unique_ptr<column> create_chars_child_column(
size_type bytes,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> create_chars_child_column(size_type bytes,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr);

/**
* @brief Creates a string_view vector from a strings column.
Expand All @@ -52,7 +51,7 @@ std::unique_ptr<column> create_chars_child_column(
rmm::device_uvector<string_view> create_string_vector_from_column(
cudf::strings_column_view const strings,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
rmm::mr::device_memory_resource* mr);

} // namespace detail
} // namespace strings
Expand Down
Loading

0 comments on commit 0278485

Please sign in to comment.