From ac18af5a4153765a4e3e631a094bcea4a1b91ff3 Mon Sep 17 00:00:00 2001 From: Louis Sugy Date: Wed, 14 Dec 2022 12:18:05 +0100 Subject: [PATCH 1/2] Use GenPC as the default random number generator in make_blobs, make_regression, etc --- cpp/include/raft/cluster/kmeans_types.hpp | 3 +-- .../raft/random/detail/make_regression.cuh | 2 +- .../raft/random/detail/rng_impl_deprecated.cuh | 2 +- cpp/include/raft/random/make_blobs.cuh | 4 ++-- cpp/include/raft/random/make_regression.cuh | 4 ++-- cpp/include/raft/random/rng.cuh | 2 +- cpp/test/linalg/reduce_rows_by_key.cu | 2 +- cpp/test/random/make_regression.cu | 6 ++++++ cpp/test/random/rng_discrete.cu | 16 ++++++++-------- 9 files changed, 23 insertions(+), 18 deletions(-) diff --git a/cpp/include/raft/cluster/kmeans_types.hpp b/cpp/include/raft/cluster/kmeans_types.hpp index f411b12b5c..b34f3320ad 100644 --- a/cpp/include/raft/cluster/kmeans_types.hpp +++ b/cpp/include/raft/cluster/kmeans_types.hpp @@ -75,8 +75,7 @@ struct KMeansParams { /** * Seed to the random number generator. */ - raft::random::RngState rng_state = - raft::random::RngState(0, raft::random::GeneratorType::GenPhilox); + raft::random::RngState rng_state{0}; /** * Metric to use for distance computation. diff --git a/cpp/include/raft/random/detail/make_regression.cuh b/cpp/include/raft/random/detail/make_regression.cuh index f06e20d4a6..cb0949c458 100644 --- a/cpp/include/raft/random/detail/make_regression.cuh +++ b/cpp/include/raft/random/detail/make_regression.cuh @@ -158,7 +158,7 @@ void make_regression_caller(const raft::handle_t& handle, DataT noise = (DataT)0.0, bool shuffle = true, uint64_t seed = 0ULL, - raft::random::GeneratorType type = raft::random::GenPhilox) + raft::random::GeneratorType type = raft::random::GenPC) { n_informative = std::min(n_informative, n_cols); diff --git a/cpp/include/raft/random/detail/rng_impl_deprecated.cuh b/cpp/include/raft/random/detail/rng_impl_deprecated.cuh index f9b55dd9d0..5c31026a89 100644 --- a/cpp/include/raft/random/detail/rng_impl_deprecated.cuh +++ b/cpp/include/raft/random/detail/rng_impl_deprecated.cuh @@ -39,7 +39,7 @@ namespace detail { class RngImpl { public: - RngImpl(uint64_t seed, GeneratorType _t = GenPhilox) + RngImpl(uint64_t seed, GeneratorType _t = GenPC) : state{seed, 0, _t}, type(_t), // simple heuristic to make sure all SMs will be occupied properly diff --git a/cpp/include/raft/random/make_blobs.cuh b/cpp/include/raft/random/make_blobs.cuh index ff1a20f58a..4f6ddaa2b2 100644 --- a/cpp/include/raft/random/make_blobs.cuh +++ b/cpp/include/raft/random/make_blobs.cuh @@ -74,7 +74,7 @@ void make_blobs(DataT* out, DataT center_box_min = (DataT)-10.0, DataT center_box_max = (DataT)10.0, uint64_t seed = 0ULL, - GeneratorType type = GenPhilox) + GeneratorType type = GenPC) { detail::make_blobs_caller(out, labels, @@ -140,7 +140,7 @@ void make_blobs( DataT center_box_min = (DataT)-10.0, DataT center_box_max = (DataT)10.0, uint64_t seed = 0ULL, - GeneratorType type = GenPhilox) + GeneratorType type = GenPC) { if (centers.has_value()) { RAFT_EXPECTS(centers.value().extent(0) == (IdxT)n_clusters, diff --git a/cpp/include/raft/random/make_regression.cuh b/cpp/include/raft/random/make_regression.cuh index a92d5bb12f..e203de4ade 100644 --- a/cpp/include/raft/random/make_regression.cuh +++ b/cpp/include/raft/random/make_regression.cuh @@ -82,7 +82,7 @@ void make_regression(const raft::handle_t& handle, DataT noise = (DataT)0.0, bool shuffle = true, uint64_t seed = 0ULL, - GeneratorType type = GenPhilox) + GeneratorType type = GenPC) { detail::make_regression_caller(handle, out, @@ -149,7 +149,7 @@ void make_regression(const raft::handle_t& handle, DataT noise = DataT{}, bool shuffle = true, uint64_t seed = 0ULL, - GeneratorType type = GenPhilox) + GeneratorType type = GenPC) { const auto n_samples = out.extent(0); assert(values.extent(0) == n_samples); diff --git a/cpp/include/raft/random/rng.cuh b/cpp/include/raft/random/rng.cuh index 9469c393f1..76e3ed1d3d 100644 --- a/cpp/include/raft/random/rng.cuh +++ b/cpp/include/raft/random/rng.cuh @@ -825,7 +825,7 @@ class DEPR Rng : public detail::RngImpl { * @param _t backend device RNG generator type * @note Refer to the `Rng::seed` method for details about seeding the engine */ - Rng(uint64_t _s, GeneratorType _t = GenPhilox) : detail::RngImpl(_s, _t) {} + Rng(uint64_t _s, GeneratorType _t = GenPC) : detail::RngImpl(_s, _t) {} /** * @brief Generates the 'a' and 'b' parameters for a modulo affine diff --git a/cpp/test/linalg/reduce_rows_by_key.cu b/cpp/test/linalg/reduce_rows_by_key.cu index 7b124cb7bb..97bf4802f2 100644 --- a/cpp/test/linalg/reduce_rows_by_key.cu +++ b/cpp/test/linalg/reduce_rows_by_key.cu @@ -112,7 +112,7 @@ class ReduceRowTest : public ::testing::TestWithParam> { rmm::device_uvector weight(0, stream); if (params.weighted) { weight.resize(nobs, stream); - raft::random::RngState r(params.seed, raft::random::GeneratorType::GenPhilox); + raft::random::RngState r(params.seed); uniform(handle, r, weight.data(), nobs, T(1), params.max_weight); } diff --git a/cpp/test/random/make_regression.cu b/cpp/test/random/make_regression.cu index 65d4c4cb31..dab261b980 100644 --- a/cpp/test/random/make_regression.cu +++ b/cpp/test/random/make_regression.cu @@ -127,6 +127,9 @@ class MakeRegressionTest : public ::testing::TestWithParam MakeRegressionTestF; const std::vector> inputsf_t = { + {0.01f, 256, 32, 16, 1, -1, 0.f, true, raft::random::GenPC, 1234ULL}, + {0.01f, 1000, 100, 47, 4, 65, 4.2f, true, raft::random::GenPC, 1234ULL}, + {0.01f, 20000, 500, 450, 13, -1, -3.f, false, raft::random::GenPC, 1234ULL}, {0.01f, 256, 32, 16, 1, -1, 0.f, true, raft::random::GenPhilox, 1234ULL}, {0.01f, 1000, 100, 47, 4, 65, 4.2f, true, raft::random::GenPhilox, 1234ULL}, {0.01f, 20000, 500, 450, 13, -1, -3.f, false, raft::random::GenPhilox, 1234ULL}}; @@ -147,6 +150,9 @@ INSTANTIATE_TEST_CASE_P(MakeRegressionTests, MakeRegressionTestF, ::testing::Val typedef MakeRegressionTest MakeRegressionTestD; const std::vector> inputsd_t = { + {0.01, 256, 32, 16, 1, -1, 0.0, true, raft::random::GenPC, 1234ULL}, + {0.01, 1000, 100, 47, 4, 65, 4.2, true, raft::random::GenPC, 1234ULL}, + {0.01, 20000, 500, 450, 13, -1, -3.0, false, raft::random::GenPC, 1234ULL}, {0.01, 256, 32, 16, 1, -1, 0.0, true, raft::random::GenPhilox, 1234ULL}, {0.01, 1000, 100, 47, 4, 65, 4.2, true, raft::random::GenPhilox, 1234ULL}, {0.01, 20000, 500, 450, 13, -1, -3.0, false, raft::random::GenPhilox, 1234ULL}}; diff --git a/cpp/test/random/rng_discrete.cu b/cpp/test/random/rng_discrete.cu index b7aef51af5..06e35061ce 100644 --- a/cpp/test/random/rng_discrete.cu +++ b/cpp/test/random/rng_discrete.cu @@ -178,16 +178,16 @@ class RngDiscreteTest : public ::testing::TestWithParam> }; const std::vector> inputs_i32 = { - {1, 10000, 5, 5, GenPhilox, 123ULL}, - {1, 10000, 10, 7, GenPhilox, 456ULL}, - {1000, 100, 10000, 20, GenPhilox, 123ULL}, - {1, 10000, 5, 5, GenPC, 1234ULL}, + {1, 10000, 5, 5, GenPC, 123ULL}, + {1, 10000, 10, 7, GenPC, 456ULL}, + {1000, 100, 10000, 20, GenPC, 123ULL}, + {1, 10000, 5, 5, GenPhilox, 1234ULL}, }; const std::vector> inputs_i64 = { - {1, 10000, 5, 5, GenPhilox, 123ULL}, - {1, 10000, 10, 7, GenPhilox, 456ULL}, - {1000, 100, 10000, 20, GenPhilox, 123ULL}, - {1, 10000, 5, 5, GenPC, 1234ULL}, + {1, 10000, 5, 5, GenPC, 123ULL}, + {1, 10000, 10, 7, GenPC, 456ULL}, + {1000, 100, 10000, 20, GenPC, 123ULL}, + {1, 10000, 5, 5, GenPhilox, 1234ULL}, }; #define RNG_DISCRETE_TEST(test_type, test_name, test_inputs) \ From fc24d6f1e951548504efc5ecef6c14e6490d3f70 Mon Sep 17 00:00:00 2001 From: Louis Sugy Date: Wed, 14 Dec 2022 13:39:08 +0100 Subject: [PATCH 2/2] Revert change on the deprecated APIs --- cpp/include/raft/random/detail/rng_impl_deprecated.cuh | 2 +- cpp/include/raft/random/rng.cuh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/include/raft/random/detail/rng_impl_deprecated.cuh b/cpp/include/raft/random/detail/rng_impl_deprecated.cuh index 5c31026a89..f9b55dd9d0 100644 --- a/cpp/include/raft/random/detail/rng_impl_deprecated.cuh +++ b/cpp/include/raft/random/detail/rng_impl_deprecated.cuh @@ -39,7 +39,7 @@ namespace detail { class RngImpl { public: - RngImpl(uint64_t seed, GeneratorType _t = GenPC) + RngImpl(uint64_t seed, GeneratorType _t = GenPhilox) : state{seed, 0, _t}, type(_t), // simple heuristic to make sure all SMs will be occupied properly diff --git a/cpp/include/raft/random/rng.cuh b/cpp/include/raft/random/rng.cuh index 76e3ed1d3d..9469c393f1 100644 --- a/cpp/include/raft/random/rng.cuh +++ b/cpp/include/raft/random/rng.cuh @@ -825,7 +825,7 @@ class DEPR Rng : public detail::RngImpl { * @param _t backend device RNG generator type * @note Refer to the `Rng::seed` method for details about seeding the engine */ - Rng(uint64_t _s, GeneratorType _t = GenPC) : detail::RngImpl(_s, _t) {} + Rng(uint64_t _s, GeneratorType _t = GenPhilox) : detail::RngImpl(_s, _t) {} /** * @brief Generates the 'a' and 'b' parameters for a modulo affine