Skip to content

Conversation

@pohaoc2
Copy link
Contributor

@pohaoc2 pohaoc2 commented Nov 11, 2025

Estimated time to review: small

Description of changes:

  • Add Bag cycles to PatchCell object to track cell cycles
  • Log cycles in the output

Notes:
We need to discuss about what information should be output in the future to avoid saving unnecessary data.

@cainja
Copy link
Member

cainja commented Nov 11, 2025

So this one is kind of a tricky change. Cell cycles is not like the other cell arguments where those are parameters that change the behavior of the cell. This type of change is more about how do we track cell history. I don't really think they should be part of the constructor of the cell (especially as you saw how widely it broke other things to make 20 file changes). We talked about making this change in some sort of event logger (Cell X divided at time Y) similar to #191, and then parsing that information to get cycle length info.

@cainja
Copy link
Member

cainja commented Nov 11, 2025

This would also change the minimal amount of information to create a cell (i.e. a container)

criticals.add((int) (100 * src.criticalHeight) / 100.0);
json.add("criticals", criticals);

// TODO: add cycles
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

@Jannetty
Copy link
Member

@pohaoc2 @allison-li-1016 naive question: if the phase of the cell cycle is based solely on the passage of time, and you have the time point of each output file and the age of each cell in the output files, can you not make a parser to give you the cell cycle phase as part of your data analysis? Do the cells need to actually track their cell cycle phases if you can reconstruct them using the data we currently output?

@allison-li-1016
Copy link
Contributor

@pohaoc2 @allison-li-1016 naive question: if the phase of the cell cycle is based solely on the passage of time, and you have the time point of each output file and the age of each cell in the output files, can you not make a parser to give you the cell cycle phase as part of your data analysis? Do the cells need to actually track their cell cycle phases if you can reconstruct them using the data we currently output?

I think 'cell cycle' here as a label is a little confusing - here we're not tracking what phase of the cell cycle the cell is in but rather how much time the cell spends in a proliferative state before actually dividing. I don't think that is something you can calculate easily from the output files alone since then you'd have to track the change in cell state for each cell over time.

I've considered using age/divisions as a proxy but I don't think that was a valid estimate since our cells ages are initialized from a distribution and the estimate assumes that the cells are always in a proliferative state. I've compared the estimate vs. the actual cell cycle tracking and the difference is very significant.

@allison-li-1016
Copy link
Contributor

So this one is kind of a tricky change. Cell cycles is not like the other cell arguments where those are parameters that change the behavior of the cell. This type of change is more about how do we track cell history. I don't really think they should be part of the constructor of the cell (especially as you saw how widely it broke other things to make 20 file changes). We talked about making this change in some sort of event logger (Cell X divided at time Y) similar to #191, and then parsing that information to get cycle length info.

I've tried adding an 'EVENTS' output file that tracks information on when lysis events occur. I think it would be a good idea to add proliferation events as part of that events output file as well since then we don't have to change the structure of containers.

I can open a PR for that and we can see if that would be a better way to implement this.

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.

5 participants