2

What is wrong with my constructor? Every time I call a function (about once every five seconds) that is supposed to generate random numbers, it generates the same numbers. Each call instantiates one of these objects below. I thought I was seeding m_gen randomly with the output of m_rd's operator() call.

Could I pass in the result of m_rd() to the constructor? What would the signature be? Shuffler(std::random device& rd)? But then that would be more difficult for the user.

Edit:

Actually, if it's possible, I would prefer a solution where you don't need to pass anything into the constructor.

shuffler.h

#include <random>

class Shuffler
{
private:
    std::random_device m_rd;
    std::mt19937 m_gen;
public:

    //! The default constructor. 
    Shuffler();

};

shuffler.cpp

#include "shuffler.h"

Shuffler::Shuffler() : m_gen(m_rd())
{
}
Taylor
  • 1,797
  • 4
  • 26
  • 51
  • anything at https://stackoverflow.com/questions/12458383/java-random-numbers-using-a-seed useful? – Peter David Carter Jun 20 '17 at 22:01
  • 1
    A random_device is [not](http://www.pcg-random.org/posts/cpps-random_device.html) a Seed Sequence. – David Schwartz Jun 20 '17 at 22:05
  • Are you using MinGW, by chance? I believe `std::random_device` does not have a useful implementation with that compiler. See https://stackoverflow.com/q/18880654/10077 – Fred Larson Jun 20 '17 at 22:11
  • 1
    If `random_device` is a pseudo-random generator that always returns the same sequence, you'll get the same value every time because you create a new instance every time. If you make `m_rd` static you could get around that problem. – Mark Ransom Jun 20 '17 at 22:37

2 Answers2

4

std::random_device is usually fine for this sort of thing, but it may not be on every platform. While most platforms' standard libraries implement it in terms of some underlying OS random functionality (i.e. /dev/urandom on Linux or CryptGenRandom on Windows), it is not required to do so by the C++ standard. On some platforms, high-quality random generators simply may not be available, and the standard allows std::random_device to be a simple, statically seeded PRNG. If it is, every std::random_device object will generate the same sequence of numbers.

For those reasons, you may want to go back to simple time-seeding. The standard provides std::chrono::high_resolution_clock:

class Shuffler
{
private:
    std::mt19937 m_gen;
public:
    Shuffler()
        : m_gen{static_cast<std::uint32_t>(
              std::chrono::high_resolution_clock::now().time_since_epoch().count()
          )}
    {}
};

std::chrono::high_resolution_clock usually has a resolution of nanoseconds or hundreds of nanoseconds. This is high enough that two PRNGs seeded by calls to the high_resolution_clock are very unlikely to end up using the same seed. This is also not guaranteed though. For example, std::chrono::high_resolution_clock only has microsecond resolution on macOS, which may or may not be good enough for your purposes.

In the end, neither method is perfect. You may want to combine the two using std::seed_seq:

std::seed_seq make_seeds() {
    thread_local std::random_device rd;
    return {{
        static_cast<std::uint32_t>(std::chrono::high_resolution_clock::now().time_since_epoch().count()),
        rd()
    }};
}

// Cast away rvalue-ness because the standard random generators need
// an lvalue reference to their seed_seq for some strange reason
template <typename T>
T& identity(T&& t) { return t; }

class Shuffler
{
private:
    std::mt19937 m_gen;
public:
    Shuffler()
        : m_gen{identity(make_seeds())}
    {}
};

As you can see, this is getting far from simple, and it's still not perfect. See these blog posts for more information about seeding and random number generators then you ever thought you wanted.

Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
0

As in this example, you have to seed it and random_device doesn't seem to do the trick*:

// do this once somewhere
unsigned seed = std::chrono::system_clock::now().time_since_epoch().count();


class Shuffler
{
private:
    std::mt19937 m_gen;
public:
    Shuffler()  : m_gen(seed) {}

};

*As stated here, random_device is not a Seed Sequence!

gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • `std::random_device` usually pulls randomness from the OS (generally /dev/urandom on *nix or CryptGenRandom on Windows). It should be fine (if slightly sub-optimal) for seeding a PRNG. – Miles Budnek Jun 20 '17 at 22:08
  • Does this solve the problem where every time I make a new object, it has the same seed? And if it does, why doesn't my way work? I read the link above, and it says c++11 does not "allow" us to write `std::mt19937 engine{std::random_device{}};` but my way, which is similar I think, compiles and runs. Maybe we are equivocating on "allow." Also, I would prefer not to have to code things outside of the constructor. – Taylor Jun 20 '17 at 22:16
  • @Taylor What you wrote basically does `std::mt19937 engine{std::random_device{}()};` (note the extra set of `()`). That should work on most platforms, but using a high-resolution clock time for your seed may be more portable since `std::random_device` is allowed to be deterministic. – Miles Budnek Jun 20 '17 at 22:19
  • @MilesBudnek when you say `std::random_device` is allowed to be deterministic, you're saying that some systems, when instantiating `m_rd` then calling `m_rd()`, and doing this over and over again with about five seconds in between, will produce the same numbers? And for this solution, why can't we do `unsigned seed = std::chrono::system_clock::now().time_since_epoch().count();` inside of the constructor? – Taylor Jun 20 '17 at 22:29
  • 1
    @Taylor It could. It usually won't on systems that have a good source of randomness (like /dev/urandom or CryptGenRandom), but the standard allows it to be a PRNG with a fixed seed. If it is, every `std::random_device` object will produce the same sequence of numbers. Using `std::chrono::high_resolution_clock::now().time_since_epoch().count()` in the constructor will work. – Miles Budnek Jun 20 '17 at 22:34
  • @MilesBudnek how do I get `std::chrono::high_resolution_clock::now().time_since_epoch()‌​.count()` to work in the constructor? I get an error like `no match for call to '(std::mt19937`... How do I cast the result of `count()` to something nice? – Taylor Jun 20 '17 at 23:19
  • @Taylor I went ahead and wrote up an answer since this comment thread was getting long and unwieldy. – Miles Budnek Jun 21 '17 at 00:23