apt-sim / AdePT

Accelerated demonstrator of electromagnetic Particle Transport
Apache License 2.0
25 stars 34 forks source link

Modernise some code in examples/common #288

Closed hageboeck closed 4 months ago

hageboeck commented 5 months ago

Here is a set of three commits cherry-picked from my async branch. They modernise the ParticleGun and the HepMC3Ascii classes, which I was able to integrate into the async work now.

The most radical change is probably the reseeding of G4's thread-local RNG in the third commit. This ensures that you get the same random sequence for every event, irrespective of how many random numbers the previous event consumed. I think that's a safe thing to do, because it saves the initial seed for each thread at startup, and adds an event offset to it, but please have a more detailed look.

phsft-bot commented 5 months ago

Can one of the admins verify this patch?

hahnjo commented 5 months ago

The most radical change is probably the reseeding of G4's thread-local RNG in the third commit. This ensures that you get the same random sequence for every event, irrespective of how many random numbers the previous event consumed. I think that's a safe thing to do, because it saves the initial seed for each thread at startup, and adds an event offset to it, but please have a more detailed look.

I'm surprised this is necessary. As far as I remembered and still after reading the code of G4WorkerRunManager::GenerateEvent and G4WorkerTaskRunManager::GenerateEvent, every event is reseeded to guarantee reproducibility with any number of threads (unless the user explicitly turns this off). That's not the case for the sequential G4RunManager though, are you using that?

hageboeck commented 5 months ago

The most radical change is probably the reseeding of G4's thread-local RNG in the third commit. This ensures that you get the same random sequence for every event, irrespective of how many random numbers the previous event consumed. I think that's a safe thing to do, because it saves the initial seed for each thread at startup, and adds an event offset to it, but please have a more detailed look.

I'm surprised this is necessary. As far as I remembered and still after reading the code of G4WorkerRunManager::GenerateEvent and G4WorkerTaskRunManager::GenerateEvent, every event is reseeded to guarantee reproducibility with any number of threads (unless the user explicitly turns this off). That's not the case for the sequential G4RunManager though, are you using that?

Oh, good point. I did this a long time ago, probably on a G4 without MT enabled. I'll take it out of the PR, and test if it's still needed.

JuanGonzalezCaminero commented 5 months ago

Working as expected after the last change, changing the number of threads gives the same result in each event. I saw a minor issue, when reading events from a file this code seems to skip the first one, for example:

Master, reading 3 events:

Evt 1
G4WT0 > pi+: 1, 801183
G4WT0 > pi-: 1, 1.8443e+06
G4WT0 > proton: 1, 1.02021e+06
Evt 2
G4WT0 > anti_neutron: 1, 17305.6
G4WT0 > neutron: 1, 1.59522e+06
G4WT0 > pi+: 3, 13909.5
G4WT0 > pi-: 3, 7682.61
Evt 3
G4WT0 > pi+: 1, 2804.15
G4WT0 > pi-: 3, 36094.9
G4WT0 > proton: 1, 236559
G4WT0 > sigma+: 1, 55231.1

New, reading 3 events

Evt 1
G4WT0 > anti_neutron: 1, 17305.6
G4WT0 > neutron: 1, 1.59522e+06
G4WT0 > pi+: 3, 13909.5
G4WT0 > pi-: 3, 7682.61
Evt 2
G4WT0 > pi+: 1, 2804.15
G4WT0 > pi-: 3, 36094.9
G4WT0 > proton: 1, 236559
G4WT0 > sigma+: 1, 55231.1
Evt 3
G4WT0 > anti_neutron: 1, 261150
G4WT0 > neutron: 1, 330919
G4WT0 > pi+: 3, 65149.5
G4WT0 > pi-: 3, 4399.75
hageboeck commented 4 months ago

Working as expected after the last change, changing the number of threads gives the same result in each event. I saw a minor issue, when reading events from a file this code seems to skip the first one, for example:

OK, that's found now. reader.skip(0) skips one event, skip(1) skips two, and so on.