Remove non-STL random and added randDiscrete

[williampma]

There's this bit of comment that says

-/** @todo Once C++11 is fully supported by GCC feel free to remove anything
- * wrapped by #ifndef USE_STL_RANDOM  
- * The situation is confusing... According to gnu.org, this has been
- * supported since gcc-4.3 (but possibly in the tr1 namespace ??)
- * In Ubuntu oneiric, with gcc-4.6.2, the build failed.
- *  With gcc-4.6.3, things work.  I'm confused. This
- * needs more user reports.
- */

so I assume removing all the old non-STL random is always wanted.

[williampma]

Weird... how come my randomUTest pass on my machine and not on Travis... I thought random on the same seed should be fine...

Or am I using the std::distribution wrong?

[ngeiswei]

Woohoo, very cool Travis check!!! :-)

[amebel]

travis builds are run on ubuntu 12.04, with some ppa for recent versions. perhaps it is a dependency issue?

[williampma]

Oh, that might be it. The old comment I quoted said it failed on gcc 4.6.2. Perhaps ubuntu 12.04 uses that...

[williampma]

Hummm.... C++11 requires GCC 4.8.1...

[amebel]

after some change on travis so as to use 4.9 and rebasing your pr on it, i am getting https://travis-ci.org/AmeBel/cogutils/builds/73349281

[williampma]

Hummm... I really don't know what's going on...

[ngeiswei]

Better use TS_ASSERT_LESS_THAN http://cxxtest.com/guide.html#ts_assert_less_than that way it returns more meaningful message when it fails.

[ngeiswei]

Very very strange, this passes on my system (gcc 4.8.4) if I don't output p[0] to p[3]. If I do, by adding the following code

        for (size_t i = 0; i < 4; ++i)
            std::cout << "p[" << i << "] = " << p[i]
                      << ", p[" << i << "]/" << size << " = "
                      << (double)p[i]/(double)size << std::endl;

I get the following

p[0] = 7400799, p[0]/10000000 = 0.74008
p[1] = 3000512, p[1]/10000000 = 0.300051
p[2] = 3997427, p[2]/10000000 = 0.399743
p[3] = 2001454, p[3]/10000000 = 0.200145

In randomUTest::test_discrete:
/home/nilg/OpenCog/AmeBel/cogutils/tests/util/randomUTest.cxxtest:76: Error: Assertion failed: p[3] > p[0]
Failed 1 and Skipped 0 of 3 tests
Success rate: 66%

and it fails...

Looks like std::mt19937 relies on quantum mechanics, if you observe it, it behaves differently.

[amebel]

just checked in container and travis https://travis-ci.org/AmeBel/cogutils#L726, success using gcc 4.8 but not on gcc 4.9

[ngeiswei]

I think I know the problem! The array p is not initialized with zeros!

[ngeiswei]

Yep, works, just add

        for (size_t i = 0; i < 4; ++i)
            p[i] = 0;

after defining p, or use a std::vector.

[williampma]

Haha, oops, it happens. Thanks!

I will fix it tomorrow.

[amebel]

forgive my ignorance, but what just happened with that line? and why would the compiler miss that?

[ngeiswei]

Well, it's another complicated C++ thing http://stackoverflow.com/questions/11812687/why-is-int-array-not-initialized-to-zeros-in-c

BTW, according to http://stackoverflow.com/questions/201101/how-to-initialize-an-array-in-c one can just write

        int p[4] = {};

[williampma]

Unit test passes now, but how come Travis is testing twice, once with CXX=g++4.9 and once with CC="gcc4.9 (what's with the single "?)?

[amebel]

#11 fixes that. 'env' key is used to set mutliple builds, which I unkowling thought was a means of setting environment variables, which in a sense is, with each list entry a single separate environment identifier.

[williampma]

Oh I see. That might to useful in the future I guess to test different setup.

Anyway, since Nil seems to be OK with the changes (he didn't comment on the old random code removal), I treat this as reviewed and will merge.

[amebel]

BTW, is it appropriate to remove the copyright notice?

[williampma]

No longer using the copyrighted code right?

[amebel]

It seems that way.