Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition on updateExternal() method call in PatchProcessMetabolism #92

Open
allison-li-1016 opened this issue Nov 13, 2024 · 3 comments
Assignees
Labels
category: agent Related to the agent subpackages migration: v2 to v3 Aligning v2 and v3 package: patch Related to the patch implementation priority: high Urgent problem that blocks the system until the issue is resolved type: bug Something isn't working as expected

Comments

@allison-li-1016
Copy link
Contributor

Description

the updateExternal() function is a method in PatchProcessMetabolism that basically allows metabolism to grab the amts of environmental glucose and oxygen from the lattices at each time step. In 2.4, updateExternal() is called at initialization of the object in the constructor, whereas in 3.0, this method is called as part of the step() function in PatchProcessMetabolism, before stepProcess() is called.
I think this change is causing a race condition for cells that are added after time point 0. In my case, T cells are added in the simulation after 10 minutes, and they are using their own version of metabolism called MetabolismCART. As part of the constructor of MetabolismCART, the internal amounts of glucose is set to the external amount.

 // Initial internal concentrations.
 intAmts = new double[2];
 intAmts[GLUCOSE] = extAmts[GLUCOSE];
 intAmts[PYRUVATE] = extAmts[GLUCOSE] * PYRU_PER_GLUC;

However, because updateExternal() is not called until the process is stepped, the extAmts array initializes as 0 and does not reflect the actual environmental glucose concentration. This will also set the internal amount of glucose to 0. There is no update for internal glucose between the initialization of the object, and the stepping of the process. As a result, because the glucose gradient is calculated based on the internal glucose amounts, this causes a huge glucose gradient and influx for the t cell at the beginning of the simulation.

Some questions and points for discussion:

  • What was the reasoning behind moving the method call from the constructor to the step() method between 2.4 and 3.0? Would it make sense to move it back to the constructor, or are there other ways to fix this race condition?
  • Moving the method call back to the constructor seems to be non-trivial, because the simulation object is no longer passed into the metabolism constructor, and updateExternal requires the simulation object. Any thoughts on the design?
@allison-li-1016 allison-li-1016 added category: agent Related to the agent subpackages migration: v2 to v3 Aligning v2 and v3 package: patch Related to the patch implementation priority: high Urgent problem that blocks the system until the issue is resolved type: bug Something isn't working as expected labels Nov 13, 2024
@allison-li-1016 allison-li-1016 self-assigned this Nov 13, 2024
@jessicasyu
Copy link
Member

Thanks for the detailed description, @allison-li-1016!

To clarify, in 2.4, updateExternal is called in both the constructor and each step, while in 3.1+, updateExternal is only called in step. This change was part of the larger change in how cell agent are initialized (from reflection to factories) and was motivated by the reasoning that a cell agent should be able to be constructed independent of the simulation instance.

This assumption holds for creating new cells and for the actions that add cells into the simulation -- a cell introduced into the environment should not "know" the local environment concentration. It gets a little weird with PatchActionConvert (we should open a separate issue for this action, it should be reusing the old cell processes instead of creating new ones).

I do not think we should be passing the simulation object to the metabolism constructor. Instead, I think we should switch all the metabolism modules to grab their initial internal amounts from parameters instead, something like:

Parameters parameters = cell.getParameters();
intAmts[GLUCOSE] =  parameters.getDouble("metabolism/INITIAL_GLUCOSE_CONCENTRATION");

This gets you a couple things:

  • You can define different initial concentrations for different cell types
  • You can utilize the new parameter distributions feature to introduce variation in initial concentrations
  • Module initialization is independent of layer initialization

Thoughts?

@allison-li-1016
Copy link
Contributor Author

@jessicasyu I agree on this proposal for cells that are initialized at time point 0 in the simulation.

However, for cells that are added into the simulation beyond time point 0, the initial glucose concentration would be dependent on the glucose concentration of the environment at that time point in the simulation. In that case, I don't think users would be able to pass the initial concentration as a parameter, and the module would need to grab that information from the lattice layers instead. I'm having a hard time thinking of how to decouple the simulation from the agent for this specific case.

@jessicasyu
Copy link
Member

I guess the question is: why is initial glucose concentration dependent on the glucose concentration of the environment at that time point in the simulation? For something like a "T cells added to a culture of cancerous cells" simulation, I imagine the T cells were cultured separately (under possibly different media conditions) and then added to the simulation. In this case, their internal glucose concentrations are not a function of the "dish" they are added to. Is that consistent with what you are trying to achieve, or am I misunderstanding? Happy to hop on a quick call if that is easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: agent Related to the agent subpackages migration: v2 to v3 Aligning v2 and v3 package: patch Related to the patch implementation priority: high Urgent problem that blocks the system until the issue is resolved type: bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

2 participants