upgrade Mersenne Twister implementation

[villaa]

Currently the random number generator -- The Mersenne Twister -- is implemented by copying an old code directory into the repository. Apparently this directory was taken from some of my ancient code and is from 2002 or so. The problem with modern ISO C++ (c++0x, c++11, c++17) is that it uses the register keyword which leads to:

Mersenne/MersenneTwister.h:187:2: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister] register uint32 s1;

To be clear, this error appears on MacOSX 10.14.6 (Mojave) with this version of clang:

Apple LLVM version 10.0.1 (clang-1001.0.46.4) Target: x86_64-apple-darwin18.7.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin

I think it's now time to change this to a more modern implementation. There is even one incorporated into the newer C++ itself but I think it has a different API, which causes annoyances and the necessity of changing of code. This needs to be checked

I could also try to use an older compiler standard. Here is a note on that, but I couldn't get it working immediately.

This is an issue from another code that seems to be identical. But it seems like they've stripped out all of the register keywords, which I'd prefer not to do because it changes the original code base. Is that OK in terms of the license?

There also seem to be other stand-alone implementations out there but I don't know if they have the same API as the one we were using.

Currently this is a barrier to correctly compiling the code on my Mac, so it rises to the level of a bug.

[villaa]

Here is what the register keyword means, and its purpose.

[villaa]

Here is the C++ standard library version of Mersenne Twister. It appears to have been introduced in the C++11 standard.

[villaa]

PR #28 is code attempting to address this problem.

[nuclearGoblin]

Note: These errors appear as warnings in gcc 7.5.0 (Ubuntu) but don't prevent compiling or running the software. Since it seems to be an issue with MT being written for a much older C++ standard, it might make sense to switch to the built-in MT (which would mean starting a new major version as this would kill compatibility with C++ standards older than 11).

[villaa]

Note: These errors appear as warnings in gcc 7.5.0 (Ubuntu) but don't prevent compiling or running the software. Since it seems to be an issue with MT being written for a much older C++ standard, it might make sense to switch to the built-in MT (which would mean starting a new major version as this would kill compatibility with C++ standards older than 11).

@gerudo7 I agree, can you open an issue for this that is not a "bug" but an "improvement"

[villaa]

solved by PR #28

[villaa]

solved by PR #28

I'm not sure this was actually solved by PR #28 because there have been some issues running on OSX.

@gerudo7 I recommend we solve this issue only by upgrading the MT to the internal C++ version.

[villaa]

It may be the cross-compilation done by using a different -std=c++17 flag for our code compared to ROOT that causes the issue #32 as well. That's why it's best to switch the MT implementation, so the C++ standard can be consistent across ROOT and our code before we start digging into possible memory errors within our own code.

[villaa]

interesting post that suggests MT would require "warm up" -- is this something that will affect us?

https://stackoverflow.com/questions/15509270/does-stdmt19937-require-warmup

[villaa]

This SO post may provide a way to get the new C++ implementation of MT to provide a random number on [0,1] rather than just a 32- or 64-bit number as it is tuned to do in these docs.

I guess the way I would go about this is to pass the C++ object std::mt19937 mt(rd()); into all random-number-needing functions as we already have in the code. Then in each random-number-needing function we get a dist object to (as far as I understand) turn the mt value into a random float on [0,1].

[villaa]

addressed by PR #54