Fix egs_chamber usage of latch for splitting #622
Conversation
|
@rtownson Ok, waiting on the test results. Do you need precision simulations? If I read the changes correctly, simulations with these changes should be numerically equivalent to previous ones using latch, no? In this case, testing can be done with very short simulations. |
|
I note that the check failed due to the egs++ failing to compile. This is a recurrent problem recently with Travis. It fails egs++ compilation, but upon relaunching the check it often succeeds (without changing the branch at all). So there seems to be something flaky on the Travis side, I have not figured it out yet... |
|
@rtownson is this ready to go in your opinion, or do you seek more testing ahead of merging? If so, can you turn this into a draft pull request (and optionally add |
|
I do still want to take it for a spin, and haven't had a chance yet. I'll turn it into a draft.. |
rtownson
changed the title
|
Let's create a WIP label!! |
ftessier
changed the title
|
@ftessier I do get different numbers, within the statistical uncertainties. My understanding was that any change to the code that requires recompilation can potentially produce a different result for the same seeds. |
|
I will look into that. In principle yes, but in this particular case I fail to see why there would be a numerical difference, especially given that most of the changes here pertain to integer variables! The compiler optimization might create somewhat different code paths though, so I will try to compile without optimization to check. |
|
@rtownson I am testing this with a full exradin A12 chamber simulation depth dose, with TmpPhsp and xcse on. As you said, the results differ numerically, and I am trying to track this down. I have created a code where |
|
Oh! While writing my previous comment it occured to me that latch is initialized from getNextParticle, but the_extra_stack->nbr_splitting isn't! Yep, that seems to be the issue. |
|
Confirmed: upon adding |
|
I was just working on this, but you beat me to the solution! Thanks Fred! |
ftessier
requested review from
blakewalters
and removed request for
blakewalters
ftessier
force-pushed
the
fix-egs-chamber-latch
branch
from
1ba7aff to
90449f9
Compare
Use a dedicated splitting variable nbr_split, instead of latch, to hold the splitting number in the egs_chamber application. A new stack variable nbr_splitting is added to pass its value through the EGSnrc back end. This avoids overwriting the latch variable in cases where it is needed otherwise, for example in the phsp scoring object. Ideally, the meaning of latch should remain consistent with BEAMnrc.
ftessier
force-pushed
the
fix-egs-chamber-latch
branch
from
90449f9 to
91ad1e7
Compare
Fix the egs_chamber russian roulette splitting algorithm so that the latch variable is no longer used. Instead, an extra stack is created with a splitting variable in it. This avoids allowing egs_chamber to overwrite the latch, in case other objects such as
egs_phsp_scoringneed to use it.Previously, if you used russian roulette in egs_chamber and scored a phsp using egs_phsp_scoring, the multiple passer flag for scoring particles may have been incorrect. I think this would be a small effect.