Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use GenPC (Permuted Congruential) as the default random number generator everywhere #1099

Merged
merged 2 commits into from
Dec 15, 2022

Conversation

Nyrio
Copy link
Contributor

@Nyrio Nyrio commented Dec 14, 2022

GenPC is already the default generator used by RngState when not passed to the constructor:

/**
* The generator type. PCGenerator has been extensively tested and is faster
* than Philox, thus we use it as the default.
*/
GeneratorType type{GeneratorType::GenPC};

But the default value for the GeneratorType remained GenPhilox in some prims, such as make_blobs and make_regression. PC is faster and more reliable, so it should be the default there too.

Also, whenever possible, the generator type shouldn't be hardcoded, it should either use the default (construct RngState with the seed only) or an argument (as is the case with make_blobs and make_regression).

Note: this will effectively modify many test inputs, so be aware of that when comparing results prior to and following the change.

@Nyrio Nyrio added 3 - Ready for Review improvement Improvement / enhancement to an existing function breaking Breaking change cpp labels Dec 14, 2022
@Nyrio Nyrio requested a review from a team as a code owner December 14, 2022 11:34
@cjnolet
Copy link
Member

cjnolet commented Dec 15, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 489aa9a into rapidsai:branch-23.02 Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review breaking Breaking change cpp improvement Improvement / enhancement to an existing function
Projects
Development

Successfully merging this pull request may close these issues.

3 participants