Use central Random Number Seed Service rather than config

[tomeichlersmith]

https://github.com/LDMX-Software/ldmx-sw/blob/4e76f4267d6986cbdef9ecdcf76efe7d5330b2a9/TrigScint/src/TrigScint/TrigScintDigiProducer.cxx#L29-L30

:cry:

1385 reminded me of this since we need to set the randomSeed for the TS digi processors within the CI.

https://github.com/LDMX-Software/ldmx-sw/blob/4e76f4267d6986cbdef9ecdcf76efe7d5330b2a9/Ecal/src/Ecal/EcalDigiProducer.cxx#L73-L78

The seeding in ECal is done in produce because it was written before a patch to Framework allowing use of conditions within onNewRun. I would suggest putting the same code within onNewRun to avoid confusion of future developers and to avoid unnecessary checks. Moving the seeding to onNewRun cleans up the code and makes it obvious that the RNG does not need to be seeded on every event. This also gives us the opportunity to switch to using STL like Tracking instead of TRandom.

Solution

One can seed their noise generator within onNewRun which is the first callback that happens after the conditions are configured. This gives you access to the RNSS and allows you to have your seeds be produced via the central service which will change the seeds whenever all of the other seeds should change.

Examples https://github.com/LDMX-Software/ldmx-sw/blob/4e76f4267d6986cbdef9ecdcf76efe7d5330b2a9/Tracking/src/Tracking/Reco/DigitizationProcessor.cxx#L71-L75

where generator_ is a std::default_random_engine which can be passed to an instance of a distribution (for example std::normal_distribution<float>) to sample it.

[tvami]

This is the same development as I did in https://github.com/LDMX-Software/ldmx-sw/pull/1359/commits/bda6d5c7145d052d7bac36cfc0eb3f8ea2640192 right? If yes, seems easy, I could put this on my todo list.

[tomeichlersmith]

Kinda. If you choose to use STL[^1], then you'll need to change TRandom to an equivalent STL distribution (std::uniform_int_distribution I believe) or you could follow whats in ECal for the ROOT way which just waits to create TRandom until access to the RNSS is available.

[^1]: I would prefer to use STL, but I want to be clear that it would incur slightly more development to drop TRandom completely.

[tomeichlersmith]

STL Implementation Ramble

The reason I like how STL implements this is because it separated the RNG from the distribution. This way, we can construct and configure a distribution within the constructor and/or configure methods and then seed the RNG somewhere else (like onNewRun). For example

void MyProc:configure(framework::config::Parameters& p) {
  my_distribution_ = std::make_unique<std::uniform_int_distribution>(0, p.getParameter<int>("max_rand_int"));
}

void MyProc::onNewRun(const ldmx::RunHeader& rh) {
 const auto& rnss{getCondition<...>(...)};
  rng_.seed(rnss.getSeed("MyProc::RNG"));
}

void MyProc::produce(Event& event) {
  int rand_id = (*my_distribution)(rng_);
}

The call site to get rand_id is ugly because we have to delay constructing my_distribution_ until configure. If you are able to construct my_distribution_ before run-time configuration, then you can avoid the std::unique_ptr wrapper and make it cleaner.

// dropping rest of construction boilerplate
MyProc::MyProc(...) : my_distribution_{0, 10} {}

// onNewRun is the same

void MyProc::produce(Event& event) {
  int rand_id = my_distribution_(rng_);
}

[tvami]

Maybe we should have another issue that's regarding moving ECAL to STL as well, I understand the implementation choice for ecal was somewhat historical.

[tomeichlersmith]

That's a fair point, I'm also willing have this issue represent both since the implementations are so similar.

[tvami]

, I'm also willing have this issue represent both

Sure let's do that

[tvami]

I also propose to do this after the we have a new release with new golds