Skip to content

Fix negative distances from bins_crossed for CylindricalMesh #3370

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

Merged
merged 7 commits into from
Apr 23, 2025

Conversation

pshriwise
Copy link
Contributor

Description

For some context, during the process of tracing a track segment along the mesh elements, the PeriodicMesh::distance_to_grid_boundary method uses the initial position as-is, while the get_indices transforms the initial coordinates to local coordinates by default. I'd like to change this to be something more consistent, but I'll save that for another PR.

Two problems are fixed here:

  1. Coordinates were adjusted for the origin more than once in the process of calling bins_crossed for cylindrical meshes. They are adjusted in once raytrace_mesh and again in the CylindricalMesh implementation of distance_to_grid_boundary. The additional transformation in distance_to_grid_boundary has been removed.
  2. When a particle partially overlaps the mesh, a get_indices call is used to determine which element the particle passes into (if it does at all). The coordinates passed have already been transformed and an additional, incorrect, transformation is applied within this method.

I've updated the filter_mesh regression test to use an origin with a z-value and adjusted the z grid of the original mesh to obtain an equivalent mesh with an origin. Without the changes here in mesh.cpp the regression results failed produced negative flux values. With the changes applied the test passes without changes to the regression results.

Fixes #3369

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)

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 the PR @pshriwise. See comment below regarding the local_coords method:

@paulromano paulromano merged commit 5dd6ff6 into openmc-dev:develop Apr 23, 2025
15 checks passed
@RemDelaporteMathurin
Copy link
Contributor

@pshriwise @paulromano do we know when the next release containing this fix will be out?

@shimwell
Copy link
Member

Tracking releases history I see over the last 137 months there have been 22 releases of OpenMC, averaging a release every 6 months. Assuming this continues then I would anticipate a release around mid October.

Hopefully before that we will have a pip install where you can install the latest version of the repo with a command like this
pip install git+https://github.com/openmc-dev/openmc.

In the meantime building from source is always an option.

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.

Negative scores with CylindricalMesh filter
4 participants