Skip to content

Add domain decomposition for random ray solver - #4026

Open
Suark94 wants to merge 65 commits into
openmc-dev:developfrom
Suark94:domain_decomp
Open

Suark94 wants to merge 65 commits into
openmc-dev:developfrom
Suark94:domain_decomp

Conversation

@Suark94

@Suark94 Suark94 commented Jul 23, 2026

Copy link
Copy Markdown

Description

This PR adds a domain decomposition capability to the random ray solver, allowing users to run random ray calculations in parallel across multiple MPI ranks. This lets random ray problems that are too large to fit in the memory of a single computational node be spread across multiple nodes and enables greater scalability on HPC clusters.

This feature is enabled automatically when the random ray solver is run with more than one MPI rank. No additional input settings are required. The geometry is decomposed with a capacity-constrained Voronoi tessellation, where each Voronoi region corresponds to one MPI rank's subdomain. A source region is assigned to a Voronoi region/ MPI rank based on the squared distance between the source region centroid and the Voronoi region centroid, adjusted with an additive weight. Using the weighted squared distance rather than the simple Euclidean distance yields compact, centroidal subdomains and prevents elongated or scattered subdomains. During the first 5 iterations, the weights are adjusted according to the measured rank load to balance the work across subdomains.

Both CSG and CAD geometries are supported. CAD models require MOAB version 5.2.0 or later.

The full method documentation is included in this PR under docs/source/methods/random_ray.rst (see the new "Domain Decomposition" section).

Fixes #3009

Verification

The implemented domain decomposition scheme preserves the original source region mesh and ray sampling routines regardless of the number of parallel MPI processes. Simulation results are therefore fully reproducible.

For verification, the random ray solver with and without domain decomposition was tested on the 2D C5G7 benchmark problem. This is a multi-group benchmark problem with 7 energy groups, which describes four simplified 2D reactor assemblies surrounded by a reflector region.

For the verification, a simulation mesh with 142,964 flat source regions was used. The simulation was run twice with the same random number seed: once with the original version of OpenMC with a single MPI rank and 128 OpenMP threads (no domain decomposition), and once with the implemented domain decomposition scheme on 128 MPI ranks with 1 thread per rank. Both runs produced an identical eigenvalue of 1.18626 ± 0.00038. Pin powers were also compared against the reference multi-group Monte Carlo benchmark solution, with average absolute pin power errors of 0.5534%, and a maximum pin power error of 2.0456% in both cases.

C5G7 geometry Rank subdomains (128 MPI ranks)
image image

Performance

The performance of the domain decomposition scheme was tested with a strong scaling study on the UKAEA “Simple Tokamak” benchmark, a large fixed source fusion neutronics simulation problem.

This problem is a nearly “worst case” challenge problem for domain decomposition as it features extreme spatial mesh resolution differences, with fine detail and small cells in areas like the divertor, and coarse mesh resolution in void areas of the problem, giving 1,626,536 source regions in total. Furthermore, the high complexity of the wall of the fusion device results in some cells being bounded by hundreds of CSG surfaces (resulting in extremely high ray tracing costs), while other areas (like the shield wall) are constrained with just a few planar surfaces. All results were obtained from simulations on the Improv cluster at Argonne National Laboratory. It was found that OpenMC performed best on the Improv architecture when 8 MPI ranks (each with 16 OpenMP threads) were used on each node.

Simulations were scaled from one node out to 40 nodes (5120 cores) while the mesh size was kept fixed. The simulations were run with 4 million rays per batch.

Runtime Times by code component
image timers_tokamak

The domain decomposition feature enables a continuous reduction in runtime all the way out to 40 nodes. As the problem is scaled to higher node counts, an increasing deviation from ideal scaling behavior can be observed due to the progressive loss of parallel efficiency. At the maximum of 40 nodes, only 38 % parallel efficiency was achieved compared to single node operation. At this scale, the cost of the domain decomposition operations begin to dominate the runtime of the code.

image

Summary of changes

New classes / files

  • DecompositionMap (decomposition_map): Contains the subdomain_map_, which maps source regions to MPI ranks. Provides methods for initializing the Voronoi volumes, estimating and balancing load, exchanging source regions and determining the owner rank of newly discovered source regions.
  • RayBank (ray_bank): Contains the list of rays to be sent to their new owner ranks and the methods for exchanging ray data between MPI ranks.

Random ray transport (random_ray)

  • Added two structs: RayBufferContainer, which holds the scalar members and vector fields of a ray while it is stored in the buffer, and RayExchangeData, which collects the scalar members for communication between MPI ranks.
  • Rays are now stopped once they leave their current MPI rank's subdomain, and their state is buffered in a RayBufferContainer. restart_ray() reinitializes a ray that has been received by its new owner rank. Additional methods check whether a ray has left its subdomain and pack it into the buffer.
  • The number of ray-trace operations is now counted for use in load balancing.

Random ray simulation (random_ray_simulation)

  • Added a separate transport_sweep_decomp() that buffers rays as they leave a subdomain
  • Additional domain-decomposition timers and output data are printed in the simulation summary.

Source regions (source_region)

  • Added ScalarSourceRegionFields, which holds all scalar fields of a source region.
  • Added SourceRegion::merge(), which merges two source regions that were discovered and contested by separate ranks in the same transport sweep.
  • Flat source regions now also store centroids, which are used for MPI rank assignment during load optimization.

Flat source domain (flat_source_domain)

  • Added is_geometry_3D() to probe the geometry type
  • Added output_to_vtk_decomp() to compose the VTK output data from the separate MPI processes.
  • External source strength and tally volumes are reduced across MPI ranks.

Supporting changes

  • parallel_map: added size() and erase(). operator[] now raises an error when the requested key is not present to help with debugging.
  • cell: Cell can now return its number of surfaces.
  • constants: added MAX_N_HANDLES (max DAGMC entity handles sent when exchanging rays, set to 5) and ITER_LOAD_BALANCE (max batches over which load balancing is performed, set to 5).
  • source: satisfies_spatial_constraints moved from protected to public so it can be used to confine Voronoi regions to the real geometry at the start of a simulation.
  • New timers quantify the cost of the domain-decomposition-specific operations.

Testing

No new regression tests were added, as the existing test suite already covers this feature. Domain decomposition activates automatically whenever the random ray solver runs on more than one MPI rank, so every existing random_ray_* regression test doubles as a domain decomposition test when the test suite is run with the --mpi flag. Since the reference results for these tests are generated from serial runs, the MPI runs directly verify that the decomposed solver reproduces the single-rank solution for the eigenvalue, fixed source, linear source, adjoint and mesh-tally variants.

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)

@Suark94
Suark94 requested a review from jtramm as a code owner July 23, 2026 16:24
Resolve conflicts between the domain decomposition work and the linear
source gradient limiter (openmc-dev#4121) and naive flux consistency fix (openmc-dev#4111)
that landed on develop in the meantime.

Resolution notes:

* SourceRegion: develop's new scalar fields (n_negative_batches_,
  converged_negative_) move into ScalarSourceRegionFields alongside the
  rest, so they are carried by the existing raw-byte MPI transfer in
  DecompositionMap::send_sr_data(). The gradient limiter's extent_ stays
  on SourceRegion, since BoundingBox is not part of the scalar block.

* SourceRegionContainer gained three configuration flags on develop
  (is_adaptive, is_strict_adaptive, track_extents). Added empty_like() so
  DecompositionMap::redistribute_source_regions() can rebuild a container
  with the same configuration instead of hardcoding the old two-argument
  constructor.

* simulate(): kept the decomposed structure from this branch and folded in
  develop's changes -- apply_transport_stabilization() is now part of
  add_source_to_scalar_flux(), and demotion_step() runs before flux_swap().

* Applied develop's OpenMP portability fix (accumulate into a local rather
  than naming a class member in a reduction clause) to both transport_sweep()
  and transport_sweep_decomp().

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
When two ranks independently discover the same source region during one
transport sweep, each of them has already applied the full external source
to its own copy in get_subdivided_source_region_handle(), which calls
apply_external_source_to_source_region() and then divides by sigma_t.
SourceRegion::merge() summed external_source_ along with the genuinely
accumulated fields, so every contested region ended up with twice the
external source it should have.

Since the external source is a fixed property of the region rather than a
tallied quantity, the receiver's copy is already correct and the sender's
contribution is dropped. Note that compute_fixed_source_normalization_factor()
renormalizes against the user-specified total strength, so a uniform doubling
would cancel; only boundary regions are contested, so the visible effect was a
spatial bias in the external source distribution.

Also union the sampled bounding boxes used by the source gradient limiter.
Both ranks sampled part of the same region, so the union is the correct
combination. The accompanying extent_ message in send_sr_data/receive_sr_data
covers the contested-region exchange, where both sides are real SourceRegion
objects; the load balancing path builds its SourceRegion from a
SourceRegionHandle and is fixed separately in "Preserve all per-region state
when load balancing rebuilds a container".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
num_base_source_region_RT_ and num_mesh_bin_RT_ are shared vectors indexed
by base source region, written from inside the OpenMP transport loop in
event_advance_ray() and attenuate_flux(). Multiple threads routinely
transport rays through the same base source region, so the unsynchronized
increments were a genuine data race.

The corrupted values feed the load estimate rather than the flux, so the
symptom was non-deterministic load balancing rather than wrong results, but
it is still undefined behavior and would be reported by a thread sanitizer.
Use omp atomic, matching how calculate_voronoi() already accumulates into
its shared per-rank arrays.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
restart_ray() reconstructed the ray position by computing a single
displacement at the top coordinate level and adding it to every level:

    Position delta_r = data.position - r();
    for (int i = 0; i < n_coord(); i++)
      this->coord(i).r() += delta_r;

That is only correct when every level shares the top-level direction. A
universe or lattice fill with a rotation gives each level its own direction
(coord(j).u()), so translating a lower level by the top-level displacement
places it at the wrong point and the receiving rank then computes wrong
distances and cells for the rest of the ray's flight.

OpenMC advances a particle with per-level directions -- via
GeometryState::move_distance(), and via the explicit loop at the end of
event_advance_ray() itself. Buffer the remaining advance distance alongside
the geometry state and use move_distance() on restart.
exchange_data_.position is kept only for the diagnostic warning in
RayBank::buffer_ray_data_to_send().

Both geometries used to verify this branch (C5G7 and the UKAEA Simple
Tokamak) are built from pure translations, which is why the original code
reproduced the serial result there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
redistribute_source_regions() had every rank run its full send loop before
posting a single receive, and send_sr_data() blocked in MPI_Waitall before
returning. That only completes because the messages are small enough for the
MPI implementation to buffer them eagerly. Once a send switches to
rendezvous it blocks until the peer posts a matching receive, which no rank
ever reaches -- a pair of ranks that each owe the other a region deadlocks.
A single message crosses a typical 8-64 KB eager threshold only at a few
hundred to a few thousand groups; the likelier trigger is many small messages
exhausting the implementation's eager buffers or credits, which needs no
unusual group structure at all.

Give send_sr_data() an optional request vector so the caller can defer the
wait, stage the outgoing SourceRegion objects so the buffers the Isend calls
reference stay alive, and drain the sends with a single Waitall after the
receives have been posted. exchange_sr_info() interleaves its sends and
receives per contested region already and is unaffected; it keeps the
blocking behavior via the default argument.

This also overlaps the exchange instead of serializing it, which matters
because load balancing already dominates the reported run time at high node
counts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
find_closest_rank() built a fresh vector<int> of candidate ranks on every
call, either by resizing and running std::iota over all ranks or by copying
the neighbor set. It is called from find_owner() inside the OpenMP transport
loop for every segment entering an undiscovered region, and from update_load()
once per source region per load balancing iteration (up to 200 iterations).

Per AGENTS.md, heap allocation in the transport loop contends on the global
allocator and precludes future GPU execution. Iterate the candidates directly
through a small lambda instead; the function now allocates nothing and the
body is shorter.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
* any_discovered_source_regions() started time_decomposition_handling and
  then hit an unreachable second start() after the return statement instead
  of stopping it, so the timer kept running until an unrelated stop() in
  compute_k_eff() or instability_check() and the reported breakdown was
  `start_`. So `time_decomposition_handling` under-counts, and any
  `elapsed()` query made while the timer was left running over-reports.

* The load model divided n_hits (int) by simulation::current_batch (int), so
  from batch 2 onward every region with fewer hits than the batch number
  contributed exactly zero hit cost -- precisely the sparse regions where the
  estimate matters. Divide in floating point.

* output_to_vtk_decomp() reduced num_neg and num_samples inside the energy
  group loop with MPI_IN_PLACE on master, folding each group's running totals
  back in on every subsequent group, and never reduced min_flux/max_flux at
  all. Accumulate locally and reduce once after the loop.

* The copy of the fixed source plotting block in output_to_vtk_decomp() read
  source_regions_.material(fsr) and temperature_idx(fsr) before checking
  fsr >= 0, reading out of bounds for voxels outside the geometry. (The same
  bug exists in output_to_vtk(); left alone here since that path is not part
  of this branch.)

* Replaced printf with write_message so the decomposition output respects
  settings::verbosity. These were the only printf calls in all of src/.

* Null-check the IndependentSource/SpatialBox casts in
  DecompositionMap::initialize() and FlatSourceDomain::is_geometry_3D()
  instead of dereferencing the result of dynamic_cast unconditionally.

* Removed dead code: the unused voxel_indices_key vector in output_to_vtk()
  (Nx*Ny*Nz * 16 bytes), the unused mesh_bins_per_base_sr_local in
  calculate_rank_load(), the duplicated reserve() block in
  RayBank::buffer_ray_data_to_send(), the commented-out <thread>/<chrono>
  includes, and a shadowed max_load in balance_load().

* Corrected the ITER_LOAD_BALANCE comment: it bounds the number of batches
  over which rebalancing runs, not the iterations within a pass.

* Guarded the MPI_Request in the send_sr_data() declaration, and reformatted
  the merge resolution with clang-format 18.

Verified to build with -DOPENMC_USE_MPI=ON and OFF.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
FlatSourceDomain::is_geometry_3D() samples up to 100 columns x 100 points
and calls exhaustive_find_cell() for each, but RandomRay::geom_dim_ is read
only by DecompositionMap when laying out the Voronoi grid. Every serial
random ray run was paying for the probe and throwing the answer away.

Fold the check into the existing MPI-guarded setup block so it runs only
when mpi::n_procs > 1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
docs/source/methods/random_ray.rst referenced ../_images/c5g7_geometry.png
and ../_images/c5g7_voronoi.png, but neither file was added, so the Sphinx
build emitted warnings and both figures rendered broken.

Dropping the directives so the docs build clean. The figures are worth
having: the C5G7 geometry and its Voronoi decomposition are exactly what the
surrounding text describes, so please re-add both directives together with
the PNGs under docs/source/_images/.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
set_fw_adjoint_sources() screens out regions whose forward flux is
negligible by comparing against ZERO_FLUX_CUTOFF * max_flux, where max_flux
was reduced only over OpenMP threads. That is the whole-problem maximum when
one rank holds the whole problem, but under domain decomposition it becomes
a per-subdomain maximum.

Every local maximum is at most the global one, so every rank but the one
holding the peak gets a *lower* threshold and screens out fewer regions --
reintroducing precisely the enormous 1/phi adjoint sources the cutoff exists
to suppress. On a deep-penetration shielding problem split so that one rank
holds the source and another the far field, the far-field rank's local
maximum can be many decades below the global one, so its noisy near-zero
fluxes turn into huge adjoint sources and the generated weight windows drive
particles towards them. The bias depends on how the domain was partitioned,
so weight windows stop being reproducible across rank counts.

This function is untouched by the domain decomposition work, which is why
the problem is invisible in the diff: it was correct under the old
"rank 0 does everything" model and became wrong when the domain was split.
The other global quantities in this file (fission rates, entropy, the fixed
source normalization, external source region counts, n_hits and the tally
volumes) are all reduced already. max_flux is the only one I found missing;
this was not an exhaustive audit of every global in the file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
redistribute_source_regions() rebuilds a rank's SourceRegionContainer by
round-tripping every region -- retained as well as migrating -- through
SourceRegion(const SourceRegionHandle&) and SourceRegionContainer::push_back().
That constructor set most of the region's scalars and arrays and silently
dropped everything
develop has added since, and push_back() hardcoded two more fields to zero:

  centroid_offset_        reset to {0,0,0}   (openmc-dev#4111)
  scalar_flux_t_          reset to 0.0       (openmc-dev#4110)
  converged_negative_     reset to 0         (openmc-dev#4110)
  n_negative_batches_     reset to 0         (openmc-dev#4110)
  extent_                 reset to inverted  (openmc-dev#4121)

This fired on every rank in every balanced batch, whether or not any region
actually moved: balance_load() falls through to redistribute_source_regions()
even when the imbalance is already inside tolerance.

The worst of the five is centroid_offset_. Within a single batch the order is

  normalize_scalar_flux_and_volumes()   writes centroid_offset
  balance_load()                        zeroes it
  add_source_to_scalar_flux()           reads it via flux_additive_term()

so for a linear-source decomposed run, batches 2..ITER_LOAD_BALANCE dropped
the q_gradient . delta_centroid term that openmc-dev#4111 added to stop gradient-scale
noise igniting self-sustaining negativity. scalar_flux_t_ and
converged_negative_ matter because AUTO resolves to an adaptive estimator by
default, so every default decomposed run discarded the first five batches of
the demotion accumulator and released a flag that source_region.h documents
as never being released.

Fixes: carry centroid_offset_ in ScalarSourceRegionFields (so the raw-byte
transfer picks it up for free), add scalar_flux_t_ to SourceRegion with its
own MPI message, wire both onto SourceRegionHandle, null-check the
feature-gated handle pointers in the constructor, and have push_back()
propagate rather than zero them.

This also completes the extent_ transfer added in "Do not sum external source
when merging contested source regions": that commit added the message, but the
migrating region was built by this same constructor, so what went over the
wire was always BoundingBox::inverted().

Added a comment on ScalarSourceRegionFields stating the invariant, since the
current layout makes this failure mode invisible and it will recur every time
develop adds a per-region field.

Two things the new reads in the handle constructor require:

SourceRegionHandle's raw pointers now all default to nullptr. extent_ is
assigned only inside the `if (handle.is_linear_)` branch of
get_source_region_handle(), so on the flat-source path -- the default -- it
was left indeterminate, and the `if (handle.extent_)` test below would
dereference garbage on the first load balancing pass of every decomposed
run. n_negative_batches_ and converged_negative_ are null-tested the same way
and were safe only because both current paths happen to assign them, so all
of them are defaulted rather than just the one that bit. Measured cost of
defaulting all 36: +48 bytes of text in source_region.cpp and -34 bytes in
flat_source_domain.cpp, where the hot-path handle construction lives -- the
compiler drops the redundant stores. The SourceRegion& constructor also stops
relying on data() of an unallocated vector being null, which libstdc++ does
but the standard does not promise.

SourceRegion's scalar_flux_t_ allocation is gated on the estimator actually
being adaptive. That constructor runs on the transport path once per newly
discovered region, so allocating unconditionally would add a heap allocation
per discovery and negroups * 8 bytes per live transient region to every run,
serial and non-adaptive included. The gate derives from the same
resolved_volume_estimator_ the container uses, which is fixed before any
SourceRegion is constructed within a solve, so the two cannot disagree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
Three related hazards where the decomposition could diverge between ranks
and then corrupt memory or hang rather than reporting a problem.

1. calculate_voronoi() and find_closest_rank() guarded their "no closest
   rank found" fatal_error with `mpi::master &&`. Non-master ranks did not
   abort -- they carried on with closest_rank == C_NONE (-1) straight into
   position_sum_per_rank[-1], ray_send_buffer_[-1],
   num_messages_sending_[-1] and finally MPI_Isend(..., dest = -1, ...).
   The condition is rank-local, so the guard turned a diagnosable abort into
   out-of-bounds writes on every rank but one (ray_send_buffer_ is a map, so
   that one merely inserts a bogus key; the other three are real).
   fatal_error() calls MPI_Abort, so removing the guard aborts the job
   cleanly.

2. generate_rank_centers() contained no MPI at all: every rank ran Lloyd's
   algorithm independently and was assumed to land on bitwise identical
   centers. The centroid accumulation uses `omp atomic` on doubles, whose
   summation order is not reproducible, so the centers can differ in their
   last bits between ranks and between runs. That matters because
   find_closest_rank() is evaluated on different ranks for the same physical
   point and the answers must agree -- a disagreement makes two ranks each
   believe the other owns a region, and the ray bounces between them
   forever. The drift is tiny, but find_owner() runs once per ray segment
   per mesh bin, so at scale the expected number of disagreements per run is
   not small. Broadcast rank 0's centers instead of hoping.

3. RayBank::buffer_ray_data_to_send() detected a ray being sent to its own
   rank, warned, and then buffered it anyway. Such a ray comes back
   unchanged next round and loops forever, emitting one warning per round
   from inside the transport loop. The analogous source region case is
   already a fatal_error in redistribute_source_regions(); make this one
   match.

Also promote the ray receive buffer arithmetic to int64_t.
total_receiving_rays_ * negroups_ was computed in int and overflowed before
the resize, under-sizing the buffers so the following MPI_Irecv wrote past
their end -- a heap overflow rather than an MPI error. The offsets that index
those buffers (recv_offset, and the loop index in update_my_ray_list) are
promoted too, since sizing them correctly while still computing the write
positions in int would leave the same overflow in place.

The MPI count arguments themselves must stay int, and there the byte products
wrap silently: sender and receiver narrow identically, so a wrapped count
matches on both sides and MPI transfers the truncated payload without error,
leaving the restarted rays to read stale buffer contents. RayExchangeData is
128 bytes and LocalCoord 80, so with four coordinate levels the coord message
wraps at roughly 6.7M rays between one pair of ranks in one round. Check the
counts up front and fail loudly instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD
…rial runs

Four changes, all about the transport loop under decomposition.

Ray handoffs are now counted and bounded. The ownership early-return in
attenuate_flux_inner() happens before n_event() is incremented, and
restart_ray() restores n_event() verbatim, so a ray that is only ever
forwarded never advances its event counter and max_particle_events cannot
stop it. Two ranks that disagree about who owns a newly discovered region
therefore hand the same ray back and forth forever, and since the sweep
loop is `while (RB.is_any_ray_alive())` -- an Allreduce -- every rank in the
job hangs on that one ray. The authors clearly hit a version of this: the
sub-TINY_BIT mesh segment skip carries the comment "this can cause rays to
bounce back and forth indefinitely". That patches one trigger, not the
class. Carry an n_transfers counter in the exchange payload and terminate a
ray that exceeds MAX_RAY_TRANSFERS with a warning.

restart_ray() now resets is_local_ explicitly. It is correct today only
because RayBank::update_my_ray_list() rebuilds my_ray_list_ from
value-initialized objects each round; if that list is ever reused in place, a
restarted ray would inherit is_local_ = false and be re-buffered immediately.
(is_alive_ is left alone: it is never read anywhere in the tree, so resetting
it would not be a safeguard.)

The sub-TINY_BIT mesh segment skip is gated on mpi::n_procs > 1. It was
applied unconditionally, so every existing serial run with an overlaid
source region mesh silently dropped segments shorter than TINY_BIT, losing
both their attenuation and their volume contribution. The numerical effect
is negligible, but it is a behavior change for a large existing user base
introduced as a side effect of an MPI workaround -- and it would perturb the
random_ray_*_mesh regression references.

The ray-trace counters are only incremented during the load balancing
window. They are read solely by calculate_rank_load(), which runs while
current_batch <= ITER_LOAD_BALANCE; past that the atomics added to fix the
data race were pure false-sharing in the innermost transport loop on a
counter nobody reads.

Finally, attenuate_flux_linear_source() and _void() go back to their
is_active parameter instead of the is_active_ member. The two agree at all
four call sites, but only because is_active_ = true also moved after the
dead-zone segment in event_advance_ray(); either change alone is a silent
bug, and the parameter is what the flat-source variants use.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NKmpLGykRSiPAXyCPqWQD

@GuySten GuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
The only thing left to do is to rerun the benchmark after the fixes I introduced.

@GuySten

GuySten commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

I've checked that the results are identical when using mpi ranks 16,32,64 on a C5G7-like model.

I am planning to merge in a few days if no one objects.

@GuySten GuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merging Soon PR will be merged in < 24 hrs if no further comments are made. Random Ray

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Random Ray Domain Decomposition

3 participants