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

Add example for initializing a PCG RNG #1352

Merged
merged 4 commits into from
Dec 15, 2023
Merged

Conversation

vks
Copy link
Collaborator

@vks vks commented Oct 30, 2023

No description provided.

@vks
Copy link
Collaborator Author

vks commented Oct 30, 2023

@kornelski Does this address your concerns?

@vks vks changed the title Add example for initializing a PCG RNG (#1347) Add example for initializing a PCG RNG Oct 30, 2023
@vks
Copy link
Collaborator Author

vks commented Oct 30, 2023

Some tests fail:

error[E0432]: unresolved import `rand`
 --> rand_pcg/src/lib.rs:37:5
  |
5 | use rand::{SeedableRng, Rng};
  |     ^^^^ use of undeclared crate or module `rand`

error[E0599]: no function or associated item named `from_entropy` found for struct `Mcg128Xsl64` in the current scope
 --> rand_pcg/src/lib.rs:40:25
  |
8 | let mut rng = Pcg64Mcg::from_entropy();
  |                         ^^^^^^^^^^^^ function or associated item not found in `Mcg128Xsl64`

Not sure what the best fix is. We could use rand_core and seed_from_u64 instead.

@kornelski
Copy link

Yes, thank you!

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error[E0432]: unresolved import rand

Indeed, this makes examples in RNG crates a pain.

For this RNG I think we should mention seed_from_u64 anyway since its main use-cases probably involve reproducible seeding. We can include a comment like:

let mut rng = Pcg64Mcg::seed_from_u64(0);
// Alternative: Pcg64Mcg::from_entropy();

@newpavlov
Copy link
Member

Indeed, this makes examples in RNG crates a pain.

I think it can be fixed by adding rand as a dev dependency.

@vks
Copy link
Collaborator Author

vks commented Nov 27, 2023

@newpavlov Would you prefer this to the current solution (seed_from_u64 and next_u64)?

@newpavlov
Copy link
Member

I am fine with either, but the docs probably should mention that the crate functionality is implemented using traits from the rand_core trait and extended further by the rand crate.

@vks
Copy link
Collaborator Author

vks commented Dec 14, 2023

@newpavlov I added an example for using rand (adding it as a dev dependency).

@vks vks requested a review from newpavlov December 14, 2023 21:06
@vks vks merged commit 3c2e82f into rust-random:master Dec 15, 2023
10 of 12 checks passed
@vks vks deleted the pcg-example branch December 15, 2023 07:32
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.

4 participants