Let a mesh surface crossing change more than one index - #4096
Open
GuySten wants to merge 1 commit into
Open
Conversation
GuySten
marked this pull request as ready for review
August 31, 2026 21:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Let a mesh surface crossing change more than one index
Pure refactor of
StructuredMesh. No behaviour change, no results change, noregolds. Second of the pieces split out of #3857, and independent of it.
Problem
StructuredMesh::raytrace_meshassumes that crossing a surface changes exactlyone mesh index, and that the distance to the next surface on the other axes is
unaffected:
That is true of every mesh currently in the code, but it is a property of those
meshes rather than of structured meshes in general, and the assumption is spread
across three separate places:
MeshDistance::next_indexbeing a singleint,the hard-coded refresh of only
distances[k], and the inline bounds check.Two more things are entangled with it. Periodic index wrapping is done inside
each mesh's
distance_to_grid_boundary, sonext_indexis pre-sanitized:which means the value stored is not "where I am going" but "where I am going,
already folded". And the logic for re-entering the mesh from outside sits inline
in the middle of
raytrace_mesh.Changes
Three changes, each exercised by code in this PR.
MeshDistance::next_index(anint) becomesoffset(aMeshIndex). Acrossing now says how each index changes rather than naming a single
destination. The existing meshes each set one component, so
ijk[i] + 1becomes
{1, 0, 0}and so on.virtual void sanitize_index(MeshIndex&), called once after the index isadvanced.
CylindricalMeshandSphericalMeshoverride it with thesanitize_phi/sanitize_thetacalls they previously folded intodistance_to_grid_boundary. Default is a no-op.distance_to_mesh(...)extracts the re-entry block, moved verbatim, out ofthe middle of
raytrace_mesh.Plus
MESH_MAX_AXES, replacing the hard-coded3inMeshIndex,shape_andthe per-axis distance array -- three independent literals that all had to agree.
Why this is behaviour-preserving
offsetdefaults to{0, 0, 0}and the old code setnext_index = ijk[i]before any early return, so a crossing that does not happen leaves the index
unchanged either way.
sanitize_indexapplies the samesanitize_phi/sanitize_thetato thesame axis, just after the index is advanced instead of before it is stored.
distances[k]is still refreshed after a crossing.distance_to_meshis the previous expression, unmoved.Value independent of #3857
The periodic wrapping that cylindrical and spherical meshes do gets a name and
one place to live rather than being folded into each mesh's distance
calculation, where it made
next_indexmean two things at once. Anddistance_to_meshis now a separately readable and testable function ratherthan a block in the middle of the longest function in the file.
Performance
Benchmarked against a synthetic stress case -- a 50³ mesh surface tally, 2000 particles, single-threaded -- chosen so that mesh tallying dominates transport time. Cylindrical and spherical meshes show no resolvable change. A regular mesh shows a small increase, bounded at roughly 12% of transport in that configuration by the least favourable reading of the data and under 3% by the robust one; four repeats per arm is not enough to pin it down further. Real models, where transport is not dominated by a fine mesh surface tally, would see a fraction of that.
Testing
No results change, so the existing mesh regression and unit tests cover this
unchanged. Nothing new is executed and no allocation is added on the transport
path --
MeshIndexisstd::array<int, 3>, soMeshDistancegrows by two intsand stays a plain aggregate.
No new data members and no allocation.
MeshDistancegrows by two ints andstays a plain aggregate.
Checklist
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)