-
Couldn't load subscription status.
- Fork 576
Fix for translational periodic boundary conditions #1820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix for translational periodic boundary conditions #1820
Conversation
ff6f3f8 to
407c93a
Compare
407c93a to
898f052
Compare
|
I had to make a few more changes on this branch. First, I reverted the temporary hostname fix in #1816 since there is now an upstream fix on GitHub Actions VM images. I also discovered that the recent introduction of the
A slightly separate issue:
Bottom line — my solution to both these issues is to 1) rely only on OpenMPI and 2) make sure h5py is built against whatever HDF5 library is used to build OpenMC. This uncovered a bug in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, normal does not mean orthonormal as it turns out! Thanks for the fix. The changes to the code look good, and the additional test is nice.
|
I've run into the HDF5 version mismatch before. I wonder if it makes sense for us to essentially roll our own h5py to ensure that the transport solver and the Python API are both using the same HDF5 libs. We already hide the h5py calls so a switch would hopefully be seamless for our users, and we can probably reuse a lot of |
…odic-fix Fix for translational periodic boundary conditions
…odic-fix Fix for translational periodic boundary conditions
…odic-fix Fix for translational periodic boundary conditions
…odic-fix Fix for translational periodic boundary conditions
Fix for translational periodic boundary conditions
A user reported that a model with periodic boundary conditions used to work in 0.12.0 and now doesn't work in 0.12.1. It turns out that translational periodic boundary conditions on general planes (Ax + By + Cz = D) only work right now if √(A² + B² + C²) = 1. This PR fixes this so that the normalization of the surface coefficients doesn't matter. I've added a simple regression test that will catch this case in the future.