Add rand_pcg crate #4

dhardy · 2018-09-27T14:10:26Z

This is based off of pitdicker's small-rngs crate and O'Neill's pcg-cpp lib.

I took some liberty to come up with new names for these RNGs.

One small niggle: the initialisation of the MCG variant forces one or two bits high, depending on the source. I copied the C version here.

@vks review?

@imneme for your information, the plan is to publish this as the rand_pcg Rust crate

dhardy · 2018-09-27T14:20:42Z

To fix:

Upgrade minimum supported version to Rust 1.26 because we need u128 — or should we instead hide Pcg64Mcg behind a CFG? Unfortunately this affects the main Rand crate too because we want to use it in SmallRng.
cargo test --package rand_pcg --features serde1 applies the feature to the wrong crate!

This is based off of pitdicker's small-rngs crate and O'Neill's pcg-cpp lib.

dhardy · 2018-10-01T10:29:07Z

See rust-lang/cargo#5364 regarding the latter issue. We can work around with -Z package-features for now, though this might cause further breakage later. Incidentally, several of our other build scripts probably get this wrong and don't do tests they should be doing.

Regarding the first item, upgrading the minimum Rust version, I'd really appreciate some feedback. We could use a build-script to auto-enable Pcg64Mcg only on compatible compiler versions, but this may be a little surprising.

Nemo157 · 2018-10-01T11:38:27Z

@dhardy you can use cargo test --manifest-path rand_pcg/Cargo.toml --package rand_pcg --features serde1 to workaround the cargo bug (and that should be guaranteed to keep working no matter what changes happen to fix it).

See Cargo issue 5364

dhardy · 2018-10-01T12:25:30Z

Thanks @Nemo157. Any idea how I can enable dependency features from a build script?

The following allows the RNG using u128 to be hidden behind a feature flag:

    if version().unwrap() >= Version::parse("1.26.0").unwrap() {
        println!("cargo:rustc-cfg=rust_1_26");

... but using this plus the serde1 feature requires that bincode/i128 be enabled. There's no documented way of doing that from the build script, and adding an i128 = ["bincode/i128"] to the features section of this Cargo.toml (plus changing the feature name above) doesn't help either.

So apparently auto-detection of the compiler version cannot support Serde for both old compilers and newer ones.

Unfortunately this does not support serde1 on old compilers, since build scripts do not appear to be able to specify feature requirements of dependencies.

coltfred

To me it seems like the implementations are faithful to their C counterparts and are proven such via the tests. Beyond that I had some comments as a fairly new person to the rand package and random number generation algorithms in general.

rand_pcg/Cargo.toml

coltfred · 2018-10-01T17:56:14Z

rand_pcg/src/pcg64.rs

+/// Congruential Generator, and 32-bit output via "xorshift high (bits),
+/// random rotation" output function.
+/// 
+/// This is equivalent to `pcg_engines::setseq_xsh_rr_64_32`, also known as


Would it be a good idea to link to that here? At very least saying what repo that came from might be a good idea.

Where do you think we should link to? It seems to be loosely documented in the C and C++ sources.

coltfred · 2018-10-01T17:56:36Z

rand_pcg/src/pcg128.rs

+/// Congruential Generator, and 64-bit output via "xorshift low (bits),
+/// random rotation" output function.
+/// 
+/// This is equivalent to `pcg_engines::mcg_xsl_rr_128_64`. Note that we


Would it be a good idea to link to that here? At very least saying what repo that came from might be a good idea.

dhardy · 2018-10-02T00:23:48Z

Thanks for the review @coltfred!

Nemo157 · 2018-10-02T08:18:34Z

Any idea how I can enable dependency features from a build script?

As far as I'm aware not possible, if you want conditional i128 support you'll have to have a feature for it (and could use a version detecting build script to give a better error message if it's not supported on the current Rust version).

dhardy · 2018-10-02T09:08:53Z

if you want conditional i128 support you'll have to have a feature for it

I tried, but it seems build scripts can't enable features either. I think in this case it's better just to say Serde support requires a compiler >= 1.26 rather than add an explicit feature for users to enable.

Nemo157 · 2018-10-02T09:40:37Z

it seems build scripts can't enable features either.

Yeah, I meant a feature that users of the crate would have to enable if they want the support (I haven't looked into what you're actually using the feature for, there's definitely a large space of conditional compilation around feature unions—especially with optional dependencies—that isn't supported well be Cargo at the moment).

Note that until rand_core 0.2 and 0.3 are compatible or rand 0.5 uses rand_core 0.3, you need to force rand_core to 0.2 to run these.

dhardy · 2018-10-02T13:43:14Z

Benchmarks (including Pcg32, Pcg64 and Pcg64Fast from @robojeb's pcg_rand crate):

test gen_bytes_lcg64_xsh32  ... bench:     365,229 ns/iter (+/- 9,274) = 2803 MB/s
test gen_bytes_mcg128_xsh64 ... bench:     224,287 ns/iter (+/- 59,182) = 4565 MB/s
test gen_bytes_pcg32        ... bench:     526,371 ns/iter (+/- 32,382) = 1945 MB/s
test gen_bytes_pcg64        ... bench:     297,365 ns/iter (+/- 14,789) = 3443 MB/s
test gen_bytes_pcg64fast    ... bench:     267,306 ns/iter (+/- 15,694) = 3830 MB/s
test gen_bytes_small        ... bench:     287,545 ns/iter (+/- 16,333) = 3561 MB/s
test gen_bytes_std          ... bench:     389,185 ns/iter (+/- 26,493) = 2631 MB/s
test gen_bytes_xorshift     ... bench:     317,449 ns/iter (+/- 6,196) = 3225 MB/s
test gen_u32_lcg64_xsh32    ... bench:       1,451 ns/iter (+/- 60) = 2756 MB/s
test gen_u32_mcg128_xsh64   ... bench:       1,605 ns/iter (+/- 24) = 2492 MB/s
test gen_u32_pcg32          ... bench:       1,448 ns/iter (+/- 64) = 2762 MB/s
test gen_u32_pcg64          ... bench:       2,287 ns/iter (+/- 39) = 1749 MB/s
test gen_u32_pcg64fast      ... bench:       2,037 ns/iter (+/- 251) = 1963 MB/s
test gen_u32_small          ... bench:       1,169 ns/iter (+/- 21) = 3421 MB/s
test gen_u32_std            ... bench:       2,237 ns/iter (+/- 204) = 1788 MB/s
test gen_u32_xorshift       ... bench:       1,176 ns/iter (+/- 74) = 3401 MB/s
test gen_u64_lcg64_xsh32    ... bench:       2,965 ns/iter (+/- 267) = 2698 MB/s
test gen_u64_mcg128_xsh64   ... bench:       1,688 ns/iter (+/- 213) = 4739 MB/s
test gen_u64_pcg32          ... bench:       2,921 ns/iter (+/- 55) = 2738 MB/s
test gen_u64_pcg64          ... bench:       2,283 ns/iter (+/- 101) = 3504 MB/s
test gen_u64_pcg64fast      ... bench:       2,012 ns/iter (+/- 104) = 3976 MB/s
test gen_u64_small          ... bench:       1,796 ns/iter (+/- 106) = 4454 MB/s
test gen_u64_std            ... bench:       3,435 ns/iter (+/- 209) = 2328 MB/s
test gen_u64_xorshift       ... bench:       1,741 ns/iter (+/- 78) = 4595 MB/s
test init_lcg64_xsh32       ... bench:          15 ns/iter (+/- 0)
test init_mcg128_xsh64      ... bench:          14 ns/iter (+/- 0)
test init_small             ... bench:          15 ns/iter (+/- 0)
test init_std               ... bench:       4,677 ns/iter (+/- 309)
test init_xorshift          ... bench:          13 ns/iter (+/- 0)

dhardy · 2018-10-02T13:47:16Z

rust-random/rand#619 should fix the failing test (when I figure it out).

This requires use of the rand_core 0.2.2 shim

dhardy mentioned this pull request Sep 27, 2018

SmallRng algorithm rust-random/rand#603

Closed

Add rand_pcg crate

72efdea

This is based off of pitdicker's small-rngs crate and O'Neill's pcg-cpp lib.

Fix CI tests (Cargo workaround)

ec78857

See Cargo issue 5364

dhardy force-pushed the pcg branch from 8875af5 to ec78857 Compare October 1, 2018 11:58

Auto-enable u128-using RNG on newer compilers

f2116a7

Unfortunately this does not support serde1 on old compilers, since build scripts do not appear to be able to specify feature requirements of dependencies.

coltfred reviewed Oct 1, 2018

View reviewed changes

dhardy added 3 commits October 2, 2018 11:22

rand_pcg: tweak documentation

1247d23

Add benchmarks for sub-crates

d52e4df

Note that until rand_core 0.2 and 0.3 are compatible or rand 0.5 uses rand_core 0.3, you need to force rand_core to 0.2 to run these.

Optimise fill_bytes for PCG generators

3762ccf

Update to rand_core 0.3

4a5a54f

This requires use of the rand_core 0.2.2 shim

dhardy merged commit a6ff305 into rust-random:master Oct 3, 2018

dhardy deleted the pcg branch October 3, 2018 10:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rand_pcg crate #4

Add rand_pcg crate #4

dhardy commented Sep 27, 2018

dhardy commented Sep 27, 2018

dhardy commented Oct 1, 2018

Nemo157 commented Oct 1, 2018

dhardy commented Oct 1, 2018

coltfred left a comment

coltfred Oct 1, 2018

dhardy Oct 2, 2018

coltfred Oct 1, 2018

dhardy commented Oct 2, 2018

Nemo157 commented Oct 2, 2018

dhardy commented Oct 2, 2018

Nemo157 commented Oct 2, 2018

dhardy commented Oct 2, 2018 •

edited

Loading

dhardy commented Oct 2, 2018

Add rand_pcg crate #4

Add rand_pcg crate #4

Conversation

dhardy commented Sep 27, 2018

dhardy commented Sep 27, 2018

dhardy commented Oct 1, 2018

Nemo157 commented Oct 1, 2018

dhardy commented Oct 1, 2018

coltfred left a comment

Choose a reason for hiding this comment

coltfred Oct 1, 2018

Choose a reason for hiding this comment

dhardy Oct 2, 2018

Choose a reason for hiding this comment

coltfred Oct 1, 2018

Choose a reason for hiding this comment

dhardy commented Oct 2, 2018

Nemo157 commented Oct 2, 2018

dhardy commented Oct 2, 2018

Nemo157 commented Oct 2, 2018

dhardy commented Oct 2, 2018 • edited Loading

dhardy commented Oct 2, 2018

dhardy commented Oct 2, 2018 •

edited

Loading