Correct Compton shell selection and Doppler broadening - #4036

Merged
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix
Jul 31, 2026
Merged

Correct Compton shell selection and Doppler broadening#4036
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix

Conversation

@paulromano

@paulromanopaulromano commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

The Compton Doppler-broadening algorithm on the current develop branch does not sample the bound-electron impulse approximation correctly. In particular, it selects a subshell using only the number of electrons in that shell. If the incident photon cannot ionize the selected shell, or if the kinematic upper bound on longitudinal electron momentum is negative, the code falls back to the free-electron Compton energy. However, a negative upper bound is not an invalid condition: it means that only part of the negative-momentum tail of the shell's Compton profile is accessible.

For shell $i$, the physically allowed longitudinal-momentum interval is

$$ -\frac{1}{\alpha_\mathrm{fs}} \le p_z \le p_{z,\max,i}, $$

where $\alpha_\mathrm{fs}$ is the fine-structure constant. The probability of selecting the shell is therefore proportional to

$$ P_i \propto f_i H(E-E_{b,i}) \int_{-1/\alpha_\mathrm{fs}}^{p_{z,\max,i}} J_i(p_z),dp_z, $$

where $f_i$ is the number of electrons in the shell, $E_{b,i}$ is its binding energy, and $J_i$ is its symmetric Compton profile. Thus, shell selection depends on both electron occupancy and the fraction of the profile that is kinematically accessible.

The develop implementation also samples only nonnegative $p_z$, chooses randomly between the two positive solutions of the scattered-energy quadratic, and does not apply the outgoing-energy rejection factor from the approximate relativistic impulse approximation. These choices distort the broadened energy distribution, particularly near shell thresholds and for forward scattering.

Relationship to #3874

#3874 correctly identifies that inaccessible shells should not be accepted and that negative values of $p_{z,\max}$ need to be handled. However, its proposed implementation has three blocking problems:

  1. The negative $p_{z,\max}$ branch samples outside the tabulated CDF. OpenMC stores only the nonnegative half of each symmetric Compton profile, so its integrated value approaches approximately $1/2$, not 1. For $p_{z,\max}<0$, Fix a bug in compton scattering electron shell selection #3874samples a CDF value between the integral at $|p_{z,\max}|$ and 1. Values above the end of the half-profile CDF can make the subsequent lookup run past the profile table.

  2. The sign of $p_z$ is lost and the wrong energy root can be selected.Fix a bug in compton scattering electron shell selection #3874 inverts the profile using $|p_{z,\max}|$ but leaves the sampled momentum nonnegative. It then always selects the lower positive root of the scattered-energy quadratic. The physical branches are instead determined by the sign of the sampled momentum: $p_z<0$ corresponds to an energy below the free-electron Compton energy and requires the lower root, whereas $p_z>0$ corresponds to an energy above it and requires the upper root.

  3. Shell selection is not weighted by the accessible profile mass.Fix a bug in compton scattering electron shell selection #3874samples shells by electron occupancy and rejects only on the binding threshold. That threshold check is necessary but insufficient. At fixed energy and angle, different shells have different values of $p_{z,\max,i}$ and therefore different accessible fractions of their momentum distributions. Selecting among all energetically open shells using electron occupancy alone produces the wrong shell probabilities.

Proposed approach

This PR replaces the existing sampler with the approximate relativistic impulse approximation procedure described in Sec. 3.4.8 of Kaltiaisenaho's Master's thesis:

The implementation uses a two-stage strategy to retain the accuracy of this procedure without paying the full cost of constructing the conditional shell distribution for ordinary collisions. It first makes two inexpensive occupancy-based shell proposals and accepts each according to its accessible profile mass. If neither proposal succeeds, it evaluates the kinematics for every shell and samples directly from the conditional mass function $f_i M_i$, where $M_i$ is the accessible profile mass. This is statistically equivalent to repeated shell rejection but avoids pathological runtimes when every shell has very little accessible mass, such as near forward scattering.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great PR!
I have a few concerns and suggestions.

Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
paulromanoand others added 2 commits July 30, 2026 11:01
Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>
@paulromano

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @GuySten. Your comments/suggestions have been addressed.

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks @paulromano for addressing the problem.

@GuyStenGuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jul 30, 2026

@amandalundamandalund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice correction @paulromano! The physics looks sound to me.

@GuySten
GuySten merged commit a815267 into openmc-dev:developJul 31, 2026
18 checks passed
@paulromano
paulromano deleted the compton-alternate-fix branch July 31, 2026 13:33
TsvikiHirsh added a commit to TsvikiHirsh/openmc that referenced this pull request Aug 10, 2026
PR openmc-dev#4040 (included in v0.16.0) inlined positron treatment:
annihilation photons from pair production are now emitted in
process_charged_secondary at the parent photon collision site, and
sample_positron_reaction is no longer on the pair-production path.
Move the next-event estimator hook for the two isotropic 511 keV
photons into the inline path (keeping the one in
sample_positron_reaction for banked positrons).
Regenerate point detector regression references: the corrected Compton
shell selection and Doppler broadening from openmc-dev#4036 shifts the
scattered-flux bins. Re-verified against thin-shell track-length
tallies: total flux agrees to 0.3% inside water and in vacuum, and the
511 keV annihilation line from 6.5 MeV photons on iron agrees to 0.8%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugsMerging SoonPR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paulromano@amandalund@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Correct Compton shell selection and Doppler broadening - #4036

Merged
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix
Jul 31, 2026
Merged

Correct Compton shell selection and Doppler broadening#4036
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix

Conversation

@paulromano

@paulromanopaulromano commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

The Compton Doppler-broadening algorithm on the current develop branch does not sample the bound-electron impulse approximation correctly. In particular, it selects a subshell using only the number of electrons in that shell. If the incident photon cannot ionize the selected shell, or if the kinematic upper bound on longitudinal electron momentum is negative, the code falls back to the free-electron Compton energy. However, a negative upper bound is not an invalid condition: it means that only part of the negative-momentum tail of the shell's Compton profile is accessible.

For shell $i$, the physically allowed longitudinal-momentum interval is

$$ -\frac{1}{\alpha_\mathrm{fs}} \le p_z \le p_{z,\max,i}, $$

where $\alpha_\mathrm{fs}$ is the fine-structure constant. The probability of selecting the shell is therefore proportional to

$$ P_i \propto f_i H(E-E_{b,i}) \int_{-1/\alpha_\mathrm{fs}}^{p_{z,\max,i}} J_i(p_z),dp_z, $$

where $f_i$ is the number of electrons in the shell, $E_{b,i}$ is its binding energy, and $J_i$ is its symmetric Compton profile. Thus, shell selection depends on both electron occupancy and the fraction of the profile that is kinematically accessible.

The develop implementation also samples only nonnegative $p_z$, chooses randomly between the two positive solutions of the scattered-energy quadratic, and does not apply the outgoing-energy rejection factor from the approximate relativistic impulse approximation. These choices distort the broadened energy distribution, particularly near shell thresholds and for forward scattering.

Relationship to #3874

#3874 correctly identifies that inaccessible shells should not be accepted and that negative values of $p_{z,\max}$ need to be handled. However, its proposed implementation has three blocking problems:

  1. The negative $p_{z,\max}$ branch samples outside the tabulated CDF. OpenMC stores only the nonnegative half of each symmetric Compton profile, so its integrated value approaches approximately $1/2$, not 1. For $p_{z,\max}&lt;0$, Fix a bug in compton scattering electron shell selection #3874samples a CDF value between the integral at $|p_{z,\max}|$ and 1. Values above the end of the half-profile CDF can make the subsequent lookup run past the profile table.

  2. The sign of $p_z$ is lost and the wrong energy root can be selected.Fix a bug in compton scattering electron shell selection #3874 inverts the profile using $|p_{z,\max}|$ but leaves the sampled momentum nonnegative. It then always selects the lower positive root of the scattered-energy quadratic. The physical branches are instead determined by the sign of the sampled momentum: $p_z&lt;0$ corresponds to an energy below the free-electron Compton energy and requires the lower root, whereas $p_z&gt;0$ corresponds to an energy above it and requires the upper root.

  3. Shell selection is not weighted by the accessible profile mass.Fix a bug in compton scattering electron shell selection #3874samples shells by electron occupancy and rejects only on the binding threshold. That threshold check is necessary but insufficient. At fixed energy and angle, different shells have different values of $p_{z,\max,i}$ and therefore different accessible fractions of their momentum distributions. Selecting among all energetically open shells using electron occupancy alone produces the wrong shell probabilities.

Proposed approach

This PR replaces the existing sampler with the approximate relativistic impulse approximation procedure described in Sec. 3.4.8 of Kaltiaisenaho's Master's thesis:

The implementation uses a two-stage strategy to retain the accuracy of this procedure without paying the full cost of constructing the conditional shell distribution for ordinary collisions. It first makes two inexpensive occupancy-based shell proposals and accepts each according to its accessible profile mass. If neither proposal succeeds, it evaluates the kinematics for every shell and samples directly from the conditional mass function $f_i M_i$, where $M_i$ is the accessible profile mass. This is statistically equivalent to repeated shell rejection but avoids pathological runtimes when every shell has very little accessible mass, such as near forward scattering.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great PR!
I have a few concerns and suggestions.

Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
paulromanoand others added 2 commits July 30, 2026 11:01
Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>
@paulromano

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @GuySten. Your comments/suggestions have been addressed.

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks @paulromano for addressing the problem.

@GuyStenGuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jul 30, 2026

@amandalundamandalund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice correction @paulromano! The physics looks sound to me.

@GuySten
GuySten merged commit a815267 into openmc-dev:developJul 31, 2026
18 checks passed
@paulromano
paulromano deleted the compton-alternate-fix branch July 31, 2026 13:33
TsvikiHirsh added a commit to TsvikiHirsh/openmc that referenced this pull request Aug 10, 2026
PR openmc-dev#4040 (included in v0.16.0) inlined positron treatment:
annihilation photons from pair production are now emitted in
process_charged_secondary at the parent photon collision site, and
sample_positron_reaction is no longer on the pair-production path.
Move the next-event estimator hook for the two isotropic 511 keV
photons into the inline path (keeping the one in
sample_positron_reaction for banked positrons).
Regenerate point detector regression references: the corrected Compton
shell selection and Doppler broadening from openmc-dev#4036 shifts the
scattered-flux bins. Re-verified against thin-shell track-length
tallies: total flux agrees to 0.3% inside water and in vacuum, and the
511 keV annihilation line from 6.5 MeV photons on iron agrees to 0.8%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugsMerging SoonPR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paulromano@amandalund@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Correct Compton shell selection and Doppler broadening - #4036

Merged
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix
Jul 31, 2026
Merged

Correct Compton shell selection and Doppler broadening#4036
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix

Conversation

@paulromano

@paulromanopaulromano commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

The Compton Doppler-broadening algorithm on the current develop branch does not sample the bound-electron impulse approximation correctly. In particular, it selects a subshell using only the number of electrons in that shell. If the incident photon cannot ionize the selected shell, or if the kinematic upper bound on longitudinal electron momentum is negative, the code falls back to the free-electron Compton energy. However, a negative upper bound is not an invalid condition: it means that only part of the negative-momentum tail of the shell's Compton profile is accessible.

For shell $i$, the physically allowed longitudinal-momentum interval is

$$ -\frac{1}{\alpha_\mathrm{fs}} \le p_z \le p_{z,\max,i}, $$

where $\alpha_\mathrm{fs}$ is the fine-structure constant. The probability of selecting the shell is therefore proportional to

$$ P_i \propto f_i H(E-E_{b,i}) \int_{-1/\alpha_\mathrm{fs}}^{p_{z,\max,i}} J_i(p_z),dp_z, $$

where $f_i$ is the number of electrons in the shell, $E_{b,i}$ is its binding energy, and $J_i$ is its symmetric Compton profile. Thus, shell selection depends on both electron occupancy and the fraction of the profile that is kinematically accessible.

The develop implementation also samples only nonnegative $p_z$, chooses randomly between the two positive solutions of the scattered-energy quadratic, and does not apply the outgoing-energy rejection factor from the approximate relativistic impulse approximation. These choices distort the broadened energy distribution, particularly near shell thresholds and for forward scattering.

Relationship to #3874

#3874 correctly identifies that inaccessible shells should not be accepted and that negative values of $p_{z,\max}$ need to be handled. However, its proposed implementation has three blocking problems:

  1. The negative $p_{z,\max}$ branch samples outside the tabulated CDF. OpenMC stores only the nonnegative half of each symmetric Compton profile, so its integrated value approaches approximately $1/2$, not 1. For $p_{z,\max}&lt;0$, Fix a bug in compton scattering electron shell selection #3874samples a CDF value between the integral at $|p_{z,\max}|$ and 1. Values above the end of the half-profile CDF can make the subsequent lookup run past the profile table.

  2. The sign of $p_z$ is lost and the wrong energy root can be selected.Fix a bug in compton scattering electron shell selection #3874 inverts the profile using $|p_{z,\max}|$ but leaves the sampled momentum nonnegative. It then always selects the lower positive root of the scattered-energy quadratic. The physical branches are instead determined by the sign of the sampled momentum: $p_z&lt;0$ corresponds to an energy below the free-electron Compton energy and requires the lower root, whereas $p_z&gt;0$ corresponds to an energy above it and requires the upper root.

  3. Shell selection is not weighted by the accessible profile mass.Fix a bug in compton scattering electron shell selection #3874samples shells by electron occupancy and rejects only on the binding threshold. That threshold check is necessary but insufficient. At fixed energy and angle, different shells have different values of $p_{z,\max,i}$ and therefore different accessible fractions of their momentum distributions. Selecting among all energetically open shells using electron occupancy alone produces the wrong shell probabilities.

Proposed approach

This PR replaces the existing sampler with the approximate relativistic impulse approximation procedure described in Sec. 3.4.8 of Kaltiaisenaho's Master's thesis:

The implementation uses a two-stage strategy to retain the accuracy of this procedure without paying the full cost of constructing the conditional shell distribution for ordinary collisions. It first makes two inexpensive occupancy-based shell proposals and accepts each according to its accessible profile mass. If neither proposal succeeds, it evaluates the kinematics for every shell and samples directly from the conditional mass function $f_i M_i$, where $M_i$ is the accessible profile mass. This is statistically equivalent to repeated shell rejection but avoids pathological runtimes when every shell has very little accessible mass, such as near forward scattering.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great PR!
I have a few concerns and suggestions.

Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
paulromanoand others added 2 commits July 30, 2026 11:01
Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>
@paulromano

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @GuySten. Your comments/suggestions have been addressed.

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks @paulromano for addressing the problem.

@GuyStenGuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jul 30, 2026

@amandalundamandalund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice correction @paulromano! The physics looks sound to me.

@GuySten
GuySten merged commit a815267 into openmc-dev:developJul 31, 2026
18 checks passed
@paulromano
paulromano deleted the compton-alternate-fix branch July 31, 2026 13:33
TsvikiHirsh added a commit to TsvikiHirsh/openmc that referenced this pull request Aug 10, 2026
PR openmc-dev#4040 (included in v0.16.0) inlined positron treatment:
annihilation photons from pair production are now emitted in
process_charged_secondary at the parent photon collision site, and
sample_positron_reaction is no longer on the pair-production path.
Move the next-event estimator hook for the two isotropic 511 keV
photons into the inline path (keeping the one in
sample_positron_reaction for banked positrons).
Regenerate point detector regression references: the corrected Compton
shell selection and Doppler broadening from openmc-dev#4036 shifts the
scattered-flux bins. Re-verified against thin-shell track-length
tallies: total flux agrees to 0.3% inside water and in vacuum, and the
511 keV annihilation line from 6.5 MeV photons on iron agrees to 0.8%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugsMerging SoonPR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paulromano@amandalund@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Correct Compton shell selection and Doppler broadening - #4036

Merged
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix
Jul 31, 2026
Merged

Correct Compton shell selection and Doppler broadening#4036
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix

Conversation

@paulromano

@paulromanopaulromano commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

The Compton Doppler-broadening algorithm on the current develop branch does not sample the bound-electron impulse approximation correctly. In particular, it selects a subshell using only the number of electrons in that shell. If the incident photon cannot ionize the selected shell, or if the kinematic upper bound on longitudinal electron momentum is negative, the code falls back to the free-electron Compton energy. However, a negative upper bound is not an invalid condition: it means that only part of the negative-momentum tail of the shell's Compton profile is accessible.

For shell $i$, the physically allowed longitudinal-momentum interval is

$$ -\frac{1}{\alpha_\mathrm{fs}} \le p_z \le p_{z,\max,i}, $$

where $\alpha_\mathrm{fs}$ is the fine-structure constant. The probability of selecting the shell is therefore proportional to

$$ P_i \propto f_i H(E-E_{b,i}) \int_{-1/\alpha_\mathrm{fs}}^{p_{z,\max,i}} J_i(p_z),dp_z, $$

where $f_i$ is the number of electrons in the shell, $E_{b,i}$ is its binding energy, and $J_i$ is its symmetric Compton profile. Thus, shell selection depends on both electron occupancy and the fraction of the profile that is kinematically accessible.

The develop implementation also samples only nonnegative $p_z$, chooses randomly between the two positive solutions of the scattered-energy quadratic, and does not apply the outgoing-energy rejection factor from the approximate relativistic impulse approximation. These choices distort the broadened energy distribution, particularly near shell thresholds and for forward scattering.

Relationship to #3874

#3874 correctly identifies that inaccessible shells should not be accepted and that negative values of $p_{z,\max}$ need to be handled. However, its proposed implementation has three blocking problems:

  1. The negative $p_{z,\max}$ branch samples outside the tabulated CDF. OpenMC stores only the nonnegative half of each symmetric Compton profile, so its integrated value approaches approximately $1/2$, not 1. For $p_{z,\max}&lt;0$, Fix a bug in compton scattering electron shell selection #3874samples a CDF value between the integral at $|p_{z,\max}|$ and 1. Values above the end of the half-profile CDF can make the subsequent lookup run past the profile table.

  2. The sign of $p_z$ is lost and the wrong energy root can be selected.Fix a bug in compton scattering electron shell selection #3874 inverts the profile using $|p_{z,\max}|$ but leaves the sampled momentum nonnegative. It then always selects the lower positive root of the scattered-energy quadratic. The physical branches are instead determined by the sign of the sampled momentum: $p_z&lt;0$ corresponds to an energy below the free-electron Compton energy and requires the lower root, whereas $p_z&gt;0$ corresponds to an energy above it and requires the upper root.

  3. Shell selection is not weighted by the accessible profile mass.Fix a bug in compton scattering electron shell selection #3874samples shells by electron occupancy and rejects only on the binding threshold. That threshold check is necessary but insufficient. At fixed energy and angle, different shells have different values of $p_{z,\max,i}$ and therefore different accessible fractions of their momentum distributions. Selecting among all energetically open shells using electron occupancy alone produces the wrong shell probabilities.

Proposed approach

This PR replaces the existing sampler with the approximate relativistic impulse approximation procedure described in Sec. 3.4.8 of Kaltiaisenaho's Master's thesis:

The implementation uses a two-stage strategy to retain the accuracy of this procedure without paying the full cost of constructing the conditional shell distribution for ordinary collisions. It first makes two inexpensive occupancy-based shell proposals and accepts each according to its accessible profile mass. If neither proposal succeeds, it evaluates the kinematics for every shell and samples directly from the conditional mass function $f_i M_i$, where $M_i$ is the accessible profile mass. This is statistically equivalent to repeated shell rejection but avoids pathological runtimes when every shell has very little accessible mass, such as near forward scattering.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great PR!
I have a few concerns and suggestions.

Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
paulromanoand others added 2 commits July 30, 2026 11:01
Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>
@paulromano

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @GuySten. Your comments/suggestions have been addressed.

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks @paulromano for addressing the problem.

@GuyStenGuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jul 30, 2026

@amandalundamandalund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice correction @paulromano! The physics looks sound to me.

@GuySten
GuySten merged commit a815267 into openmc-dev:developJul 31, 2026
18 checks passed
@paulromano
paulromano deleted the compton-alternate-fix branch July 31, 2026 13:33
TsvikiHirsh added a commit to TsvikiHirsh/openmc that referenced this pull request Aug 10, 2026
PR openmc-dev#4040 (included in v0.16.0) inlined positron treatment:
annihilation photons from pair production are now emitted in
process_charged_secondary at the parent photon collision site, and
sample_positron_reaction is no longer on the pair-production path.
Move the next-event estimator hook for the two isotropic 511 keV
photons into the inline path (keeping the one in
sample_positron_reaction for banked positrons).
Regenerate point detector regression references: the corrected Compton
shell selection and Doppler broadening from openmc-dev#4036 shifts the
scattered-flux bins. Re-verified against thin-shell track-length
tallies: total flux agrees to 0.3% inside water and in vacuum, and the
511 keV annihilation line from 6.5 MeV photons on iron agrees to 0.8%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugsMerging SoonPR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paulromano@amandalund@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Correct Compton shell selection and Doppler broadening - #4036

Merged
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix
Jul 31, 2026
Merged

Correct Compton shell selection and Doppler broadening#4036
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix

Conversation

@paulromano

@paulromanopaulromano commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

The Compton Doppler-broadening algorithm on the current develop branch does not sample the bound-electron impulse approximation correctly. In particular, it selects a subshell using only the number of electrons in that shell. If the incident photon cannot ionize the selected shell, or if the kinematic upper bound on longitudinal electron momentum is negative, the code falls back to the free-electron Compton energy. However, a negative upper bound is not an invalid condition: it means that only part of the negative-momentum tail of the shell's Compton profile is accessible.

For shell $i$, the physically allowed longitudinal-momentum interval is

$$ -\frac{1}{\alpha_\mathrm{fs}} \le p_z \le p_{z,\max,i}, $$

where $\alpha_\mathrm{fs}$ is the fine-structure constant. The probability of selecting the shell is therefore proportional to

$$ P_i \propto f_i H(E-E_{b,i}) \int_{-1/\alpha_\mathrm{fs}}^{p_{z,\max,i}} J_i(p_z),dp_z, $$

where $f_i$ is the number of electrons in the shell, $E_{b,i}$ is its binding energy, and $J_i$ is its symmetric Compton profile. Thus, shell selection depends on both electron occupancy and the fraction of the profile that is kinematically accessible.

The develop implementation also samples only nonnegative $p_z$, chooses randomly between the two positive solutions of the scattered-energy quadratic, and does not apply the outgoing-energy rejection factor from the approximate relativistic impulse approximation. These choices distort the broadened energy distribution, particularly near shell thresholds and for forward scattering.

Relationship to #3874

#3874 correctly identifies that inaccessible shells should not be accepted and that negative values of $p_{z,\max}$ need to be handled. However, its proposed implementation has three blocking problems:

  1. The negative $p_{z,\max}$ branch samples outside the tabulated CDF. OpenMC stores only the nonnegative half of each symmetric Compton profile, so its integrated value approaches approximately $1/2$, not 1. For $p_{z,\max}&lt;0$, Fix a bug in compton scattering electron shell selection #3874samples a CDF value between the integral at $|p_{z,\max}|$ and 1. Values above the end of the half-profile CDF can make the subsequent lookup run past the profile table.

  2. The sign of $p_z$ is lost and the wrong energy root can be selected.Fix a bug in compton scattering electron shell selection #3874 inverts the profile using $|p_{z,\max}|$ but leaves the sampled momentum nonnegative. It then always selects the lower positive root of the scattered-energy quadratic. The physical branches are instead determined by the sign of the sampled momentum: $p_z&lt;0$ corresponds to an energy below the free-electron Compton energy and requires the lower root, whereas $p_z&gt;0$ corresponds to an energy above it and requires the upper root.

  3. Shell selection is not weighted by the accessible profile mass.Fix a bug in compton scattering electron shell selection #3874samples shells by electron occupancy and rejects only on the binding threshold. That threshold check is necessary but insufficient. At fixed energy and angle, different shells have different values of $p_{z,\max,i}$ and therefore different accessible fractions of their momentum distributions. Selecting among all energetically open shells using electron occupancy alone produces the wrong shell probabilities.

Proposed approach

This PR replaces the existing sampler with the approximate relativistic impulse approximation procedure described in Sec. 3.4.8 of Kaltiaisenaho's Master's thesis:

The implementation uses a two-stage strategy to retain the accuracy of this procedure without paying the full cost of constructing the conditional shell distribution for ordinary collisions. It first makes two inexpensive occupancy-based shell proposals and accepts each according to its accessible profile mass. If neither proposal succeeds, it evaluates the kinematics for every shell and samples directly from the conditional mass function $f_i M_i$, where $M_i$ is the accessible profile mass. This is statistically equivalent to repeated shell rejection but avoids pathological runtimes when every shell has very little accessible mass, such as near forward scattering.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great PR!
I have a few concerns and suggestions.

Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
paulromanoand others added 2 commits July 30, 2026 11:01
Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>
@paulromano

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @GuySten. Your comments/suggestions have been addressed.

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks @paulromano for addressing the problem.

@GuyStenGuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jul 30, 2026

@amandalundamandalund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice correction @paulromano! The physics looks sound to me.

@GuySten
GuySten merged commit a815267 into openmc-dev:developJul 31, 2026
18 checks passed
@paulromano
paulromano deleted the compton-alternate-fix branch July 31, 2026 13:33
TsvikiHirsh added a commit to TsvikiHirsh/openmc that referenced this pull request Aug 10, 2026
PR openmc-dev#4040 (included in v0.16.0) inlined positron treatment:
annihilation photons from pair production are now emitted in
process_charged_secondary at the parent photon collision site, and
sample_positron_reaction is no longer on the pair-production path.
Move the next-event estimator hook for the two isotropic 511 keV
photons into the inline path (keeping the one in
sample_positron_reaction for banked positrons).
Regenerate point detector regression references: the corrected Compton
shell selection and Doppler broadening from openmc-dev#4036 shifts the
scattered-flux bins. Re-verified against thin-shell track-length
tallies: total flux agrees to 0.3% inside water and in vacuum, and the
511 keV annihilation line from 6.5 MeV photons on iron agrees to 0.8%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugsMerging SoonPR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paulromano@amandalund@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Correct Compton shell selection and Doppler broadening - #4036

Merged
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix
Jul 31, 2026
Merged

Correct Compton shell selection and Doppler broadening#4036
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix

Conversation

@paulromano

@paulromanopaulromano commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

The Compton Doppler-broadening algorithm on the current develop branch does not sample the bound-electron impulse approximation correctly. In particular, it selects a subshell using only the number of electrons in that shell. If the incident photon cannot ionize the selected shell, or if the kinematic upper bound on longitudinal electron momentum is negative, the code falls back to the free-electron Compton energy. However, a negative upper bound is not an invalid condition: it means that only part of the negative-momentum tail of the shell's Compton profile is accessible.

For shell $i$, the physically allowed longitudinal-momentum interval is

$$ -\frac{1}{\alpha_\mathrm{fs}} \le p_z \le p_{z,\max,i}, $$

where $\alpha_\mathrm{fs}$ is the fine-structure constant. The probability of selecting the shell is therefore proportional to

$$ P_i \propto f_i H(E-E_{b,i}) \int_{-1/\alpha_\mathrm{fs}}^{p_{z,\max,i}} J_i(p_z),dp_z, $$

where $f_i$ is the number of electrons in the shell, $E_{b,i}$ is its binding energy, and $J_i$ is its symmetric Compton profile. Thus, shell selection depends on both electron occupancy and the fraction of the profile that is kinematically accessible.

The develop implementation also samples only nonnegative $p_z$, chooses randomly between the two positive solutions of the scattered-energy quadratic, and does not apply the outgoing-energy rejection factor from the approximate relativistic impulse approximation. These choices distort the broadened energy distribution, particularly near shell thresholds and for forward scattering.

Relationship to #3874

#3874 correctly identifies that inaccessible shells should not be accepted and that negative values of $p_{z,\max}$ need to be handled. However, its proposed implementation has three blocking problems:

  1. The negative $p_{z,\max}$ branch samples outside the tabulated CDF. OpenMC stores only the nonnegative half of each symmetric Compton profile, so its integrated value approaches approximately $1/2$, not 1. For $p_{z,\max}&lt;0$, Fix a bug in compton scattering electron shell selection #3874samples a CDF value between the integral at $|p_{z,\max}|$ and 1. Values above the end of the half-profile CDF can make the subsequent lookup run past the profile table.

  2. The sign of $p_z$ is lost and the wrong energy root can be selected.Fix a bug in compton scattering electron shell selection #3874 inverts the profile using $|p_{z,\max}|$ but leaves the sampled momentum nonnegative. It then always selects the lower positive root of the scattered-energy quadratic. The physical branches are instead determined by the sign of the sampled momentum: $p_z&lt;0$ corresponds to an energy below the free-electron Compton energy and requires the lower root, whereas $p_z&gt;0$ corresponds to an energy above it and requires the upper root.

  3. Shell selection is not weighted by the accessible profile mass.Fix a bug in compton scattering electron shell selection #3874samples shells by electron occupancy and rejects only on the binding threshold. That threshold check is necessary but insufficient. At fixed energy and angle, different shells have different values of $p_{z,\max,i}$ and therefore different accessible fractions of their momentum distributions. Selecting among all energetically open shells using electron occupancy alone produces the wrong shell probabilities.

Proposed approach

This PR replaces the existing sampler with the approximate relativistic impulse approximation procedure described in Sec. 3.4.8 of Kaltiaisenaho's Master's thesis:

The implementation uses a two-stage strategy to retain the accuracy of this procedure without paying the full cost of constructing the conditional shell distribution for ordinary collisions. It first makes two inexpensive occupancy-based shell proposals and accepts each according to its accessible profile mass. If neither proposal succeeds, it evaluates the kinematics for every shell and samples directly from the conditional mass function $f_i M_i$, where $M_i$ is the accessible profile mass. This is statistically equivalent to repeated shell rejection but avoids pathological runtimes when every shell has very little accessible mass, such as near forward scattering.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great PR!
I have a few concerns and suggestions.

Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
paulromanoand others added 2 commits July 30, 2026 11:01
Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>
@paulromano

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @GuySten. Your comments/suggestions have been addressed.

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks @paulromano for addressing the problem.

@GuyStenGuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jul 30, 2026

@amandalundamandalund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice correction @paulromano! The physics looks sound to me.

@GuySten
GuySten merged commit a815267 into openmc-dev:developJul 31, 2026
18 checks passed
@paulromano
paulromano deleted the compton-alternate-fix branch July 31, 2026 13:33
TsvikiHirsh added a commit to TsvikiHirsh/openmc that referenced this pull request Aug 10, 2026
PR openmc-dev#4040 (included in v0.16.0) inlined positron treatment:
annihilation photons from pair production are now emitted in
process_charged_secondary at the parent photon collision site, and
sample_positron_reaction is no longer on the pair-production path.
Move the next-event estimator hook for the two isotropic 511 keV
photons into the inline path (keeping the one in
sample_positron_reaction for banked positrons).
Regenerate point detector regression references: the corrected Compton
shell selection and Doppler broadening from openmc-dev#4036 shifts the
scattered-flux bins. Re-verified against thin-shell track-length
tallies: total flux agrees to 0.3% inside water and in vacuum, and the
511 keV annihilation line from 6.5 MeV photons on iron agrees to 0.8%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugsMerging SoonPR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paulromano@amandalund@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Correct Compton shell selection and Doppler broadening - #4036

Merged
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix
Jul 31, 2026
Merged

Correct Compton shell selection and Doppler broadening#4036
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix

Conversation

@paulromano

@paulromanopaulromano commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

The Compton Doppler-broadening algorithm on the current develop branch does not sample the bound-electron impulse approximation correctly. In particular, it selects a subshell using only the number of electrons in that shell. If the incident photon cannot ionize the selected shell, or if the kinematic upper bound on longitudinal electron momentum is negative, the code falls back to the free-electron Compton energy. However, a negative upper bound is not an invalid condition: it means that only part of the negative-momentum tail of the shell's Compton profile is accessible.

For shell $i$, the physically allowed longitudinal-momentum interval is

$$ -\frac{1}{\alpha_\mathrm{fs}} \le p_z \le p_{z,\max,i}, $$

where $\alpha_\mathrm{fs}$ is the fine-structure constant. The probability of selecting the shell is therefore proportional to

$$ P_i \propto f_i H(E-E_{b,i}) \int_{-1/\alpha_\mathrm{fs}}^{p_{z,\max,i}} J_i(p_z),dp_z, $$

where $f_i$ is the number of electrons in the shell, $E_{b,i}$ is its binding energy, and $J_i$ is its symmetric Compton profile. Thus, shell selection depends on both electron occupancy and the fraction of the profile that is kinematically accessible.

The develop implementation also samples only nonnegative $p_z$, chooses randomly between the two positive solutions of the scattered-energy quadratic, and does not apply the outgoing-energy rejection factor from the approximate relativistic impulse approximation. These choices distort the broadened energy distribution, particularly near shell thresholds and for forward scattering.

Relationship to #3874

#3874 correctly identifies that inaccessible shells should not be accepted and that negative values of $p_{z,\max}$ need to be handled. However, its proposed implementation has three blocking problems:

  1. The negative $p_{z,\max}$ branch samples outside the tabulated CDF. OpenMC stores only the nonnegative half of each symmetric Compton profile, so its integrated value approaches approximately $1/2$, not 1. For $p_{z,\max}&lt;0$, Fix a bug in compton scattering electron shell selection #3874samples a CDF value between the integral at $|p_{z,\max}|$ and 1. Values above the end of the half-profile CDF can make the subsequent lookup run past the profile table.

  2. The sign of $p_z$ is lost and the wrong energy root can be selected.Fix a bug in compton scattering electron shell selection #3874 inverts the profile using $|p_{z,\max}|$ but leaves the sampled momentum nonnegative. It then always selects the lower positive root of the scattered-energy quadratic. The physical branches are instead determined by the sign of the sampled momentum: $p_z&lt;0$ corresponds to an energy below the free-electron Compton energy and requires the lower root, whereas $p_z&gt;0$ corresponds to an energy above it and requires the upper root.

  3. Shell selection is not weighted by the accessible profile mass.Fix a bug in compton scattering electron shell selection #3874samples shells by electron occupancy and rejects only on the binding threshold. That threshold check is necessary but insufficient. At fixed energy and angle, different shells have different values of $p_{z,\max,i}$ and therefore different accessible fractions of their momentum distributions. Selecting among all energetically open shells using electron occupancy alone produces the wrong shell probabilities.

Proposed approach

This PR replaces the existing sampler with the approximate relativistic impulse approximation procedure described in Sec. 3.4.8 of Kaltiaisenaho's Master's thesis:

The implementation uses a two-stage strategy to retain the accuracy of this procedure without paying the full cost of constructing the conditional shell distribution for ordinary collisions. It first makes two inexpensive occupancy-based shell proposals and accepts each according to its accessible profile mass. If neither proposal succeeds, it evaluates the kinematics for every shell and samples directly from the conditional mass function $f_i M_i$, where $M_i$ is the accessible profile mass. This is statistically equivalent to repeated shell rejection but avoids pathological runtimes when every shell has very little accessible mass, such as near forward scattering.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great PR!
I have a few concerns and suggestions.

Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
paulromanoand others added 2 commits July 30, 2026 11:01
Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>
@paulromano

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @GuySten. Your comments/suggestions have been addressed.

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks @paulromano for addressing the problem.

@GuyStenGuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jul 30, 2026

@amandalundamandalund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice correction @paulromano! The physics looks sound to me.

@GuySten
GuySten merged commit a815267 into openmc-dev:developJul 31, 2026
18 checks passed
@paulromano
paulromano deleted the compton-alternate-fix branch July 31, 2026 13:33
TsvikiHirsh added a commit to TsvikiHirsh/openmc that referenced this pull request Aug 10, 2026
PR openmc-dev#4040 (included in v0.16.0) inlined positron treatment:
annihilation photons from pair production are now emitted in
process_charged_secondary at the parent photon collision site, and
sample_positron_reaction is no longer on the pair-production path.
Move the next-event estimator hook for the two isotropic 511 keV
photons into the inline path (keeping the one in
sample_positron_reaction for banked positrons).
Regenerate point detector regression references: the corrected Compton
shell selection and Doppler broadening from openmc-dev#4036 shifts the
scattered-flux bins. Re-verified against thin-shell track-length
tallies: total flux agrees to 0.3% inside water and in vacuum, and the
511 keV annihilation line from 6.5 MeV photons on iron agrees to 0.8%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugsMerging SoonPR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paulromano@amandalund@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Correct Compton shell selection and Doppler broadening - #4036

Merged
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix
Jul 31, 2026
Merged

Correct Compton shell selection and Doppler broadening#4036
GuySten merged 9 commits into
openmc-dev:developfrom
paulromano:compton-alternate-fix

Conversation

@paulromano

@paulromanopaulromano commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

The Compton Doppler-broadening algorithm on the current develop branch does not sample the bound-electron impulse approximation correctly. In particular, it selects a subshell using only the number of electrons in that shell. If the incident photon cannot ionize the selected shell, or if the kinematic upper bound on longitudinal electron momentum is negative, the code falls back to the free-electron Compton energy. However, a negative upper bound is not an invalid condition: it means that only part of the negative-momentum tail of the shell's Compton profile is accessible.

For shell $i$, the physically allowed longitudinal-momentum interval is

$$ -\frac{1}{\alpha_\mathrm{fs}} \le p_z \le p_{z,\max,i}, $$

where $\alpha_\mathrm{fs}$ is the fine-structure constant. The probability of selecting the shell is therefore proportional to

$$ P_i \propto f_i H(E-E_{b,i}) \int_{-1/\alpha_\mathrm{fs}}^{p_{z,\max,i}} J_i(p_z),dp_z, $$

where $f_i$ is the number of electrons in the shell, $E_{b,i}$ is its binding energy, and $J_i$ is its symmetric Compton profile. Thus, shell selection depends on both electron occupancy and the fraction of the profile that is kinematically accessible.

The develop implementation also samples only nonnegative $p_z$, chooses randomly between the two positive solutions of the scattered-energy quadratic, and does not apply the outgoing-energy rejection factor from the approximate relativistic impulse approximation. These choices distort the broadened energy distribution, particularly near shell thresholds and for forward scattering.

Relationship to #3874

#3874 correctly identifies that inaccessible shells should not be accepted and that negative values of $p_{z,\max}$ need to be handled. However, its proposed implementation has three blocking problems:

  1. The negative $p_{z,\max}$ branch samples outside the tabulated CDF. OpenMC stores only the nonnegative half of each symmetric Compton profile, so its integrated value approaches approximately $1/2$, not 1. For $p_{z,\max}&lt;0$, Fix a bug in compton scattering electron shell selection #3874samples a CDF value between the integral at $|p_{z,\max}|$ and 1. Values above the end of the half-profile CDF can make the subsequent lookup run past the profile table.

  2. The sign of $p_z$ is lost and the wrong energy root can be selected.Fix a bug in compton scattering electron shell selection #3874 inverts the profile using $|p_{z,\max}|$ but leaves the sampled momentum nonnegative. It then always selects the lower positive root of the scattered-energy quadratic. The physical branches are instead determined by the sign of the sampled momentum: $p_z&lt;0$ corresponds to an energy below the free-electron Compton energy and requires the lower root, whereas $p_z&gt;0$ corresponds to an energy above it and requires the upper root.

  3. Shell selection is not weighted by the accessible profile mass.Fix a bug in compton scattering electron shell selection #3874samples shells by electron occupancy and rejects only on the binding threshold. That threshold check is necessary but insufficient. At fixed energy and angle, different shells have different values of $p_{z,\max,i}$ and therefore different accessible fractions of their momentum distributions. Selecting among all energetically open shells using electron occupancy alone produces the wrong shell probabilities.

Proposed approach

This PR replaces the existing sampler with the approximate relativistic impulse approximation procedure described in Sec. 3.4.8 of Kaltiaisenaho's Master's thesis:

The implementation uses a two-stage strategy to retain the accuracy of this procedure without paying the full cost of constructing the conditional shell distribution for ordinary collisions. It first makes two inexpensive occupancy-based shell proposals and accepts each according to its accessible profile mass. If neither proposal succeeds, it evaluates the kinematics for every shell and samples directly from the conditional mass function $f_i M_i$, where $M_i$ is the accessible profile mass. This is statistically equivalent to repeated shell rejection but avoids pathological runtimes when every shell has very little accessible mass, such as near forward scattering.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great PR!
I have a few concerns and suggestions.

Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadinclude/openmc/photon.h Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp
Comment threadsrc/photon.cpp Outdated
Comment threadsrc/photon.cpp Outdated
paulromanoand others added 2 commits July 30, 2026 11:01
Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>
@paulromano

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @GuySten. Your comments/suggestions have been addressed.

@GuyStenGuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks @paulromano for addressing the problem.

@GuyStenGuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jul 30, 2026

@amandalundamandalund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice correction @paulromano! The physics looks sound to me.

@GuySten
GuySten merged commit a815267 into openmc-dev:developJul 31, 2026
18 checks passed
@paulromano
paulromano deleted the compton-alternate-fix branch July 31, 2026 13:33
TsvikiHirsh added a commit to TsvikiHirsh/openmc that referenced this pull request Aug 10, 2026
PR openmc-dev#4040 (included in v0.16.0) inlined positron treatment:
annihilation photons from pair production are now emitted in
process_charged_secondary at the parent photon collision site, and
sample_positron_reaction is no longer on the pair-production path.
Move the next-event estimator hook for the two isotropic 511 keV
photons into the inline path (keeping the one in
sample_positron_reaction for banked positrons).
Regenerate point detector regression references: the corrected Compton
shell selection and Doppler broadening from openmc-dev#4036 shifts the
scattered-flux bins. Re-verified against thin-shell track-length
tallies: total flux agrees to 0.3% inside water and in vacuum, and the
511 keV annihilation line from 6.5 MeV photons on iron agrees to 0.8%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugsMerging SoonPR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paulromano@amandalund@GuySten