Skip to content

normalize new_u after periodic crossing to prevent unnormalized direction#4015

Open
lewisgross1296 wants to merge 1 commit into
openmc-dev:developfrom
lewisgross1296:normalize_after_periodic_crossing
Open

normalize new_u after periodic crossing to prevent unnormalized direction#4015
lewisgross1296 wants to merge 1 commit into
openmc-dev:developfrom
lewisgross1296:normalize_after_periodic_crossing

Conversation

@lewisgross1296

@lewisgross1296 lewisgross1296 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Description

Closes #3970. After many months and much support from @pshriwise and @gonuke, we've come to a resolution to an issue I was having with my model related to using a periodic BC aligned to the y-axis, see forum post.

It turns out that most boundary conditions in OpenMC re-normalize particle direction after they cross a boundary (e.g. reflective or white BCs) and that ~6 years ago, this was not done for the periodic case. It wasn't an issue for a while due to the restriction that models must use z-axis aligned periodicity. However, allowing arbitrary periodic BCs #3591 combined with logic in rotate_angle exposed a rare, but consistent, behavior that the un-normalized nature of the direction vector could grow beyond unity until eventually, a negative distance to collision gets computed. This negative distance results from an assumption of unity norm directions in the function logic. In this case, OpenMC reports a -nan batch and has a fatal error.

We've tried a lot to debug, however the most relevant is that

  1. When my model is rotated to use z-periodicity, the -nan goes away
  2. When my model remains y-aligned but the BC is changed from periodic to reflective, the -nan goes away
  3. Patrick was able to reproduce the -nan by removing the normalization in the reflective BC case in boundary_condition.cpp and re-running my y-aligned model while only changing the BC to reflective.

It seems there are very small amounts of floating point error picked up that magnify each time they cross a boundary or for lucky (unlucky?) directions interacting with rotate_angle's branching on the b parameter.

This solution will make the code more consistent with itself i.e. normalizing after any type of boundary crossing that is not transnational. Thankfully, this solution means that only periodic BC related tests need to be re-golded (which I will do in the next commit). I have a feeling of which tests should fail with this change (see below), but I'm going to let them fail just to be sure.

ligross@cnerg-docker-09:~/research/openmc/tests/regression_tests (normalize_after_periodic_crossing) $ ls per*
periodic:
__init__.py  inputs_true.dat  results_true.dat  test.py

periodic_6fold:
False-False  False-True  __init__.py  test.py  True-False  True-True

periodic_cyls:
__init__.py  test.py  xcyl_model  ycyl_model

periodic_hex:
__init__.py  inputs_true.dat  results_true.dat  test.py

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)

…ror cannot create an unnormalized direction vector. add comments to the other handle_particle methods, which already do the same
@lewisgross1296 lewisgross1296 force-pushed the normalize_after_periodic_crossing branch from a515e2f to f5cfcad Compare July 13, 2026 20:34
@lewisgross1296

Copy link
Copy Markdown
Contributor Author

I wasn't expecting tests to pass per se, but maybe the examples are not as affected by the change enough to affect the statistics to a noticeable degree? I.e. the effect of normalization when hitting the periodic surface isn't enough to bias k and its uncertainty in these examples.

@gonuke

gonuke commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

I wasn't expecting tests to pass per se, but maybe the examples are not as affected by the change enough to affect the statistics to a noticeable degree? I.e. the effect of normalization when hitting the periodic surface isn't enough to bias k and its uncertainty in these examples.

First, I'm pretty sure the periodic tests aren't being compared statistically, but are expected to be numerically exact.

That said, they each use only 4000-5000 histories total, so it conceivable that they never encounter a history where this normalization has an impact.

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.

Very rare event results in unnormalized direction vector leading to transport failure

2 participants