ranlux24_base(0) should be equal to ranlux24_base()

[Stephan T. Lavavej <stl@…>]

After fixing a bug in VC11's <random> (see [1]), I compared VC's fixed output to Boost 1.52.0's output and I believe I found a bug in Boost. Here's a self-contained test case:

C:\Temp>type meow.cpp
#include <ios>
#include <iostream>

#ifdef USE_BOOST
    #include <boost/random.hpp>
    using std::boolalpha;
    using std::cout;
    using std::endl;
    using boost::random::minstd_rand0;
    using boost::random::mt19937;
    using boost::random::ranlux24_base;
#else
    #include <random>
    using namespace std;
#endif

template <typename Engine> typename Engine::result_type run_10k(Engine engine) {
    for (int i = 0; i < 9999; ++i) {
        engine();
    }

    return engine();
}

int main() {
    cout << boolalpha;

    #ifdef USE_BOOST
        cout << "Using boost." << endl;
    #else
        cout << "Using std." << endl;
    #endif

    cout << "N3485 26.5.5 [rand.predef]/1 satisfied: " << (run_10k( minstd_rand0()) == 1043618065) << endl;
    cout << "N3485 26.5.5 [rand.predef]/3 satisfied: " << (run_10k(      mt19937()) == 4123659995) << endl;
    cout << "N3485 26.5.5 [rand.predef]/5 satisfied: " << (run_10k(ranlux24_base()) == 7937952   ) << endl;

    cout << "I believe  minstd_rand0(0) ==  minstd_rand0() should be  true: "
                    << (minstd_rand0(0) ==  minstd_rand0()) << endl;
    cout << "I believe       mt19937(0) ==       mt19937() should be false: "
                         << (mt19937(0) ==       mt19937()) << endl;
    cout << "I believe ranlux24_base(0) == ranlux24_base() should be  true: "
                   << (ranlux24_base(0) == ranlux24_base()) << endl;

    cout << "run_10k( minstd_rand0(0)): " << run_10k( minstd_rand0(0)) << endl;
    cout << "run_10k(      mt19937(0)): " << run_10k(      mt19937(0)) << endl;
    cout << "run_10k(ranlux24_base(0)): " << run_10k(ranlux24_base(0)) << endl;
}

With my fixed build of Milan/VC12, I'm getting:

C:\Temp>cl /EHsc /nologo /W4 /MTd meow.cpp
meow.cpp

C:\Temp>meow
Using std.
N3485 26.5.5 [rand.predef]/1 satisfied: true
N3485 26.5.5 [rand.predef]/3 satisfied: true
N3485 26.5.5 [rand.predef]/5 satisfied: true
I believe  minstd_rand0(0) ==  minstd_rand0() should be  true: true
I believe       mt19937(0) ==       mt19937() should be false: false
I believe ranlux24_base(0) == ranlux24_base() should be  true: true
run_10k( minstd_rand0(0)): 1043618065
run_10k(      mt19937(0)): 1543171712
run_10k(ranlux24_base(0)): 7937952

(Note that VC11 RTM/Update 1 will suffer the bug described by [1].)

If I use Boost's implementation with VC11 Update 1, I get:

C:\Temp>cl /EHsc /nologo /W4 /MTd /DUSE_BOOST meow.cpp /I boost-1.52.0\include
meow.cpp

C:\Temp>meow
Using boost.
N3485 26.5.5 [rand.predef]/1 satisfied: true
N3485 26.5.5 [rand.predef]/3 satisfied: true
N3485 26.5.5 [rand.predef]/5 satisfied: true
I believe  minstd_rand0(0) ==  minstd_rand0() should be  true: true
I believe       mt19937(0) ==       mt19937() should be false: false
I believe ranlux24_base(0) == ranlux24_base() should be  true: false
run_10k( minstd_rand0(0)): 1043618065
run_10k(      mt19937(0)): 1543171712
run_10k(ranlux24_base(0)): 14007167

Here's why I believe that ranlux24_base(0) should be equal to ranlux24_base(). ranlux24_base is a subtract_with_carry_engine. 26.5.3.3 [rand.eng.sub]/7 says:

"explicit subtract_with_carry_engine(result_type value = default_seed); [...] linear_congruential_engine<result_type, 40014u,0u,2147483563u> e(value == 0u ? default_seed : value);"

/7 doesn't refer to "value" anywhere else. Therefore, 0 should behave identically to the default_seed 19780503u.

(For comparison, VC and Boost agree that minstd_rand0(0) == minstd_rand0(). 26.5.3.1 [rand.eng.lcong]/5 says:

"explicit linear_congruential_engine(result_type s = default_seed); [...] If c mod m is 0 and s mod m is 0, sets the engine's state to 1, otherwise sets the engine's state to s mod m."

The default_seed is 1u and minstd_rand0 sets c to 0, so that's why 0 behaves identically to the default_seed.)

[1] ​http://connect.microsoft.com/VisualStudio/feedback/details/776456

[Marshall Clow]

Fixed the formatting in the description.

Also, I verified the same behavior occurs with clang/libc++

[Marshall Clow]

The following change causes your test program to run successfully, and doesn't break any of the existing Boost.Random tests.

However, someone who understands the random code should probably review this and make sure it is the correct change.

Index: boost/random/subtract_with_carry.hpp
===================================================================
--- boost/random/subtract_with_carry.hpp	(revision 82663)
+++ boost/random/subtract_with_carry.hpp	(working copy)
@@ -161,7 +161,7 @@
                                         IntType, value)
     {
         typedef linear_congruential_engine<uint32_t,40014,0,2147483563> gen_t;
-        gen_t intgen(static_cast<boost::uint32_t>(value));
+        gen_t intgen(static_cast<boost::uint32_t>(value == 0 ? default_seed : value));
         detail::generator_seed_seq<gen_t> gen(intgen);
         seed(gen);
     }

[Steven Watanabe]

This fix appears to be correct, however,

a) Please fix subtract_with_carry_01_engine also. b) Please add a test case.

[Marshall Clow]

(In [82664]) Fix behavior with zero seed re: '26.5.3.3 [rand.eng.sub]/7'; thanks to stl@microsoft for the report; Refs #7951

[Marshall Clow]

(In [82820]) Merge Fix for zero seed in Boost.Random; Fixes #7951