RNG seeding

[djboersma]

While answering a user question about random number generation, I came across these lines: https://github.com/OpenGATE/Gate/blob/develop/source/general/src/GateRandomEngine.cc#L154

  // use clhep engine to initialize other engine
  std::srand(static_cast<unsigned int>(*theRandomEngine));
  srandom(static_cast<unsigned int>(*theRandomEngine));

#ifdef G4ANALYSIS_USE_ROOT
  gRandom->SetSeed(static_cast<unsigned int>(*theRandomEngine));
#endif

I do not understand this code (*). There is so much wrong with this. The object pointer of some object is a very bad choice for a seed. But more importantly: we should be using only one single random number engine, not a whole zoo of them. Yes, the user may configure which RNG that is, but then all random numbers in the Gate run should be generated with that one generator. I have not yet dared to check how much the reset of the GATE code invokes std::rand and/or random and gRandom, but in any case I think these embarrassing lines should be deleted. Or if there is some terrible reason why we do need multiple RNGs, then at least we should make sure that the seed will be random whenever it needs to be random, and reproducible whenever it needs to be reproducible. An object pointer is neither.

(*) That is a polite lie. My real reaction to this code is not suitable for a public website.

[BishopWolf]

I do not understand this code (*).

Simply, you are assigning another package like clhep and root the current random engine, so everything has the same random number generator, remember gate is not the one calling the generators, but geant4, clhep and root.

[djboersma]

What do you mean by "everything has the same random number generator"? The srand, srandom functions and the SetSeed method are used to set the seed value of three totally different random number engines.

IF "everything has the same random number generator" is really supposed to mean that std::rand, std::random and gRandom are now somehow all aliases to the same RNG, namely the CLHEP one, then NO, that is definitely not what this code achieves.

I looked a little harder at the code and I have to admit that I previously misread the code: I thought that the object pointer theRandomEngine was cast to an unsigned int. Using a memory address for random number generation would be bad. But that is not what the code does: it dereferences the pointer before applying the cast. And the cast indirectly invokes the "operator unsigned int" method of the HepRandomEngine base class, which generates a random number and returns it as a unsigned int.

So this code is basically a very obfuscated way to seed those other RNGs, using random numbers from the CLHEP RNG.

And yes, geant4 is actually using the RNG, not the Gate actors. Or that is how it should be. So I really hope we can just delete this piece of confusing code. Ideally this will have no effect at all.

I did some grepping through all Gate code:

• I see that only the C-like GPU-related code seems to be using rand(), but as far as I can see srand() is called there just before any use of rand(), so we can probably safely delete the srand() call from GateRandomEngine.cc. Maybe we can get rid of rand() altogether by passing to the C-like GPU functions an array of random numbers harvested from theRandomEngine instead of a single seed value.
• I do not find any use of std::random at all, so I think that the srandom line in GateRandomEngine.cc can be removed as well.
• I found 5 instances of #include <TRandom.h> and commented out all of them except for the one in GateRandomEngine.cc; and the code compiled normally. Also there are no other uses of gRandom except for setting the seed. There may be more indirect uses of the ROOT random number generator. ROOT histograms can be filled randomly with the FillRandom method, but the only instances of "FillRandom" in Gate source code have nothing to do with ROOT histograms, but rather with "sinograms", which fortunately are ROOT-free. So it looks like we can get rid of the gRandom seed initialization as well, but I'm not 100% sure.