Skip to content

Conversation

elmbeech
Copy link
Contributor

Currently the phenotype_function, custom_function, and contact_function, which are specified in the custom.cpp, are only strictly defaults cell type specific. They are not inherited by cell types based on the defaults cell type.
This behavior is (at least to me) very unintuitive.
This pull request moves the phenotype_function, custom_function, and contact_function in the template model to the place where they become inheritable to any cell type based on the default cell type.
If there is a cell type that really needs its own such function, this still could be later specifically overwritten.

@drbergman
Copy link
Collaborator

drbergman commented Sep 30, 2025

My understanding had been that this structure was chosen to require the user to opt in to changing a given cell type's (phenotype, custom, contact)_function. I'm not entirely clear on why they are added to cell_defaults and if that matters for simulation execution. My guess there was that these are meant to demonstrate how to set these functions. I think showing how to grab a particular cell def and adding the functions in the comment block could be constructive here.

On the dev side of things, it would be helpful to have the changes in this PR reflect exactly the changes you are trying to make. If you start at the current MathCancer:development branch, create a new branch, make your changes, then push and open a PR, it'll make review and discussion clearer. It will mean closing this PR and opening a new one.

@elmbeech
Copy link
Contributor Author

elmbeech commented Sep 30, 2025

As far as I understood, the development branch should be equivalent to the latest master branch. This patch is based on a fresh checkout from the master branch.
Maybe @rheiland can add a comment on why we think these changes are more useful.

@drbergman
Copy link
Collaborator

Ah I see what happened. I was confused why some of the earlier commits were showing up in this history here. TL;DR the development branch is often left one commit behind the master branch when a new tag is released in this repo. Usually, this would work perfectly with what you did, but two events led to this not working as cleanly as could be. Sorry about that!

@elmbeech
Copy link
Contributor Author

elmbeech commented Oct 5, 2025

@drbergman no worries!
I will keep on working on fresh checked-out branches from the latest release.
I think this is the easiest for merging later for the next release.
Also, I usually name my patch branches according to the "source release", e.g. patch1142_* for physicell 1.14.2 patches.

All these pull requests are based on internal discussion that we have here (e.g. Randy and I ), when I develop on my current model. I don't need these changes for my model to work (unlike the episode sample project PR from last x-mas that went into the 1.14.2 release). These are more discussion points that can be discussed for the next official PhysiCell release. I make these PRs that the issues are not forgotten.

Best, Elmar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants