Skip to content

Conversation

@GuySten
Copy link
Contributor

@GuySten GuySten commented Dec 18, 2025

Description

This PR simplify and fix logic of periodic boundary conditions.
It prevents particles getting lost when the periodic surfaces have normals in any direction.
I also added tests that check that the code run correctly regardless of the periodic plane normal direction.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) 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)

@GuySten GuySten marked this pull request as draft December 18, 2025 17:42
@GuySten GuySten marked this pull request as ready for review December 18, 2025 21:16
@GuySten GuySten added the Bugs label Dec 19, 2025
@GuySten GuySten changed the title Simplify rotational periodic boundary conditions Fix a bug in rotational periodic boundary conditions Dec 19, 2025
@GuySten GuySten marked this pull request as draft December 19, 2025 06:14
@GuySten GuySten marked this pull request as ready for review December 19, 2025 10:07
Copy link
Contributor

@paulromano paulromano 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 this fix and simplification! The one thing that still doesn't seem quite right is the warning about the angle not evenly subdividing 360. Depending on the orientation of the surfaces and whether positive/negative half-spaces are used to define the region, sometimes that warning is shown even when the angle is fine. To illustrate that, I've put together this example with two planes that are created using Plane.from_points, where each set of three points is randomly permuted. The region being plotted is the same in each case but some permutations of the points end up with the warning and some don't.

import openmc
from math import cos, sin, pi, radians
import numpy as np
import random

start = 0.
degrees = 45.
ang1 = radians(start)
ang2 = radians(start + degrees)

p1_points = [(0., 0., 0.), (cos(ang1), sin(ang1), 0.), (0., 0., 1.)]
p2_points = [(0., 0., 0.), (cos(ang2), sin(ang2), 0.), (0., 0., 1.)]
random.shuffle(p1_points)
random.shuffle(p2_points)

p1 = openmc.Plane.from_points(*p1_points, boundary_type='periodic')
p2 = openmc.Plane.from_points(*p2_points, boundary_type='periodic')
p1.periodic_surface = p2
zcyl = openmc.ZCylinder(r=5., boundary_type='vacuum')

# Figure out which side of planes to use based on a point in the middle
ang_mid = radians(start + degrees/2.)
mid_point = (cos(ang_mid), sin(ang_mid), 0.)
r1 = -p1 if mid_point in -p1 else +p1
r2 = -p2 if mid_point in -p2 else +p2

(r1 & r2 & -zcyl).plot()

@GuySten
Copy link
Contributor Author

GuySten commented Dec 23, 2025

In my testing, the normalization of the angle range to [-PI,PI] fix the warning problem.

@GuySten GuySten requested a review from paulromano December 24, 2025 00:28
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