From 3733668b2ae41127ea5057dbbea22bdc3cce80a8 Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Fri, 17 Jan 2025 22:35:31 +0000 Subject: [PATCH 01/11] streams to scalar classes --- cpp/include/cudf/scalar/scalar.hpp | 26 ++++++++++++++++++-------- cpp/src/scalar/scalar.cpp | 15 +++++++++------ cpp/tests/binaryop/assert-binops.h | 6 +++--- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/cpp/include/cudf/scalar/scalar.hpp b/cpp/include/cudf/scalar/scalar.hpp index 360dde11fc0..4170a0df699 100644 --- a/cpp/include/cudf/scalar/scalar.hpp +++ b/cpp/include/cudf/scalar/scalar.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2024, NVIDIA CORPORATION. + * Copyright (c) 2019-2025, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -177,9 +177,12 @@ class fixed_width_scalar : public scalar { void set_value(T value, rmm::cuda_stream_view stream = cudf::get_default_stream()); /** - * @brief Explicit conversion operator to get the value of the scalar on the host. + * @brief Returns the value of the scalar on the host. + * + * @param stream CUDA stream used for device memory operations. + * @return The value of the scalar */ - explicit operator value_type() const; + [[nodiscard]] T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Get the value of the scalar. @@ -386,7 +389,7 @@ class fixed_point_scalar : public scalar { rmm::device_async_resource_ref mr = cudf::get_current_device_resource_ref()); /** - * @brief Get the value of the scalar. + * @brief Get the unscaled value of the scalar. * * @param stream CUDA stream used for device memory operations. * @return The value of the scalar @@ -403,9 +406,12 @@ class fixed_point_scalar : public scalar { rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** - * @brief Explicit conversion operator to get the value of the scalar on the host. + * @brief Returns the value of the scalar as decimal32, decimal64 or decimal128 on the host. + * + * @param stream CUDA stream used for device memory operations. + * @return The value of the scalar */ - explicit operator value_type() const; + [[nodiscard]] T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Returns a raw pointer to the value in device memory. @@ -516,9 +522,13 @@ class string_scalar : public scalar { rmm::device_async_resource_ref mr = cudf::get_current_device_resource_ref()); /** - * @brief Explicit conversion operator to get the value of the scalar in a host std::string. + * @brief Returns the value of the scalar in a host std::string. + * + * @param stream CUDA stream used for device memory operations. + * @return The value of the scalar */ - explicit operator std::string() const; + [[nodiscard]] std::string get_value( + rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Get the value of the scalar in a host std::string. diff --git a/cpp/src/scalar/scalar.cpp b/cpp/src/scalar/scalar.cpp index 4b0b08fe251..6d523a029a1 100644 --- a/cpp/src/scalar/scalar.cpp +++ b/cpp/src/scalar/scalar.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2024, NVIDIA CORPORATION. + * Copyright (c) 2019-2025, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -110,7 +110,10 @@ size_type string_scalar::size() const { return _data.size(); } char const* string_scalar::data() const { return static_cast(_data.data()); } -string_scalar::operator std::string() const { return this->to_string(cudf::get_default_stream()); } +std::string string_scalar::get_value(rmm::cuda_stream_view stream) const +{ + return this->to_string(cudf::get_default_stream()); +} std::string string_scalar::to_string(rmm::cuda_stream_view stream) const { @@ -184,9 +187,9 @@ T fixed_point_scalar::fixed_point_value(rmm::cuda_stream_view stream) const } template -fixed_point_scalar::operator value_type() const +T fixed_point_scalar::get_value(rmm::cuda_stream_view stream) const { - return this->fixed_point_value(cudf::get_default_stream()); + return this->fixed_point_value(stream); } template @@ -267,9 +270,9 @@ T const* fixed_width_scalar::data() const } template -fixed_width_scalar::operator value_type() const +T fixed_width_scalar::get_value(rmm::cuda_stream_view stream) const { - return this->value(cudf::get_default_stream()); + return this->value(stream); } /** diff --git a/cpp/tests/binaryop/assert-binops.h b/cpp/tests/binaryop/assert-binops.h index 6933d15d508..d254cba6350 100644 --- a/cpp/tests/binaryop/assert-binops.h +++ b/cpp/tests/binaryop/assert-binops.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2023, NVIDIA CORPORATION. + * Copyright (c) 2019-2025, NVIDIA CORPORATION. * * Copyright 2018-2019 BlazingDB, Inc. * Copyright 2018 Christian Noboa Mardini @@ -81,7 +81,7 @@ void ASSERT_BINOP(cudf::column_view const& out, TypeOp&& op, ValueComparator const& value_comparator = ValueComparator()) { - auto lhs_h = static_cast(lhs).operator TypeLhs(); + auto lhs_h = static_cast(lhs).get_value(cudf::get_default_stream()); auto rhs_h = cudf::test::to_host(rhs); auto rhs_data = rhs_h.first; auto out_h = cudf::test::to_host(out); @@ -129,7 +129,7 @@ void ASSERT_BINOP(cudf::column_view const& out, TypeOp&& op, ValueComparator const& value_comparator = ValueComparator()) { - auto rhs_h = static_cast(rhs).operator TypeRhs(); + auto rhs_h = static_cast(rhs).get_value(cudf::get_default_stream()); auto lhs_h = cudf::test::to_host(lhs); auto lhs_data = lhs_h.first; auto out_h = cudf::test::to_host(out); From 40f6ba7435a3f899927f1b5f8c33c422222788ee Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Fri, 17 Jan 2025 22:44:51 +0000 Subject: [PATCH 02/11] streams to avro --- cpp/include/cudf/io/avro.hpp | 4 +++- cpp/src/io/functions.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cpp/include/cudf/io/avro.hpp b/cpp/include/cudf/io/avro.hpp index b307d05c09d..78cff9faa77 100644 --- a/cpp/include/cudf/io/avro.hpp +++ b/cpp/include/cudf/io/avro.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2024, NVIDIA CORPORATION. + * Copyright (c) 2020-2025, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -208,6 +208,7 @@ class avro_reader_options_builder { * @endcode * * @param options Settings for controlling reading behavior + * @param stream CUDA stream used for device memory operations and kernel launches * @param mr Device memory resource used to allocate device memory of the table in the returned * table_with_metadata * @@ -215,6 +216,7 @@ class avro_reader_options_builder { */ table_with_metadata read_avro( avro_reader_options const& options, + rmm::cuda_stream_view stream = cudf::get_default_stream(), rmm::device_async_resource_ref mr = cudf::get_current_device_resource_ref()); /** @} */ // end of group diff --git a/cpp/src/io/functions.cpp b/cpp/src/io/functions.cpp index d63fa9f5c35..69fd4068712 100644 --- a/cpp/src/io/functions.cpp +++ b/cpp/src/io/functions.cpp @@ -189,7 +189,9 @@ std::vector> make_datasinks(sink_info const& info) } // namespace -table_with_metadata read_avro(avro_reader_options const& options, rmm::device_async_resource_ref mr) +table_with_metadata read_avro(avro_reader_options const& options, + rmm::cuda_stream_view stream, + rmm::device_async_resource_ref mr) { namespace avro = cudf::io::detail::avro; @@ -199,7 +201,7 @@ table_with_metadata read_avro(avro_reader_options const& options, rmm::device_as CUDF_EXPECTS(datasources.size() == 1, "Only a single source is currently supported."); - return avro::read_avro(std::move(datasources[0]), options, cudf::get_default_stream(), mr); + return avro::read_avro(std::move(datasources[0]), options, stream, mr); } table_with_metadata read_json(json_reader_options options, From 5d068575f1f6815a555be41dc8696e3fe70fdd2a Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Sat, 18 Jan 2025 00:19:28 +0000 Subject: [PATCH 03/11] added test --- cpp/include/cudf/scalar/scalar.hpp | 7 ++--- cpp/tests/CMakeLists.txt | 1 + cpp/tests/streams/scalar_test.cpp | 50 ++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 cpp/tests/streams/scalar_test.cpp diff --git a/cpp/include/cudf/scalar/scalar.hpp b/cpp/include/cudf/scalar/scalar.hpp index 4170a0df699..c869d790f46 100644 --- a/cpp/include/cudf/scalar/scalar.hpp +++ b/cpp/include/cudf/scalar/scalar.hpp @@ -182,7 +182,7 @@ class fixed_width_scalar : public scalar { * @param stream CUDA stream used for device memory operations. * @return The value of the scalar */ - [[nodiscard]] T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; + T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Get the value of the scalar. @@ -411,7 +411,7 @@ class fixed_point_scalar : public scalar { * @param stream CUDA stream used for device memory operations. * @return The value of the scalar */ - [[nodiscard]] T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; + T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Returns a raw pointer to the value in device memory. @@ -527,8 +527,7 @@ class string_scalar : public scalar { * @param stream CUDA stream used for device memory operations. * @return The value of the scalar */ - [[nodiscard]] std::string get_value( - rmm::cuda_stream_view stream = cudf::get_default_stream()) const; + std::string get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Get the value of the scalar in a host std::string. diff --git a/cpp/tests/CMakeLists.txt b/cpp/tests/CMakeLists.txt index 6a89b1e48d6..9476c0ed817 100644 --- a/cpp/tests/CMakeLists.txt +++ b/cpp/tests/CMakeLists.txt @@ -724,6 +724,7 @@ ConfigureTest(STREAM_REPLACE_TEST streams/replace_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_RESHAPE_TEST streams/reshape_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_ROLLING_TEST streams/rolling_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_ROUND_TEST streams/round_test.cpp STREAM_MODE testing) +ConfigureTest(STREAM_SCALAR_TEST streams/scalar_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_SEARCH_TEST streams/search_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_SORTING_TEST streams/sorting_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_STREAM_COMPACTION_TEST streams/stream_compaction_test.cpp STREAM_MODE testing) diff --git a/cpp/tests/streams/scalar_test.cpp b/cpp/tests/streams/scalar_test.cpp new file mode 100644 index 00000000000..4ab9a0af97e --- /dev/null +++ b/cpp/tests/streams/scalar_test.cpp @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2023-2025, NVIDIA CORPORATION. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include + +#include + +template +struct TypedScalarTest : public cudf::test::BaseFixture {}; + +TYPED_TEST_SUITE(TypedScalarTest, cudf::test::FixedWidthTypes); + +TYPED_TEST(TypedScalarTest, DefaultValidity) +{ + using Type = cudf::device_storage_type_t; + Type value = static_cast(cudf::test::make_type_param_scalar(7)); + cudf::scalar_type_t s(value); + CUDF_EXPECT_NO_THROW(s.get_value(cudf::test::get_default_stream())); + + EXPECT_TRUE(s.is_valid()); + EXPECT_EQ(value, s.value()); +} + +struct StringScalarTest : public cudf::test::BaseFixture {}; + +TEST_F(StringScalarTest, DefaultValidity) +{ + std::string value = "test string"; + auto s = cudf::string_scalar(value); + CUDF_EXPECT_NO_THROW(s.get_value(cudf::test::get_default_stream())); + EXPECT_TRUE(s.is_valid()); +} From ffab23904534c70cfd650fe6aa5fdcf5ec879b20 Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Sat, 18 Jan 2025 00:25:18 +0000 Subject: [PATCH 04/11] fix --- cpp/src/scalar/scalar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/scalar/scalar.cpp b/cpp/src/scalar/scalar.cpp index 6d523a029a1..c3663987022 100644 --- a/cpp/src/scalar/scalar.cpp +++ b/cpp/src/scalar/scalar.cpp @@ -112,7 +112,7 @@ char const* string_scalar::data() const { return static_cast(_data. std::string string_scalar::get_value(rmm::cuda_stream_view stream) const { - return this->to_string(cudf::get_default_stream()); + return this->to_string(stream); } std::string string_scalar::to_string(rmm::cuda_stream_view stream) const From b6f2c1ae15ee381c62d9be728af6f6bb322b29e9 Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Thu, 23 Jan 2025 22:03:42 +0000 Subject: [PATCH 05/11] might need to rollback --- cpp/include/cudf/scalar/scalar.hpp | 6 +++--- cpp/src/scalar/scalar.cpp | 8 +++++++- cpp/tests/binaryop/assert-binops.h | 19 +++++++++++++++++-- cpp/tests/streams/scalar_test.cpp | 19 +++++++++---------- 4 files changed, 36 insertions(+), 16 deletions(-) diff --git a/cpp/include/cudf/scalar/scalar.hpp b/cpp/include/cudf/scalar/scalar.hpp index c869d790f46..83b7838a916 100644 --- a/cpp/include/cudf/scalar/scalar.hpp +++ b/cpp/include/cudf/scalar/scalar.hpp @@ -182,7 +182,7 @@ class fixed_width_scalar : public scalar { * @param stream CUDA stream used for device memory operations. * @return The value of the scalar */ - T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; + //T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Get the value of the scalar. @@ -411,7 +411,7 @@ class fixed_point_scalar : public scalar { * @param stream CUDA stream used for device memory operations. * @return The value of the scalar */ - T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; + //T get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Returns a raw pointer to the value in device memory. @@ -527,7 +527,7 @@ class string_scalar : public scalar { * @param stream CUDA stream used for device memory operations. * @return The value of the scalar */ - std::string get_value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; + //std::string value(rmm::cuda_stream_view stream = cudf::get_default_stream()) const; /** * @brief Get the value of the scalar in a host std::string. diff --git a/cpp/src/scalar/scalar.cpp b/cpp/src/scalar/scalar.cpp index c3663987022..b0224f83294 100644 --- a/cpp/src/scalar/scalar.cpp +++ b/cpp/src/scalar/scalar.cpp @@ -110,10 +110,12 @@ size_type string_scalar::size() const { return _data.size(); } char const* string_scalar::data() const { return static_cast(_data.data()); } -std::string string_scalar::get_value(rmm::cuda_stream_view stream) const +/* +std::string string_scalar::value(rmm::cuda_stream_view stream) const { return this->to_string(stream); } +*/ std::string string_scalar::to_string(rmm::cuda_stream_view stream) const { @@ -186,11 +188,13 @@ T fixed_point_scalar::fixed_point_value(rmm::cuda_stream_view stream) const numeric::scaled_integer{_data.value(stream), numeric::scale_type{type().scale()}}}; } +/* template T fixed_point_scalar::get_value(rmm::cuda_stream_view stream) const { return this->fixed_point_value(stream); } +*/ template typename fixed_point_scalar::rep_type* fixed_point_scalar::data() @@ -269,11 +273,13 @@ T const* fixed_width_scalar::data() const return _data.data(); } +/* template T fixed_width_scalar::get_value(rmm::cuda_stream_view stream) const { return this->value(stream); } +*/ /** * @brief These define the valid fixed-width scalar types. diff --git a/cpp/tests/binaryop/assert-binops.h b/cpp/tests/binaryop/assert-binops.h index d254cba6350..a1a3ded5456 100644 --- a/cpp/tests/binaryop/assert-binops.h +++ b/cpp/tests/binaryop/assert-binops.h @@ -81,7 +81,15 @@ void ASSERT_BINOP(cudf::column_view const& out, TypeOp&& op, ValueComparator const& value_comparator = ValueComparator()) { - auto lhs_h = static_cast(lhs).get_value(cudf::get_default_stream()); + //auto lhs_h = static_cast(lhs).value(cudf::get_default_stream()); + TypeLhs lhs_h; + if constexpr (std::is_same_v>) { + auto sv = static_cast(lhs).value(cudf::get_default_stream()); + lhs_h = std::string(sv.begin(), sv.end()); + } + else { + lhs_h = static_cast(lhs).value(cudf::get_default_stream()); + } auto rhs_h = cudf::test::to_host(rhs); auto rhs_data = rhs_h.first; auto out_h = cudf::test::to_host(out); @@ -129,11 +137,18 @@ void ASSERT_BINOP(cudf::column_view const& out, TypeOp&& op, ValueComparator const& value_comparator = ValueComparator()) { - auto rhs_h = static_cast(rhs).get_value(cudf::get_default_stream()); auto lhs_h = cudf::test::to_host(lhs); auto lhs_data = lhs_h.first; auto out_h = cudf::test::to_host(out); auto out_data = out_h.first; + TypeRhs rhs_h; + if constexpr (std::is_same_v>) { + auto sv = static_cast(rhs).value(cudf::get_default_stream()); + rhs_h = std::string(sv.begin(), sv.end()); + } + else { + rhs_h = static_cast(rhs).value(cudf::get_default_stream()); + } ASSERT_EQ(out_data.size(), lhs_data.size()); for (size_t i = 0; i < out_data.size(); ++i) { diff --git a/cpp/tests/streams/scalar_test.cpp b/cpp/tests/streams/scalar_test.cpp index 4ab9a0af97e..87f7dfe0d89 100644 --- a/cpp/tests/streams/scalar_test.cpp +++ b/cpp/tests/streams/scalar_test.cpp @@ -15,13 +15,11 @@ */ #include -#include #include -#include -#include #include #include +#include template struct TypedScalarTest : public cudf::test::BaseFixture {}; @@ -33,10 +31,7 @@ TYPED_TEST(TypedScalarTest, DefaultValidity) using Type = cudf::device_storage_type_t; Type value = static_cast(cudf::test::make_type_param_scalar(7)); cudf::scalar_type_t s(value); - CUDF_EXPECT_NO_THROW(s.get_value(cudf::test::get_default_stream())); - - EXPECT_TRUE(s.is_valid()); - EXPECT_EQ(value, s.value()); + EXPECT_EQ(value, s.value(cudf::test::get_default_stream())); } struct StringScalarTest : public cudf::test::BaseFixture {}; @@ -44,7 +39,11 @@ struct StringScalarTest : public cudf::test::BaseFixture {}; TEST_F(StringScalarTest, DefaultValidity) { std::string value = "test string"; - auto s = cudf::string_scalar(value); - CUDF_EXPECT_NO_THROW(s.get_value(cudf::test::get_default_stream())); - EXPECT_TRUE(s.is_valid()); + auto s = cudf::string_scalar(value); + auto sv = s.value(cudf::test::get_default_stream()); + EXPECT_EQ(cudf::string_view(value.c_str(), value.size()), sv); + /* + auto expected_value = std::string(sv.data(), sv.size_bytes()); + EXPECT_EQ(value, expected_value); + */ } From 610cc0b56fb4a729ceb5ff84ad88b14a6411c707 Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Fri, 24 Jan 2025 02:40:55 +0000 Subject: [PATCH 06/11] working --- cpp/include/cudf/scalar/scalar.hpp | 17 +---------- cpp/src/scalar/scalar.cpp | 16 +---------- cpp/tests/CMakeLists.txt | 1 + cpp/tests/binaryop/assert-binops.h | 19 ++++++++++-- cpp/tests/streams/scalar_test.cpp | 46 ++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 34 deletions(-) create mode 100644 cpp/tests/streams/scalar_test.cpp diff --git a/cpp/include/cudf/scalar/scalar.hpp b/cpp/include/cudf/scalar/scalar.hpp index 360dde11fc0..4bee369a123 100644 --- a/cpp/include/cudf/scalar/scalar.hpp +++ b/cpp/include/cudf/scalar/scalar.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2024, NVIDIA CORPORATION. + * Copyright (c) 2019-2025, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -176,11 +176,6 @@ class fixed_width_scalar : public scalar { */ void set_value(T value, rmm::cuda_stream_view stream = cudf::get_default_stream()); - /** - * @brief Explicit conversion operator to get the value of the scalar on the host. - */ - explicit operator value_type() const; - /** * @brief Get the value of the scalar. * @@ -402,11 +397,6 @@ class fixed_point_scalar : public scalar { [[nodiscard]] T fixed_point_value( rmm::cuda_stream_view stream = cudf::get_default_stream()) const; - /** - * @brief Explicit conversion operator to get the value of the scalar on the host. - */ - explicit operator value_type() const; - /** * @brief Returns a raw pointer to the value in device memory. * @return A raw pointer to the value in device memory @@ -515,11 +505,6 @@ class string_scalar : public scalar { rmm::cuda_stream_view stream = cudf::get_default_stream(), rmm::device_async_resource_ref mr = cudf::get_current_device_resource_ref()); - /** - * @brief Explicit conversion operator to get the value of the scalar in a host std::string. - */ - explicit operator std::string() const; - /** * @brief Get the value of the scalar in a host std::string. * diff --git a/cpp/src/scalar/scalar.cpp b/cpp/src/scalar/scalar.cpp index 4b0b08fe251..03233db6970 100644 --- a/cpp/src/scalar/scalar.cpp +++ b/cpp/src/scalar/scalar.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2024, NVIDIA CORPORATION. + * Copyright (c) 2019-2025, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -110,8 +110,6 @@ size_type string_scalar::size() const { return _data.size(); } char const* string_scalar::data() const { return static_cast(_data.data()); } -string_scalar::operator std::string() const { return this->to_string(cudf::get_default_stream()); } - std::string string_scalar::to_string(rmm::cuda_stream_view stream) const { std::string result(size(), '\0'); @@ -183,12 +181,6 @@ T fixed_point_scalar::fixed_point_value(rmm::cuda_stream_view stream) const numeric::scaled_integer{_data.value(stream), numeric::scale_type{type().scale()}}}; } -template -fixed_point_scalar::operator value_type() const -{ - return this->fixed_point_value(cudf::get_default_stream()); -} - template typename fixed_point_scalar::rep_type* fixed_point_scalar::data() { @@ -266,12 +258,6 @@ T const* fixed_width_scalar::data() const return _data.data(); } -template -fixed_width_scalar::operator value_type() const -{ - return this->value(cudf::get_default_stream()); -} - /** * @brief These define the valid fixed-width scalar types. * diff --git a/cpp/tests/CMakeLists.txt b/cpp/tests/CMakeLists.txt index 4451f6b64c5..e031597ed18 100644 --- a/cpp/tests/CMakeLists.txt +++ b/cpp/tests/CMakeLists.txt @@ -725,6 +725,7 @@ ConfigureTest(STREAM_REPLACE_TEST streams/replace_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_RESHAPE_TEST streams/reshape_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_ROLLING_TEST streams/rolling_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_ROUND_TEST streams/round_test.cpp STREAM_MODE testing) +ConfigureTest(STREAM_SCALAR_TEST streams/scalar_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_SEARCH_TEST streams/search_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_SORTING_TEST streams/sorting_test.cpp STREAM_MODE testing) ConfigureTest(STREAM_STREAM_COMPACTION_TEST streams/stream_compaction_test.cpp STREAM_MODE testing) diff --git a/cpp/tests/binaryop/assert-binops.h b/cpp/tests/binaryop/assert-binops.h index 6933d15d508..887b1b25436 100644 --- a/cpp/tests/binaryop/assert-binops.h +++ b/cpp/tests/binaryop/assert-binops.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2023, NVIDIA CORPORATION. + * Copyright (c) 2019-2025, NVIDIA CORPORATION. * * Copyright 2018-2019 BlazingDB, Inc. * Copyright 2018 Christian Noboa Mardini @@ -69,6 +69,19 @@ struct NearEqualComparator { } }; +template +TypeLhs scalar_host_value(cudf::scalar const& lhs) +{ + auto sclr = static_cast(lhs); + auto stream = cudf::get_default_stream(); + if constexpr (std::is_same_v) + return sclr.to_string(stream); + else if constexpr (std::is_same_v>) + return sclr.fixed_point_value(stream); + else + return sclr.value(stream); +} + template (lhs).operator TypeLhs(); + auto lhs_h = scalar_host_value(lhs); auto rhs_h = cudf::test::to_host(rhs); auto rhs_data = rhs_h.first; auto out_h = cudf::test::to_host(out); @@ -129,7 +142,7 @@ void ASSERT_BINOP(cudf::column_view const& out, TypeOp&& op, ValueComparator const& value_comparator = ValueComparator()) { - auto rhs_h = static_cast(rhs).operator TypeRhs(); + auto rhs_h = scalar_host_value(rhs); auto lhs_h = cudf::test::to_host(lhs); auto lhs_data = lhs_h.first; auto out_h = cudf::test::to_host(out); diff --git a/cpp/tests/streams/scalar_test.cpp b/cpp/tests/streams/scalar_test.cpp new file mode 100644 index 00000000000..635bedbbf6b --- /dev/null +++ b/cpp/tests/streams/scalar_test.cpp @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2023-2025, NVIDIA CORPORATION. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include + +#include + +template +struct TypedScalarTest : public cudf::test::BaseFixture {}; + +TYPED_TEST_SUITE(TypedScalarTest, cudf::test::FixedWidthTypes); + +TYPED_TEST(TypedScalarTest, DefaultValidity) +{ + using Type = cudf::device_storage_type_t; + Type value = static_cast(cudf::test::make_type_param_scalar(7)); + cudf::scalar_type_t s(value, true, cudf::test::get_default_stream()); + EXPECT_EQ(value, s.value(cudf::test::get_default_stream())); +} + +struct StringScalarTest : public cudf::test::BaseFixture {}; + +TEST_F(StringScalarTest, DefaultValidity) +{ + std::string value = "test string"; + auto s = cudf::string_scalar(value, true, cudf::test::get_default_stream()); + EXPECT_EQ(value, s.to_string(cudf::test::get_default_stream())); +} From 49d3129da877bf04a261ae91dac3a17755273dae Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Fri, 24 Jan 2025 02:48:49 +0000 Subject: [PATCH 07/11] fixing bad merge --- cpp/include/cudf/scalar/scalar.hpp | 2 +- cpp/tests/binaryop/assert-binops.h | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/cpp/include/cudf/scalar/scalar.hpp b/cpp/include/cudf/scalar/scalar.hpp index b4d05327b63..4bee369a123 100644 --- a/cpp/include/cudf/scalar/scalar.hpp +++ b/cpp/include/cudf/scalar/scalar.hpp @@ -381,7 +381,7 @@ class fixed_point_scalar : public scalar { rmm::device_async_resource_ref mr = cudf::get_current_device_resource_ref()); /** - * @brief Get the unscaled value of the scalar. + * @brief Get the value of the scalar. * * @param stream CUDA stream used for device memory operations. * @return The value of the scalar diff --git a/cpp/tests/binaryop/assert-binops.h b/cpp/tests/binaryop/assert-binops.h index 39544401085..887b1b25436 100644 --- a/cpp/tests/binaryop/assert-binops.h +++ b/cpp/tests/binaryop/assert-binops.h @@ -147,14 +147,6 @@ void ASSERT_BINOP(cudf::column_view const& out, auto lhs_data = lhs_h.first; auto out_h = cudf::test::to_host(out); auto out_data = out_h.first; - TypeRhs rhs_h; - if constexpr (std::is_same_v>) { - auto sv = static_cast(rhs).value(cudf::get_default_stream()); - rhs_h = std::string(sv.begin(), sv.end()); - } - else { - rhs_h = static_cast(rhs).value(cudf::get_default_stream()); - } ASSERT_EQ(out_data.size(), lhs_data.size()); for (size_t i = 0; i < out_data.size(); ++i) { From a17e5a5a3f69194e556960118febcfe9287f3c7e Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Fri, 24 Jan 2025 03:03:36 +0000 Subject: [PATCH 08/11] formatting --- cpp/src/scalar/scalar.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cpp/src/scalar/scalar.cpp b/cpp/src/scalar/scalar.cpp index 9949ce5038e..9bd05e798ce 100644 --- a/cpp/src/scalar/scalar.cpp +++ b/cpp/src/scalar/scalar.cpp @@ -181,10 +181,7 @@ T fixed_point_scalar::fixed_point_value(rmm::cuda_stream_view stream) const numeric::scaled_integer{_data.value(stream), numeric::scale_type{type().scale()}}}; } -typename fixed_point_scalar::rep_type* fixed_point_scalar::data() -{ - return _data.data(); -} +typename fixed_point_scalar::rep_type* fixed_point_scalar::data() { return _data.data(); } template typename fixed_point_scalar::rep_type const* fixed_point_scalar::data() const From 95d856d74746a6ce8a891943cce789500403fd40 Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Fri, 24 Jan 2025 20:03:43 +0000 Subject: [PATCH 09/11] small fix --- cpp/src/scalar/scalar.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/src/scalar/scalar.cpp b/cpp/src/scalar/scalar.cpp index 9bd05e798ce..03233db6970 100644 --- a/cpp/src/scalar/scalar.cpp +++ b/cpp/src/scalar/scalar.cpp @@ -181,7 +181,11 @@ T fixed_point_scalar::fixed_point_value(rmm::cuda_stream_view stream) const numeric::scaled_integer{_data.value(stream), numeric::scale_type{type().scale()}}}; } -typename fixed_point_scalar::rep_type* fixed_point_scalar::data() { return _data.data(); } +template +typename fixed_point_scalar::rep_type* fixed_point_scalar::data() +{ + return _data.data(); +} template typename fixed_point_scalar::rep_type const* fixed_point_scalar::data() const From 8224686fb4b6b83ca0a59ccb08736a85721e6f0b Mon Sep 17 00:00:00 2001 From: Shruti Shivakumar Date: Mon, 27 Jan 2025 18:35:53 +0000 Subject: [PATCH 10/11] pr reviews --- cpp/tests/binaryop/assert-binops.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cpp/tests/binaryop/assert-binops.h b/cpp/tests/binaryop/assert-binops.h index 887b1b25436..ac37636d7d3 100644 --- a/cpp/tests/binaryop/assert-binops.h +++ b/cpp/tests/binaryop/assert-binops.h @@ -74,12 +74,13 @@ TypeLhs scalar_host_value(cudf::scalar const& lhs) { auto sclr = static_cast(lhs); auto stream = cudf::get_default_stream(); - if constexpr (std::is_same_v) + if constexpr (std::is_same_v) { return sclr.to_string(stream); - else if constexpr (std::is_same_v>) + } else if constexpr (std::is_same_v>) { return sclr.fixed_point_value(stream); - else + } else { return sclr.value(stream); + } } template Date: Mon, 27 Jan 2025 23:29:45 -0800 Subject: [PATCH 11/11] Update cpp/tests/streams/scalar_test.cpp Co-authored-by: Vukasin Milovanovic --- cpp/tests/streams/scalar_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/tests/streams/scalar_test.cpp b/cpp/tests/streams/scalar_test.cpp index 3f37994f39f..a626afc8530 100644 --- a/cpp/tests/streams/scalar_test.cpp +++ b/cpp/tests/streams/scalar_test.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023-2025, NVIDIA CORPORATION. + * Copyright (c) 2025, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.