warn when a transformation hands an attack to a non-attacking type - #432
Draft
drbergman wants to merge 3 commits into
Draft
warn when a transformation hands an attack to a non-attacking type#432drbergman wants to merge 3 commits into
drbergman wants to merge 3 commits into
Conversation
Invert the decision made earlier on this branch. Transformation now leaves an in-progress attack running instead of ending it. `pAttackTarget` lives in `phenotype.cell_interactions`, so `phenotype = cd.phenotype` would drop it; hold it across the assignment and put it back. The other two halves of the attack link -- the target's `attacked_by` and the shared spring -- live in `state`, which transformation already leaves alone, so restoring `pAttackTarget` keeps all three consistent without touching the spring. This also makes transformation symmetric: attacks *against* a transforming cell already persisted, while attacks *by* it were ended.
It lives in phenotype.cell_interactions too, so the transformation reset the attacker's lifetime damage tally to zero -- now visibly wrong when the attack it was counting is still running afterwards.
Transformation now keeps an in-progress attack running. That attack can land on a cell definition that would never have started it: attack_rates default to zero, so a type the user never wrote attack parameters for inherits the attack anyway, and the attack_damage_rate (1.0) and attack_duration (30 min) defaults then decide how much damage it does. Whether the attack should continue is a modelling question, not one PhysiCell can answer, so nothing is changed -- but the case is easy to reach by accident, so warn when the new type has a zero attack rate against the target's type. Reported once per combination of (old type, new type, target type), since the same transformation recurs throughout a run.
Collaborator
|
I wonder if we should adopt the creation/use of a std::ofstream warnings.log file? The concern has always been that warnings from cout will simply fly by the user. There will likely be more warnings that could go into such a .log file. At least users could check it post-sim. Unfortunately this feature would affect all sample projects... or else we just show it's possible in the template project. |
Collaborator
Author
|
I think that's a great idea. If we can build it entirely apart from user files, then we don't need this to touch sample projects. We could demonstrate how to hook into it for main.cpp and custom_modules in the template project. |
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.
Draft. Stacks on #341 — the first two commits here are that PR's; the new work is the last commit. Review/merge #341 first and this will shrink to one commit.
Why
#341 makes an in-progress attack survive the attacker's transformation. That attack can land on a cell definition that would never have started it, which is the concern raised on #341:
attack_ratesdefault to 0, so a type the user never wrote attack parameters for inherits the attack anywayattack_damage_ratedefaults to 1.0 andattack_durationto 30 min, so those defaults — not the model — decide how much damage gets doneThis is easy to arrive at by accident: it needs no attack configuration on the new type at all.
What this does
Nothing to the simulation. Whether such an attack should continue is a modelling question, and every automatic answer I could come up with (drop the attack when the new type's attack rate is zero, add a
conserved-style flag, re-roll the duration) decides something on the user's behalf. So this only warns, and leaves the call to them.The warning fires when the new type's attack rate against the target's cell type is zero — i.e. the cell is now continuing an attack it could not itself have begun:
Reported once per
(old type, new type, target type)combination — the same transformation recurs constantly in a real model, and per-cell-per-step output would bury everything else.Notes for review
convert_to_cell_definitionruns inside the parallel phenotype loop. The dedupe list is guarded by a named critical:detach_cells_as_spring()and theattacked_byhelpers take the unnamed one, and OpenMP criticals are not reentrant.intthroughfind_cell_definition_index()rather than readingpTarget->type_name, since a target transforming on another thread in the same step would be rewriting that string.find_cell_definition_indexonly callsfind()on a map that is fixed after setup.attack_ratesis indexed viafind_cell_definition_index, not bytypedirectly —Cell::typeis the XML ID, which is not necessarily the definition index. (Cell_Interactions::attack_rate(name)was not reused here: it doesunordered_map::operator[], which inserts on a miss and so is not safe to call concurrently.)Verification
Built and ran
interaction-samplewithCD8+ T cell -> neutrophiltransformation enabled and a 30 min attack duration:neutrophil's attack rate againstbacteriais set > 0OMP_NUM_THREADS=8interactionsandrules_sampleare the only sample configs with both nonzero attack and transformation rates. Both run to completion on their stock configs with zero warnings, so this does not add noise to existing models.Open question
Should this be suppressible from the config, or is once-per-combination quiet enough?