search

nrc-cnrc / EGSnrc

Toolkit for Monte Carlo simulation of ionizing radiation — Trousse d'outils logiciels pour la simulation Monte Carlo du rayonnement ionisant
http://nrc-cnrc.github.io/EGSnrc
GNU Affero General Public License v3.0
242 stars 146 forks source link

Bug in egs++ recycling phase-spaces in parallel when ncase > nphsp #1156

Closed rtownson closed 2 months ago

rtownson commented 4 months ago

Describe the bug When turning on particle recycling using a phase-space source in egs++, and then running the job in parallel, when ncase > nphsp there will be warnings printed to the screen, and unexpected re-use of particles may occur. Since the jobs are assigned a chunk of the phase-space, it can't seek to a position past the end of the phase-space. But since you are recycling particles, it should be possible to set ncase = nrecycle*nphsp.

This was reported here: https://www.reddit.com/r/EGSnrc/comments/1dqow18/local_parallelization_questions/

The problem can be seen in iaea_phsp_source.cpp IAEA_PhspSource::setSimulationChunk and in egs_run_control.cpp EGS_JCFControl::getNextChunk(). Also in egs_phsp_source.cpp.

Note that it will be extra complex to account for in IAEAphsp, because we allow different recycling numbers for photons and electrons. Since we don't know ahead of time exactly how many photons and electrons will be in each chunk, this might be tricky to account for.

Operating system all

EGSnrc version v2023 and since phase-spaces were implemented in egs++

blakewalters commented 3 months ago

@rtownson, some of the issues reported by the user, such sampling beyond the end of the phase space file ("illegal simulation chunk"), should have been addressed by https://github.com/nrc-cnrc/EGSnrc/pull/1126. In addition to this fix, though, are you proposing an option to override the setting of ncase in favour of ncase=nrecycle*nphsp when applicable?

rtownson commented 3 months ago

Excellent, I had already forgotten about your fix! No, I was not proposing to set ncase=nrecycle*nphsp, I just meant that it should be possible.

blakewalters commented 3 months ago

@rtownson, I think this issue is fixed by https://github.com/nrc-cnrc/EGSnrc/pull/1126. I recommend closing it and then possibly opening a new issue regarding calculating ncase based on nrcycl/Nphsp if we want to add that functionality.

rtownson commented 3 months ago

@blakewalters This issue is already linked to #1126 so it will close automatically when that gets merged (see the Development section in the top right). I don't think we need to add that functionality for ncase, or at least I didn't mean to suggest it.