Skip to content

Conversation

j-fletcher
Copy link
Contributor

@j-fletcher j-fletcher commented Aug 21, 2025

Description

Obtaining a sample $x$ from a power law distribution of degree $n$, defined over the interval $[a, b]$, requires evaluating the following:

$x = (a' + \xi\times\Delta)^{1/(n+1)}$,

where the offset $a' = a^{n+1}$, the span $\Delta = b^{n+1} - a'$, and $\xi$ is a (pseudo)random number sampled from $U[0, 1]$. In OpenMC, this operation uses numpy.power() when performed in Python and std::pow() in C++, both of which raise an error if a negative number is raised to a non-integer power.

Currently, there are no warnings against specifying an interval wherein $a' + \xi\times\Delta < 0$ for some value of $\xi$, and when std::pow() is called in such cases, OpenMC raises the following RuntimeError:

RuntimeError: Too few source sites satisfied the constraints (minimum source rejection fraction = 0.05). Please check your source definition or set a lower value of Settings.source_rejection_fraction.

This PR checks the values of $a$ and $b$ when a PowerLaw is created to make sure that the distribution is defined over a nonnegative-valued interval. This makes clear the cause of the above error before it occurs.

Fixes #3541

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (not 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 (not applicable)

Copy link
Contributor

@eepeterson eepeterson left a comment

Choose a reason for hiding this comment

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

Thanks for the added checks, to be consistent with the documentation, it might be worth adding an explicit check that b > a in the constructor so that we're positive that a is the lower bound and b is the upper bound. It doesn't actually change the distribution of sampled values if they are flipped I don't think, but it could be confusing.

@j-fletcher
Copy link
Contributor Author

Good idea--I've made that change in the PowerLaw constructor now.

@j-fletcher j-fletcher requested a review from eepeterson August 25, 2025 18:13
@paulromano paulromano enabled auto-merge (squash) September 8, 2025 19:12
@GuySten GuySten dismissed eepeterson’s stale review September 11, 2025 12:57

There requested changes are already fixed and another reviewer accepted them.

@paulromano paulromano merged commit c717528 into openmc-dev:develop Sep 11, 2025
18 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sampling from PowerLaw over negative-valued interval causes OpenMC to raise RuntimeError
3 participants