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

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

Open
tbxfreeware opened this issue Sep 6, 2024 · 1 comment

Comments

@tbxfreeware
Copy link

tbxfreeware commented Sep 6, 2024

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) }
    {}
@tbxfreeware tbxfreeware changed the title With pcg32 and pcg64, the streams you select are ___discarded___ when you call either seed() or seed(itype) With pcg32 and pcg64, the streams you select are discarded when you call either seed() or seed(itype) Sep 6, 2024
@tbxfreeware
Copy link
Author

tbxfreeware commented Sep 6, 2024

Here is a short program you can run to verify that this bug does, indeed, exist.

// 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

Here is the output:

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

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

No branches or pull requests

1 participant