Thread-reproducible simulation: counter-based RNG + race fixes - #427
Open
heberlr wants to merge 9 commits into
Open
Thread-reproducible simulation: counter-based RNG + race fixes#427heberlr wants to merge 9 commits into
heberlr wants to merge 9 commits into
Conversation
Add a deterministic, keyed random generator, Random(cell_id, time_step, purpose, sub_index), built on Philox4x32-10, plus a thread-local deterministic context that UniformRandom(), UniformInt(), NormalRandom(), and LogNormalRandom() consult when active. Existing call sites keep working unchanged, but draw from a reproducible sequence instead of a thread-order-dependent one whenever the context is installed.
Add PhysiCell_settings.use_counter_based_rng (default off) and the <options><rng_mode> config element (counter_based/philox/counter to enable; legacy/thread_local/mt19937 to disable explicitly). Install a per-cell deterministic RNG context, keyed by cell ID/step/phase, around each parallel per-cell update phase, and sort cells_ready_to_divide / cells_ready_to_die by cell ID before applying them serially, so new cell IDs and (*all_cells) ordering are thread-count independent. RNG determinism alone doesn't stop one cell's thread from mutating another cell's live state mid-update in the same parallel loop. Three such races are fixed here with #pragma omp ordered around the per-cell call, forcing the same relative commit order as a single-thread run: - standard_cell_cell_interactions() (attack/ingest/fuse) mutates a target cell with no ordering; two attackers can race on one target. - dynamic_spring_attachments() checks a neighbor's attachment capacity before attach_cells_as_spring() re-validates it, so two cells can both attach to an already-full neighbor. - Secretion::advance() -> simulate_secretion_and_uptake() accumulates into a shared per-voxel density vector with no lock; two cells sharing a voxel on different threads can lose an update.
Runs a built project twice at two thread counts with the same config and seed, then diffs the final snapshot output to catch thread-count-dependent divergence.
Explain the Philox-based deterministic RNG scheme, the existing division/death ordering precedent, the four cross-cell mutation races found and fixed, how to enable counter-based RNG, and how to test a new/modified project for thread reproducibility with beta/test_thread_repro.py.
…current on move Update the bound in update_voxel_in_container() whenever a cell crosses into a new voxel, not just on the rare type-conversion path. Without this the bound goes stale as cells migrate, and a voxel's neighbor search can wrongly skip a cell that has since moved in. Adapted from MathCancer#409, with #pragma omp critical added around the read-compare-write: in this codebase update_voxel_in_container() is also reachable from fuse_cell()'s parallel context, so the unprotected version from the upstream PR would still race here even though it may not have in the original context.
convert_to_cell_definition Unrelated to PR MathCancer#409: this read-compare-write already existed on the type-conversion path and was never protected. convert_to_cell_definition() is reachable from rule-triggered transformations inside the (unordered) phenotype loop, so two cells transforming in the same voxel in the same step could race on this shared bound. Protected with #pragma omp critical; sufficient since max doesn't depend on evaluation order.
Move the use_counter_based_rng branch out of run_secretion_phase(),
run_spring_attachment_phase(), and run_cell_cell_interactions_phase() and
into their call sites in update_all_cells(). Each is now
if(use_counter_based_rng){ call a small *_ordered() function } else {
original inline loop }, so the legacy path stays untouched inline code
instead of living inside a shared function, and use_counter_based_rng is
still checked once per phase per step, not once per cell.
Also drop the activate_random_context()/clear_random_context() calls from
the three else branches: inside a branch that only runs when
use_counter_based_rng is already false, that guarded call is provably
dead code, so removing it makes the legacy path true original behavior
rather than calls that happen to no-op.
… code. Also bring the content in line with the current architecture: add "Why this matters" (reproducibility, debugging, potential bitwise restart) and "Credit" (D. E. Shaw Research / Random123, philox.h provenance) sections, correct Fixes 1/2/4 to name the *_ordered() functions and describe gating at the call site instead of inside a shared function, and document why the else branches no longer call activate_random_context()/clear_random_context().
drbergman
reviewed
Aug 7, 2026
Comment on lines
+280
to
+299
| if (rng_mode_node) | ||
| { | ||
| std::string rng_mode = xml_get_my_string_value(rng_mode_node); | ||
| if( rng_mode == "counter_based" || rng_mode == "counter" || rng_mode == "philox" ) | ||
| { | ||
| PhysiCell_settings.use_counter_based_rng = true; | ||
| std::cout << "Using counter-based RNG mode" << std::endl; | ||
| } | ||
| else if( rng_mode == "legacy" || rng_mode == "thread_local" || rng_mode == "mt19937" ) | ||
| { | ||
| PhysiCell_settings.use_counter_based_rng = false; | ||
| std::cout << "Using legacy RNG mode" << std::endl; | ||
| } | ||
| else if( rng_mode != "" ) | ||
| { | ||
| std::cout << "ERROR: unsupported rng_mode '" << rng_mode << "'. Use 'legacy' or 'counter_based'." << std::endl; | ||
| exit(-1); | ||
| } | ||
| } | ||
|
|
Collaborator
There was a problem hiding this comment.
Could we document here what happens if the config file does not have an rng_mode node? I assume it is use_counter_based_rng = false
Collaborator
There was a problem hiding this comment.
and I did just see the answer in the header file 😆 . I still think that showing up here would be helpful as this becomes the one place to look for what it gets set to
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
2 participants
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.
Summary
Adds an opt-in, deterministic RNG mode (Philox4x32-10, keyed by
(cell_id, time_step, purpose, sub_index)) so a simulation produces bit-identical output regardless of OpenMP thread count — and fixes four core-engine race conditions that RNG determinism alone doesn't address, since they involve one cell's thread mutating another cell's live state, not random draws.Off by default (
<options><rng_mode>counter_based</rng_mode></options>to enable); existing projects are unaffected unless they opt in.Why
Thread-count-dependent (and in some cases run-to-run-dependent, at a fixed thread count) results meant there was never a reliable way to tell a genuine model bug from scheduling noise, or to reproduce a specific run for debugging. Every race fixed here was a real, silent correctness bug — not just a reproducibility gap — that had gone undetected because nothing before could reliably say "these two runs should match and don't." See
protocols/counter_based_rng.mdfor the full design writeup, including why each fix needsorderedvs. justcritical, and why fixes are gated behinduse_counter_based_rngrather than applied unconditionally.What's included
RNG core (
modules/philox.h,core/PhysiCell_utilities.{h,cpp}): a keyedRandom(cell_id, time_step, purpose, sub_index)entry point on top of D. E. Shaw Research's Philox4x32-10 (vendored from Random123; see Credit section in the doc), plus a thread-local deterministic context so existingUniformRandom()/NormalRandom()/etc. call sites — core and user code alike — pick it up automatically without being rewritten.Simulation-loop wiring (
modules/PhysiCell_settings.{h,cpp},core/PhysiCell_cell_container.{h,cpp}): the<rng_mode>config option, and a deterministic context installed around each per-cell update phase, keyed by phase-specific purpose. Division/death are applied serially in ID-sorted order (when enabled) so new cell IDs and cell ordering stay thread-count independent — the existing pattern the four fixes below extend.Four cross-cell mutation races found and fixed, discovered via repeated 1-vs-4-thread comparisons on real, multi-behavior projects, not by inspection:
standard_cell_cell_interactions()(attack/ingest/fuse) — mutates a target cell with no commit ordering; two attackers can race on the same target.dynamic_spring_attachments()— checks a neighbor's attachment capacity beforeattach_cells_as_spring()re-validates it; two cells can both attach to an already-full neighbor.max_cell_interactive_distance_in_voxel— unprotected read-compare-write on a shared per-voxel bound, inconvert_to_cell_definition()and (adopting upstream PR #409)update_voxel_in_container().Secretion::advance()→simulate_secretion_and_uptake()— accumulates directly into a shared voxel density vector with no lock; a genuine lost-update on substrate mass, independent of RNG mode, that predates this branch.Fixes 1, 2, and 4 use
#pragma omp ordered(the operation isn't commutative — order matters); fix 3 only needscritical(max is order-independent). Fixes 1/2/4 are gated behinduse_counter_based_rng:orderedisn't free (DOACROSS-style serialization of the marked region, see literature discussion in the doc), so the legacy path keeps its original, untouched, fully-parallel loop. That means these three races are still present withcounter_based_rngoff — a deliberate, documented trade-off, not an oversight.Testing tool (
beta/test_thread_repro.py): runs a built project at two thread counts with the same seed and diffs the final output — the tool that surfaced all four races above.Docs (
protocols/counter_based_rng.md): full design rationale, why each fix needs the synchronization primitive it uses, why gating was chosen over unconditional fixes, how to enable the feature, and how to test a new/modified project for thread reproducibility.