Feature extended asym div - #385
Open
drbergman wants to merge 11 commits into
Open
Conversation
more robust macro to check for windows
1.14.2 release
- Thanks @davidlzhou for development and testing!! - subsumes standard asymmetric division - old syntax still works: "asymmetric division to <cell_type>" - note: this syntax relies implicitly on the cell type the rule is being assigned to - hence, we need to an int to index these distinctly from the extended asym div rules - but they will still access the new extended asym div hash maps to get/set probabilities - uses pairs of ints (cell types) as keys into hash map for asym div probabilities - behaviors implemented - "extended asymmetric division to <cell_type_1> and <cell_type_2>" - sample project added: `make extended-asym-div-sample` - get and set functions for safe access to asymmetric_division_probabilities - asymmetric_division_probability functions to give robust getter access - set_asymmetric_division_probability functions to provide safe setter access - reorganize so asym div functionality is inside phenotype.cpp and standard_models.cpp - simplify write with new getters
This comment was marked as resolved.
This comment was marked as resolved.
select_daughter_types() never formed a cumulative distribution. It drew one UniformRandom() and compared it against each entry's own probability, so the first entry whose probability happened to exceed r won, and realised daughter type frequencies did not match the configured ones. It now walks a running sum. The outcome also depended on std::unordered_map iteration order, which is unspecified and varies with the standard library, insertion history, and rehashing -- so two runs with the same seed could pick different daughter types on different platforms. asymmetric_division_probabilities is now a std::map ordered by the (min,max) form of the type pair, which depends only on the cell type indices. pair_hash and equality_function are replaced by a single pair_compare. exit(-1) fired on correctly configured models. Probabilities meant to sum to exactly 1 routinely sum to a hair over it in double precision -- 0.11 + 0.33 + 0.56 is 1.000000000000000222 -- which tripped the "cannot be normalized" branch on models that were fine. A 1e-12 tolerance on both the entry test and the negative-probability test fixes that; the arithmetic is otherwise unchanged, and a genuinely over-specified model still reports and exits rather than being quietly rescaled. The diagnostic now prints the CELL's current probabilities instead of iterating every cell type pair from the definitions, since a rule or custom function may have changed them since setup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It was the branch's only change to this file, and development has since restructured .gitignore, so the two sides conflict on every merge. Dropping it makes the branch's .gitignore identical to the merge base, which lets development's version apply cleanly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
asymmetric_division_function tolerates a small excess when checking that the probabilities sum to at most 1, because probabilities meant to sum to exactly 1 land a hair over it in double precision. That tolerance was hard-coded at 1e-12, which is the right default but not right for every model: how much slack is needed depends on how many probabilities are summed and how they are produced. A model whose probabilities come out of rules rather than literals can exceed 1 by far more than 1e-12 -- excesses around 8e-3 have been observed in practice -- and such a model currently exits with no way to say that is acceptable. Adds <asymmetric_division_probability_tolerance> to the <options> block. Absent, the default 1e-12 applies and nothing is printed, so existing configs are unaffected. Present, the value is used and echoed at startup. A negative value is rejected at parse time rather than silently inverting both comparisons. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Read the asymmetric division block back as pairs when resuming resume_from_MultiCellDS arrived from development while this branch was changing asymmetric_division_probabilities from a vector indexed by cell type into a map keyed on the unordered daughter-type pair. The merge left the resume path reading it the old way, so the branch did not compile: std::map has no resize, and no operator[] taking an int. The mismatch was not only a compile error. The writer emits one value per pair, walking the upper triangle, which is n*(n+1)/2 values; the reader consumed n. Every field after it in the stream -- cell integrity, custom data -- would have been read from the wrong offset. Clears the map and reads the same n*(n+1)/2 values in the same order, storing them by pair. This matches the fix already carried on my-physicell, so the two trees converge here rather than diverging on a second spelling of the same repair. Verified against unit_tests/resume_sim: for its 3 cell types all 6 pairs are read, the cell_integrity fields immediately after them come back correctly aligned, and a run resumed from snapshot 48 reaches the same final composition as the uninterrupted run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Adjust asymmetric division probabilities whenever they sum past 1 asymmetric_division_function entered its renormalization block on total > 1.0 + tolerance, so raising the tolerance did not only relax when an overshoot is an error -- it also decided whether the probabilities were rewritten at all. That block sets the symmetric division probability, so gating it on a user-settable threshold let the tolerance change the model rather than its strictness. With a tolerance of 0.5 and probabilities of 0.3 for (stem, stem) and 1.0 for (stem, progenitor_1), the total of 1.3 fell inside the old gate, so nothing was adjusted and the draw ran against a distribution summing to 1.3. Because select_daughter_types draws from [0,1), (stem, stem) kept its 0.3 instead of being taken down to 0, and progenitor_1 was starved to 0.7. Running the asymmetric division sample that way ends with 464 cells, 137 of them stem; adjusting whenever the total passes 1 ends with 16 cells and a stem population that stays at 1, which is what those probabilities describe. Adjust on total > 1.0 and leave the tolerance its one job: deciding whether the resulting symmetric division probability is negative enough to be an error rather than round-off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add a weighted mode for asymmetric division The extended asymmetric division values are probabilities: they must sum to at most 1, and whatever they leave short of 1 is the chance of dividing symmetrically. That constraint is awkward for the models this feature is aimed at. The intended use is rule-driven values, and a Hypothesis Grammar rule cannot see what the other daughter-type pairs currently evaluate to, so keeping a sum at or below 1 is not something a rule set can enforce. The configurable tolerance buys slack for round-off, but cannot rescue a model whose values legitimately sum to 2 or 3. Adds <asymmetric_division_mode> to the <options> block, accepting "probabilities" (the default, and what an absent element means) or "weights". Under weights the values are relative weights, normalized by their own sum at each division, so their scale stops mattering and each rule can be written on its own. A string rather than a bool so a typo errors out instead of quietly reading as false and running the other model. The mode is model-wide rather than per cell definition: mixing them would make one rule line mean a probability for one source type and a weight for another, with nothing in the line to say which. Weights carry no implicit symmetric-division remainder -- symmetric division needs its own (type,type) weight -- except that an all-zero total, which has no normalized distribution, is defined as symmetric division. That needs no special case: leaving the draw scale at 1.0 makes select_daughter_types fall through to the parent and daughter types unchanged. The draw itself is one line. select_daughter_types takes a total_weight that scales the random draw; probability mode passes 1.0, which is exact in double precision, so no existing model's RNG stream moves. Confirmed by running the asymmetric division sample before and after: 487 .mat files byte-identical, and 244 SVG and 243 XML identical once wall-clock timestamps are stripped. Setting <asymmetric_division_probability_tolerance> together with weights is rejected at parse time. The tolerance bounds how far probabilities may sum past 1, and weights are normalized rather than bounded, so there is nothing for it to do; erroring beats letting one of the two silently win. An explicit "probabilities" mode alongside a tolerance stays legal, since that is unambiguous. The extended_asym_div sample gains a second config and rules file expressing the same model in weights, plus a README covering both modes and the trap that separates them: probabilities take the whole overshoot out of the symmetric entry, while weights scale every entry proportionally. The weights rules saturate at 1.0 where the probabilities rules use 0.5, chosen so the normalized weights reproduce the probabilities exactly. Verified single-threaded: across all 121 snapshots the two runs agree on every cell, and the only recorded difference is the asymmetric division values. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
Update: this PR now supports a weights model for selecting asym div probabilities. the underlying data structures are used in both cases. A settings field now holds a boolean for which path to use. if using weights, the random number is not from |
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.
extended asym div that allows for any combo of cell types for both daughter cells. Thank you @davidlzhou for contributing!
See the new sample project for how to use.
Config
Note: Order of
name1andname2attributes does not matter. An ordered map on unordered pairs of integers is used to get/set the relevant probabilities.Behaviors
Note: Order does not matter. An ordered map on unordered pairs of integers is used to get/set the relevant probabilities.
Tolerance on the probability sum
The check that the probabilities sum to at most 1 allows a small excess, because probabilities meant to sum to exactly 1 land a hair over it in double precision —
0.11 + 0.33 + 0.56is1.000000000000000222. That tolerance defaults to1e-12and is configurable, since how much slack a model needs depends on how many probabilities it sums and how they are produced. Probabilities driven by rules rather than XML literals have been seen to exceed 1 by around8e-3.Absent, the default applies and nothing is printed. Present, the value is used and echoed at startup. A negative value is rejected at parse time rather than silently inverting the comparison. This only widens the band in which an over-specified distribution is accepted and absorbed — one that exceeds 1 beyond the tolerance still reports and exits, and nothing is rescaled.