Fix asymmetric division daughter selection, determinism, and spurious exit(-1)

[drbergman]

Three problems in Asymmetric_Division, plus a unit test suite.

1. select_daughter_types never formed a cumulative distribution

double r = UniformRandom();
for( auto it = probabilities.begin(); it != probabilities.end(); ++it )
{
    if( r <= it->second )   // r vs each INDIVIDUAL probability
    { return it->first; }
}

r was tested against each entry's own probability instead of a running sum, so the first entry whose probability happened to exceed r won. Realised daughter-type frequencies did not match the configured probabilities. PhysiCell_utilities.cpp choose_event() already had the correct idiom.

2. The outcome depended on std::unordered_map iteration order

Even with the arithmetic corrected, the code drew one UniformRandom() and then walked a hash-ordered container. Iteration order there is unspecified and varies with the standard library, with insertion history, and with rehashing — so two runs with the same seed could select different daughter types across platforms or compilers. For a simulator that seeds specifically for reproducibility that is a defect in its own right, and fixing the arithmetic alone does not address it.

asymmetric_division_probabilities is now a std::map with a pair_compare ordering keyed on the upper-triangular (min, max) form of the type pair. That is a function of the cell type indices alone, so iteration order — and hence the daughter types chosen for a given seed — no longer depends on insertion order, rehashing, or the standard library implementation.

This is why modules/PhysiCell_MultiCellDS.cpp appears in the diff: std::map is node-based and has no reserve(), so recreate_sim_state() drops that call. Four lines, and it is a build requirement of the container change rather than unrelated churn.

3. exit(-1) fired on correctly configured models

The sym_div_prob < 0.0 branch hard-killed the simulation from inside a cell division. It turns out this triggers on models whose probabilities are configured correctly, purely from floating-point round-off — a distribution summing to 1.0 in exact arithmetic can land fractionally below zero after the subtraction. At least one user has commented this branch out of their production build to stop it killing multi-hour HPC runs, which means they are now running with no diagnostic at all.

Replaced with a round-off tolerance plus a warn-once-per-cell-type rescale, extracted into a testable enforce_valid_distribution(). A long run survives and is still told.

Worth recording what was checked and found not to be broken: the > 1.0 normalization was suspected of drifting over repeated divisions, since it rewrites the cell's own map rather than normalizing a copy. It does not. It is exactly idempotent, verified across 200,000 configurations and 1,000,000 repeated applications, so it was left alone.

Tests

New unit_tests/asymmetric_division/. The frequency test samples select_daughter_types against a known distribution and asserts the empirical frequencies match — it fails on the old code and passes on the new. Determinism, round-off, and rescale behaviour are covered too.

Both the standard_asymmetric_division and extended_asymmetric_division XML paths route through this code and both still work.

The same change is prepared for feature-extended-asym-div (PR MathCancer#385 upstream) and is not included here.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes correctness and reproducibility issues in PhysiCell’s asymmetric division daughter-type selection, and replaces a hard exit(-1) failure mode with a tolerant, warn-and-continue repair pathway. It also adds a dedicated unit test suite to validate frequency matching, determinism, and round-off / rescale behavior.

Changes:

• Correct select_daughter_types() to sample from a cumulative distribution (instead of comparing against individual probabilities).
• Make selection deterministic for a given RNG seed by changing asymmetric_division_probabilities from std::unordered_map to an ordered std::map with a custom comparator.
• Replace fatal exit(-1) on floating-point/overspec cases with enforce_valid_distribution() (tolerant absorb/rescale behavior) plus warn-once-per-cell-type logging, and add unit tests covering these cases.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
core/PhysiCell_phenotype.h	Switch asymmetric-division probability storage to std::map with deterministic ordering; add enforce_valid_distribution() API and helper totals/normalization declarations.
core/PhysiCell_phenotype.cpp	Fix daughter selection to use cumulative probability; implement probabilities_total_excluding_self(), normalize_probabilities(), and enforce_valid_distribution().
core/PhysiCell_standard_models.cpp	Use the new repair function in asymmetric_division_function() and replace fatal exit with warn-once behavior.
modules/PhysiCell_MultiCellDS.cpp	Remove reserve() on asymmetric_division_probabilities since it is now a std::map.
unit_tests/asymmetric_division/test_asymmetric_division.cpp	Add a unit test executable validating sampling frequencies, determinism, and repair behavior.
unit_tests/asymmetric_division/Makefile	Add build/run targets for the new asymmetric division unit tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.