search

eic / EICrecon

EIC Reconstruction - JANA based
https://eic.github.io/EICrecon
GNU Lesser General Public License v3.0
6 stars 29 forks source link

Add an RNG service and avoid using `TRandom` #539

Open c-dilks opened 1 year ago

c-dilks commented 1 year ago

Is your feature request related to a problem? Please describe. Problem: In the algorithm PhotoMultiplierHitDigi, if the seed for the TRandomMixMax object is set to 0 (use "random" seed, see documentation), then the randomly generated number can sometimes remain the same for several generations, then start actually randomizing later; this is a possible symptom of thread unsafety. This issue does not appear to happen if the seed is non-zero (a fixed seed, useful for reproducibility, but less random than seed 0).

TRandom is used in the following algorithms and factories:

src/algorithms/digi/PhotoMultiplierHitDigi.h
src/algorithms/digi/SiliconTrackerDigi.h
src/algorithms/reco/MC2SmearedParticle.h
src/global/digi/SiliconTrackerDigi_factory.h
src/global/tracking/TrackerHitReconstruction_factory.h

and some tracking efficiency benchmarks.

Describe the solution you'd like Ideally, we should implement a thread-safe random number generation service.

When resolved, remove the following warnings and consider using seed=0:

Describe alternatives you've considered Use seed != 0, which appears to work okay, but is not really ideal for the long term, especially if we want more randomness.

DraTeots commented 1 year ago

BTW, there is random generator service but it is barely usable in the current implementation.

c-dilks commented 1 year ago

Nice, let's make it a proper service (e.g., src/RandGen_driver.cpp should not live in src) and start using it! Unfortunately I don't have time to help do this, but this seems like a very good entry-level issue for a new person to get involved with.

wdconinc commented 1 year ago

this seems like a very good entry-level issue for a new person to get involved with.

Not quite. Getting random numbers isn't hard. It's getting the same random numbers in the same sequence to the same algorithms, even when algorithms may run out of sequence the next time you run it... And this must all work in multithreaded running.

c-dilks commented 1 year ago

BTW, there is random generator service but it is barely usable in the current implementation.

Removed by https://github.com/eic/EICrecon/pull/640.