static random distribution does not update the intervals

[niccolot]

I' ve just realized that one of the suggested way to generate random numbers, namely

inline double random_double(double min = 0, double max = 1) {
    /**
     * @brief returns a double in the range [min, max)
     */
    static std::uniform_real_distribution<double> distribution(min, max); // should not be static
    static std::mt19937 generator;

    return distribution(generator);
}

produces numbers only in the first (min,max) range it is instanciated with, e.g.

int main()
{
    for (int i=0; i<5; ++i) {
        std::cout << random_double() << std::endl;
    }
    std::cout << "\n";
    for (int i=0; i<5; ++i) {
        std::cout << random_double(1,2) << std::endl;
    }
}

prints

0.135477
0.835009
0.968868
0.221034
0.308167

0.547221
0.188382
0.992881
0.996461
0.967695

I think is kind of a coincidence that everything works regardless, the random_double() is always used with (0,1), the only problem could arise in

Vec3 HittableList::random(const Vec3& origin) const {
    auto int_size = int(objects.size());
    auto vec = objects[random_int(0, int_size-1)]->random(origin);

// rest

but i think it would not produce any particularly visible artifact.

[hollasch]

Wow, good catch!

[hollasch]

This should be an issue. Converting.

[ssstroke]

It is interesting to note here that it affects the distribution of spheres and their materials in the final scene of the first book and first scene of the second book. My renders always looked different compared to renders presented in the book, i.e. I always got lots of metal balls and they were always purple. This helped me a lot, thanks.
But the book's renders were last updated 5 years ago at the time of my writing, so it does not make sense. Perhaps 5 years ago random numbers were being generated another way?

[dimitry-ishenko]

For anyone who comes across this issue, there is a subtle bug in @niccolot's implementation in the first post.

If you combine both random_double() functions using the default parameters, you will have a problem. Since distribution is declared static, it will be initialized only once with whatever min/max params you call random_double() for the first time. And, from there on the function will operate on the same range, even if you call it with different min/max.

That's why in the book the two functions are separate.

Technically, since uniform_real_distribution is a pretty simple class that stores min/max values and uses them to "lerp" output of the random engine, it can be declared non-static as @niccolot points out in the comment. In that case the function can be simplified to:

inline auto random_double(double min = 0, double max = 1)
{
    static std::mt19937 gen;
    return std::uniform_real_distribution{min, max}(gen);
}

However, there is still a semantic issue with this implementation. It allows one to call this function like so:

auto x = random_double(10);

But, what does this mean? People familiar with python (see range()) or bash (see seq) might think that it will generate numbers in the range [0, 10), but in fact, it is called with invalid range [10, 1).

FWIW, you could do something like this to emulate python/bash behavior:

inline auto random_double(double min, double max)
{
    static std::mt19937 gen;
    return std::uniform_real_distribution{min, max}(gen);
}

inline auto random_double(double max = 1) { return random_double(0, max); }

Or, if you want to emulate the book behavior, but let c++ do the interpolation:

inline auto random_double(double min, double max)
{
    static std::mt19937 gen;
    return std::uniform_real_distribution{min, max}(gen);
}

inline auto random_double() { return random_double(0, 1); }

In either case, you still end up with two functions though.

[niccolot]

Yes, indeed it was my fault. I' ve separated the two functions (with random_double(min, max) that calls random_double() but it is the same thing as you posted)