With `pcg32` and `pcg64`, the streams you select are discarded when you call either `seed()` or `seed(itype)`

imneme / pcg-cpp

PCG — C++ Implementation

Apache License 2.0

745 stars 99 forks source link

With pcg32 and pcg64, the streams you select are discarded when you call either seed() or seed(itype).

When the stream_mixin is specific_stream, functions seed() and seed(itype) have the—perhaps unintended—side effect of resetting the stream to (default_increment<itype>() >> 1). The function seed(itype) is the one which takes a seed of type itype.

This is a consequence of the placement-new technique used by the seeding routines.

Rather than directly updating engine state, which would involve "conditioning" the seed in the same way as the constructor, the PCG seeding routines construct a new engine, on top of the existing one, using "placement-new" syntax. The actual seeding is performed by the engine constructor.

The engine created when the user invokes either seed() or seed(itype) reinstalls the default stream, overwriting any stream that may have been selected by the user.

I view this as a bug, but perhaps it is intentional. Users, however, will be surprised, when they call a function to set the seed, and discover that the stream has changed.

If asked, I would recommend ditching the placement-new allocation, in favor of letting the seeding functions install the new seed "manually." This could be accomplished by isolating the "stirring/conditioning" of the seed in its own function.

Here is the one I am using:

    itype constexpr stir(itype const seed) noexcept
    {
        // This is where seeds are "conditioned." This function is 
        // called both by ctors and seeding routines.
        auto constexpr const three = static_cast<itype>(3u);
        return static_cast<itype> (
            this->is_mcg
            ? seed | three
            : (seed + increment()) * multiplier() + increment()
            );
    }

With this function in hand, the seeding routines are a breeze. The default argument below, allows a single function to replace both seed() and seed(itype).

public:
    static constexpr const itype default_seed
    {
        // Every random number engine in the C++ Standard Library
        // has a public static constant named default_seed.
        static_cast<itype>(0xcafef00dd15ea5e5ULL)
    };
    void constexpr seed(itype const seed = default_seed) noexcept
    {
        // This seeding function DOES NOT overwrite any stream that 
        // may have been selected by the user when the stream_mixin 
        // is specific_stream. The same function in O'Neill's 
        // program DOES overwrite the stream, resetting it to the 
        // value it was given during construction.
        state_ = stir(seed);
    }

A similar change to the constructor accomplishes the same thing, while making its code more readable.

    constexpr engine(itype const seed = default_seed) noexcept
        : state_{ stir(seed) }
    {}

// main.cpp #include <cassert> #include <iostream> #include "pcg_random.hpp" using state_type = pcg32::state_type; state_type arbitrary_stream() { // Choose an arbitrary stream which is different from // the stream established by the default constructor. pcg32 e; state_type const default_stream{ e.stream() }; state_type const clear_msb{ static_cast<state_type>(-1LL) >> 1 }; state_type const arbitrary_stream{ ~default_stream & clear_msb }; assert(default_stream != arbitrary_stream); e.set_stream(arbitrary_stream); assert(e.stream() == arbitrary_stream); return arbitrary_stream; } //---------------------------------------------------------------------- bool seed_default_resets_stream() { pcg32 e; state_type const default_stream{ e.stream() }; state_type const another_stream{ arbitrary_stream() }; assert(another_stream != default_stream); e.set_stream(another_stream); assert(e.stream() == another_stream); e.seed(); assert(e.stream() == default_stream); return e.stream() == default_stream; } //---------------------------------------------------------------------- bool seed_itype_resets_stream() { pcg32 e; state_type const default_stream{ e.stream() }; state_type const another_stream{ arbitrary_stream() }; assert(another_stream != default_stream); e.set_stream(another_stream); assert(e.stream() == another_stream); state_type const arbitrary_seed{ 1u }; e.seed(arbitrary_seed); assert(e.stream() == default_stream); return e.stream() == default_stream; } //---------------------------------------------------------------------- int main() { std::cout << "Test pcg_random issue 94" "\nhttps://github.com/imneme/pcg-cpp/issues/94" "\n" "\nGiven a `pcg32` engine named `e`, does calling `e.seed()` " "\nreset the stream to the value used by the default constructor? " "\n" "\n Answer: " << (seed_default_resets_stream() ? "yes" : "no") << "\n" "\n" "\nGiven a `pcg32` engine named `e`, and an arbitrary seed " "\nnamed `s`, does calling `e.seed(s)` reset the stream to " "\nthe value used by the default constructor? " "\n" "\n Answer: " << (seed_itype_resets_stream() ? "yes" : "no") << "\n\n"; } // end file: main.cpp

Test pcg_random issue 94 https://github.com/imneme/pcg-cpp/issues/94 Given a `pcg32` engine named `e`, does calling `e.seed()` reset the stream to the value used by the default constructor? Answer: yes Given a `pcg32` engine named `e`, and an arbitrary seed named `s`, does calling `e.seed(s)` reset the stream to the value used by the default constructor? Answer: yes

imneme / pcg-cpp

With `pcg32` and `pcg64`, the streams you select are discarded when you call either `seed()` or `seed(itype)` #94