Skip to content

Set all tally estimators to collision in RR simulations - #4042

Open
nuclearkevin wants to merge 2 commits into
openmc-dev:developfrom
nuclearkevin:fix_rr_tally_map
Open

Set all tally estimators to collision in RR simulations#4042
nuclearkevin wants to merge 2 commits into
openmc-dev:developfrom
nuclearkevin:fix_rr_tally_map

Conversation

@nuclearkevin

Copy link
Copy Markdown
Member

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

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)

@nuclearkevin
nuclearkevin requested a review from jtramm as a code owner August 3, 2026 16:56

@GuySten GuySten 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.

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

@GuySten GuySten added the Bugs label Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants