Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Incomplete multiple-scattering comment in egsnrc.mortran #1131

Open
dworogers opened this issue May 23, 2024 · 0 comments · Fixed by #1132
Open

Incomplete multiple-scattering comment in egsnrc.mortran #1131

dworogers opened this issue May 23, 2024 · 0 comments · Fixed by #1132
Assignees
@ftessier
Labels
documentation improvement
Milestone
Release 2024

Comments

@dworogers
Copy link

dworogers commented May 23, 2024 •
edited by ftessier
Loading

Lines 1247-1250 in egsnrc.mortran state

IF((tustep <= tperp) & ((~exact_bca) | (tustep > skindepth)))
                [
                    "We are further way from a boundary than a skindepth, so
                    "perform a normal condensed-history step

The comment ignores the fact that only if the step is greater than skindepth does the CH step get taken. Otherwise the CH step is replaced by single scattering. As far as I can tell, the coding is correct, it's just the comment that I find misleading. It should be something like

"We are further away from a boundary than a skindepth or the requested CH step is
"greater than skindepth so perform a normal condensed-history step
@ftessier ftessier added this to the Release 2024 milestone May 23, 2024
ftessier added a commit that referenced this issue May 23, 2024
@ftessier
Fix #1131: clarify comment about ms conditional
e60ee5d 
Clarify that multiple-scattering transport is triggered only if the
intended electron step is longer than a skin depth, even in bulk media
away from any region boundaries. This improves efficiency, because a
multiple-scattering step costs about the same as 3 single-scattering
steps (the default skin depth is 3 mean free paths). Below a skin depth,
it is more efficient (and more accurate) to use single-scattering.
@ftessier ftessier linked a pull request May 23, 2024 that will close this issue
Fix #1131: clarify comment about ms conditional #1132
Merged
@ftessier ftessier changed the title misleading comment in egsnrc.mortran Incomplete multiple-scattering comment in egsnrc.mortran May 23, 2024
rtownson pushed a commit that referenced this issue Jun 18, 2024
@ftessier @rtownson
Fix #1131: clarify comment about ms conditional
013892f 
Clarify that multiple-scattering transport is triggered only if the
intended electron step is longer than a skin depth, even in bulk media
away from any region boundaries. This improves efficiency, because a
multiple-scattering step costs about the same as 3 single-scattering
steps (the default skin depth is 3 mean free paths). Below a skin depth,
it is more efficient (and more accurate) to use single-scattering.
@ftessier ftessier self-assigned this Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ftessier ftessier

Labels
documentation improvement
Projects
None yet
Milestone
Release 2024
Development

Successfully merging a pull request may close this issue.

Fix #1131: clarify comment about ms conditional nrc-cnrc/EGSnrc
2 participants
@ftessier @dworogers