Add Flystem Cells by Jannetty · Pull Request #220 · bagherilab/ARCADE

Jannetty · 2026-04-12T21:43:00Z

Estimated time to review: Large (sorry)

Summary of changes:
This code adds the fly neuroblast code into the codebase. The changes include:

Adding fly stem cell classes to the potts cell container convert method so convert can call the PottsCellFlyStem constructor when the stem cell classes are specified in the constructor.
Adding PottsCellFlyStem class (and test class)
Adding PottsCellFlyStemProliferation module which includes all plane of division logic, growth rate regulation logic, differentiation logic (and tests)
Made getUniqueIDs a public function for all implementations so that the PottsCellFlyStemProliferation module can use it to determine the identities of neighboring cells
Adding parameters needed for PottsCellFlyStem and PottsCellFlyStemProliferationModule

How to verify changes:

run tests
Here is a minimal neuroblast setup file which you can run on this branch:

<set prefix="2025-10-23-01">
    <series name="dd" start="1" end="1"
            ds=".3" margin="0" height="1" length="802" width="802"
            dt=".083" ticks="576" interval="4">
        <potts>
            <potts.term id="volume" />
            <potts.term id="adhesion" />
            <potts.term id="surface" />
        </potts>
        <agents>
            <populations>
                <population id="fly-stem-wt" class="fly-stem-wt" init="1" >
                    <population.link id="fly-gmc" weight="1"/>
                    <population.parameter id="proliferation/SIZE_TARGET" value="1.2" />
                    <population.parameter id="CRITICAL_VOLUME" value="52" units="um^3" conversion="DS^-3" />
                    <population.parameter id="proliferation/CELL_GROWTH_RATE" value="5" units="um^3/hour" conversion="DS^-3DT"/>
                    <population.parameter id="proliferation/BASAL_APOPTOSIS_RATE" value="0"/>
                    <population.parameter id="proliferation/DIV_ROTATION_DISTRIBUTION" value="NORMAL(MU=36,SIGMA=30)" />
                    <population.parameter id="proliferation/DIFFERENTIATION_RULESET" value="location" />
                    <population.parameter id="proliferation/DIFFERENTIATION_RULESET_EQUALITY_RANGE" value="10" />
                    <population.parameter id="proliferation/APICAL_AXIS_RULESET" value="uniform" />
                    <population.parameter id="proliferation/APICAL_AXIS_ROTATION_DISTRIBUTION" value="UNIFORM(MIN=0,MAX=360)" />
                    <population.parameter id= "proliferation/DYNAMIC_GROWTH_RATE_VOLUME" value="0" />
                    <population.parameter id= "proliferation/DYNAMIC_GROWTH_RATE_NB_CONTACT" value="0" />
                    <population.parameter id= "proliferation/VOLUME_BASED_CRITICAL_VOLUME" value="1" />
                </population>
                <population id="fly-gmc" class="fly-gmc" init="0">
                 <population.link id="fly-neuron" weight="1"/>
                 <population.parameter id="proliferation/BASAL_APOPTOSIS_RATE" value="0"/>
                 <population.parameter id="CRITICAL_VOLUME" value="11" units="um^3" conversion="DS^-3" />
                 <population.parameter id="proliferation/CELL_GROWTH_RATE" value="1.22" units="um^2/hour" conversion="DS^-2.DT"/>
                 <population.parameter id= "proliferation/DYNAMIC_GROWTH_RATE_VOLUME" value="0" />
                </population>
                <population id="fly-neuron" class="fly-neuron" init="0">
                <population.parameter id="proliferation/CELL_GROWTH_RATE" value="0"/>
                <population.parameter id="proliferation/BASAL_APOPTOSIS_RATE" value="0"/>
                <population.parameter id="CRITICAL_VOLUME" value="11" units="um^3" conversion="DS^-3" />
                </population>
            </populations>
        </agents>
    </series>
</set>

resolves #137
resolves #109
resolves #91
resolves #58
resolves #50

…inline import

…rgebranch

…degrees in line with experimental data

…division axis is rotating, defaults to apical axis but can be previous division instead

…t to move according to user specification

…oliferationModule and reference it instead of cell-specific critical volume for growth regulation math

…ns with deterministic division always yield a NB and a GMC

kristaphommatha

Look great so far! I looked over all the files except for the proliferation module and proliferation module tests, and had a couple of points of confusion for the files I did review!

I'll plan to come back and review the proliferation module and tests within the next couple of days! 🫡

kristaphommatha · 2026-08-24T22:11:08Z

 import arcade.core.util.MiniBox;
 import arcade.core.util.Parameters;
+import arcade.potts.util.PottsEnums.Phase;
+import arcade.potts.util.PottsEnums.Region;


I see right below these lines there is the exact same import, except static. Why were these non-static imports added/ what is the difference between the static/non-static imports?

kristaphommatha · 2026-08-24T22:15:59Z

+            case "fly-stem-wt":
+                return new PottsCellFlyStem(this, location, parameters, links);
+            case "fly-stem-mudmut":
+                return new PottsCellFlyStem(this, location, parameters, links);


I know that in the PottsCellFlyStem constructor, mudmut and wt cells have a different stemType code that differentiates them. This comment might be nitpicky, but without that context, this implementation could be confusing because it looks like both cases act the exact same way.

Is there a way to specify in the constructor call that you are creating a mudmut or wt cell?

kristaphommatha · 2026-08-25T17:44:11Z

+            PottsCellContainer container, Location location, Parameters parameters, GrabBag links) {
+        super(container, location, parameters, links);
+
+        if (module != null) {


What is the reasoning behind this check?

From the setStateModule method, I assume that the only module that isn't null is proliferation. I'm just curious about why is it important that the state is undefined instead of proliferative?

kristaphommatha · 2026-08-25T17:48:55Z

+        PottsCellContainer container =
+                new PottsCellContainer(
+                        randomIntBetween(1, 10),
+                        randomIntBetween(1, 10),


This might be really nitpicky and not even matter, but would there be any unexpected behavior if cellID and the parentID were the same?

kristaphommatha · 2026-08-25T17:53:06Z

+
+        int cellPop = randomIntBetween(1, 10);
+        MiniBox parameters = new MiniBox();
+        parameters.put("CLASS", "fly-stem-mudmut");


I feel like because of the way the PottsCellFlyStem constructor is implemented, this test and the one above are virtually the same.

I'm not sure if you need both tests here if the class difference between fly-stem-wt and fly-stem-mudmut is tested already in the constructor_validWTStemType_createsInstance() and constructor_validMUDMUTStemType_createsInstance() tests under PottsCellFlyStemTest. I could be wrong though and would love to talk about this more

kristaphommatha · 2026-08-25T18:02:41Z

+        PottsCellContainer container =
+                cell.make(cellID, State.PROLIFERATIVE, random, cellPop, cellCriticalVolume);
+
+        assertAll(


Would make on fly-stem-mudmut and fly-stem-wt cells create containers with the exact same parameters?

…olumes (instead of each inheriting its own

kristaphommatha

Left some comments on PottsModuleFlyStemProliferation! I still need to look over the tests, though!

kristaphommatha · 2026-09-16T17:35:11Z

+import arcade.potts.util.PottsEnums.Direction;
+import arcade.potts.util.PottsEnums.Phase;
+import arcade.potts.util.PottsEnums.State;
+import static arcade.potts.util.PottsEnums.Direction;


Same question as another comment I made, but is there a reason both static arcade.potts.util.PottsEnums.Direction and arcade.potts.util.PottsEnums.Direction are imported? What is the difference between importing static vs. non-static?

kristaphommatha · 2026-09-16T17:48:43Z

+     * Ruleset for determining which daughter cell is the GMC. Can be `smaller_gmc`, `basal_gmc`,
+     * `random`, or `apical_gmc`.
+     */
+    final String differentiationRuleset;


Would it be worth making differentiationRuleset,apicalAxisRuleset, apicalAxisRotationDistribution, and divRotationReference enum variables to make it more clear that they can only be specific values?

kristaphommatha · 2026-09-16T18:01:42Z

+    }
+
+    /**
+     * Chooses the division plane according to the type of stem cell this module is attached to.


I think this doc could be a little more specific. When I read "according to the type of stem cell this module is attached to", I didn't know what was meant by "type"-- I thought this meant there were other stem cell types but was confused when the parameter only specified a flyStemCell. Maybe you could write that it chooses the dvisision plane depending on if the fly stem cell is WT or MUDMUT?

kristaphommatha · 2026-09-17T17:47:42Z

+        int newID = sim.getID();
+        double daughterCriticalVol;
+        if (volumeBasedCriticalVolume) {
+            double floor = populationCriticalVolume * .20;


Why specifically set the floor at 20% of populationCriticalVolume? Will there ever be a future use case where someone would want to modify this percentage?

kristaphommatha · 2026-09-17T17:50:03Z

+
+    /**
+     * Determines whether the daughter cell should be a neuroblast or a GMC according to the
+     * orientation. This is deterministic.


Suggested change

* orientation. This is deterministic.

* orientation of the division plane. This is deterministic.

Alternatively:

Suggested change

* orientation. This is deterministic.

* orientation of the apical axis. This is deterministic.

kristaphommatha · 2026-09-17T18:09:44Z

+    /**
+     * Updates the neuroblast (NB) contact-dependent growth rate.
+     *
+     * <p>Growth is repressed as a function of NB contact using a Hill function. The number of


I think this would be more straightforward if the function was explicitly written out in the javadoc!

kristaphommatha · 2026-09-17T18:12:23Z

+        double offset = sampleDivisionPlaneOffset();
+
+        if (flyStemCell.getStemType() == StemType.WT
+                || (flyStemCell.getStemType() == StemType.MUDMUT


Aren't the only StemTypes for a flyStemCell WT and MUDMUT? Is this logic in place for if we add a different StemType in the future? Otherwise this will always be true

kristaphommatha · 2026-09-17T18:18:58Z

+                        centroid1, centroid2, ((PottsCellFlyStem) cell).getApicalAxis(), range));
+            }
+        }
+        throw new IllegalArgumentException(


Shouldn't this be inside the else if statement? As is, this can run if the cell is neither a WT or MUDMUT, but this is not reflected in the exception message.

kristaphommatha · 2026-09-17T18:25:56Z

+    }
+
+    /**
+     * Gets the smaller location with fewer voxels and returns it.


Maybe add that if the two locations are the same size, then expected behavior is to return the second location?

kristaphommatha · 2026-09-17T18:27:28Z

+    }
+
+    /**
+     * Gets the location that is lower along the apical axis.


I don't quite understand what is meant by "lower" here

allison-li-1016 · 2026-09-18T19:50:45Z

+    protected HashSet<PottsCellFlyStem> getNBNeighbors(Simulation sim) {
+        Potts potts = ((PottsSimulation) sim).getPotts();
+        ArrayList<Voxel> voxels = ((PottsLocation) cell.getLocation()).getVoxels();
+        HashSet<PottsCellFlyStem> stemNeighbors = new HashSet<PottsCellFlyStem>();


HashSets as a data structure should only keep unique objects as a default behavior so I don't think it makes sense to manually keep track of unique IDs (lines 327-332, and the uniqueIDs function) when adding to the set.

Currently nothing overrides the default equals() method in the core Cell class. Unless it is possible reassign cellIDs to the same cell object after initiating the object, cells with different cellIDs should already register as unique objects if you drop it in the HashSet.

If you are worried about cellIDs being reassigned at some point, I think it makes more sense to overwrite the equals() function to compare on cellIDs instead.

As is, I think you can just loop through the neighbors (not counting itself), check if they are the same population ID, and throw it into the hashset. The hashset should take care of the unique cells for you.

allison-li-1016 · 2026-09-18T20:11:43Z

+        int nbsInContact;
+        nbsInContact = getNBNeighbors(sim).size();
+
+        double neighborSignal = Math.max(0.0, (double) nbsInContact);


could nbsInContact be negative?

allison-li-1016 · 2026-09-18T20:20:40Z

+     * @return whether or not the daughter cell should be a stem cell
+     */
+    private boolean daughterStemRuleBasedDifferentiation(PottsLocation loc1, PottsLocation loc2) {
+        if (((PottsCellFlyStem) cell).getStemType() == StemType.WT) {


Would consider changing this to a switch statement if we anticipate adding more stem types into the model.

Overall the double nested if/else statement is a little difficult to parse through, would consider simplifying with switch statements if we want to be explicit, or just using 'else' if there are only 2 options and we don't anticipate adding more

allison-li-1016 · 2026-09-18T20:22:38Z

+     * @param range Maximum allowed distance along the apical axis.
+     * @return true if the centroids are within the given range along the apical axis.
+     */
+    static boolean centroidsWithinRangeAlongApicalAxis(


Super nitpicky so feel free to ignore:

At first glance at the method name I thought it was returning a list of centroids rather than a boolean and got a little confused. Maybe 'centroidsAreWithinRange..."?

allison-li-1016 · 2026-09-18T20:25:43Z

+                        Direction.XY_PLANE.vector,
+                        StemType.MUDMUT.splitDirectionRotation);
+        // If TRUE, the daughter should be stem. Otherwise, should be GMC
+        return Math.abs(normalVector.getX() - expectedMUDNormalVector.getX()) <= EPSILON


It might just be me but I got a little lost in the sauce with this return statement. It might be worth adding a comment explaining why we are comparing each coordinate to a margin of error?

allison-li-1016 · 2026-09-18T20:27:16Z

+        int newID = sim.getID();
+        double daughterCriticalVol;
+        if (volumeBasedCriticalVolume) {
+            double floor = populationCriticalVolume * .20;


Might be worth making this 20% a global static constant so its explicit what it is

allison-li-1016 · 2026-09-18T20:35:24Z

+     * @param potts the potts instance for this simulation
+     * @param random the random number generator
+     */
+    private void scheduleNewCell(


In the other proliferation modules (for both potts and patch), I've seen the pattern to be:

addCell( ){ // find available location // reset current cell // create new cell through make() // register + schedule new cell }

Is there a reason why you chose to make a separate function for schedule, and call schedule inside the make functions instead of putting everything in addCell?

allison-li-1016 · 2026-09-18T20:37:19Z

+            case "global":
+                return ((PottsCellFlyStem) cell).getApicalAxis();
+            case "normal":
+                if (!(apicalAxisRotationDistribution instanceof NormalDistribution)) {


I think you also check for this in like 702 - can we just check for this once outside of the switch statement?

allison-li-1016 · 2026-09-18T20:39:22Z

+    protected double calculateGMCDaughterCellCriticalVolume(PottsLocation gmcLoc) {
+        double criticalVol;
+        if (volumeBasedCriticalVolume) {
+            criticalVol = Math.max(gmcLoc.getVolume(), initialSize * .1);


Same comment as above with the .2 -> consider making this a global constant to be explicit abt what the .2/.1 represents

allison-li-1016 · 2026-09-18T20:42:33Z

+     * @param loc2 {@link PottsLocation} to compare to location1.
+     * @return the smaller location.
+     */
+    public static PottsLocation getSmallerLocation(PottsLocation loc1, PottsLocation loc2) {


We should move this to the utils or maybe the grid/location class?

I don't think it makes sense to call this static function on the fly cells class since the purpose seems to be on the grid level, and not specific to flystemcells.

allison-li-1016 · 2026-09-18T20:49:23Z

+     * @param apicalAxis Unit {@link Vector} defining the apical-basal direction.
+     * @return the basal location (lower along the apical axis).
+     */
+    public static PottsLocation getBasalLocation(


I am confused on why getBasalLocation and getApicalLocation are static methods.

Do all flystemcells share the same apical axis/orientation?

I think my confusion partly stems from the fact that the actions happening in this method are on the grid level rather than the flystemcell itself. However, the basal/apical axis is specific to the flystemcell class, so Im not sure whats the best way to resolve this.

allison-li-1016

I have a couple clarification questions -- will take a deeper look into the tests after clarification.

Jannetty added 20 commits April 11, 2026 18:48

update build and CI configuration

7647685

add updateGrowthRate framework and make getUniqueIDs public

57817c1

adding dostrings

6392d9e

another docstring

488ab02

more formatting

4376f32

final formatting

731082f

adding missing parameters to potts xml. Removing unnecessary parameter

6cb465d

removing redundant section of docstring

64ce606

puppycrawler asking for some odd spacing

2ce8a27

puppycrawl now wants me to undo the odd spacing it asked for

3e8bebc

removing part of PR more relevant to neuroblasts

9238f51

first portion of neuroblast files

6ec7a8d

Merge branch 'feature/build-ci-cleanup' into feature/flystem-mergebranch

b559f06

finish covering PottsModuleFlyStemProliferation with tests

19212bb

adding tests

6b7db55

importing PottsCell and Bag to GMC differentiation module instead of …

d80b49f

…inline import

fixing PottsCellFlyStem line length issue

6d8a7f8

addressing puppycrawler issues

30126b1

more formatting

510c8a9

format

4c8b0d1

Jannetty requested review from a team, allison-li-1016, cainja, daniellevahdat, jacob-evarts, jessicasyu, kristaphommatha and pohaoc2 and removed request for a team April 12, 2026 22:17

Jannetty self-assigned this Apr 12, 2026

Jannetty added 14 commits August 20, 2026 11:30

clarifying parameter names and docstrings

465b22d

Merge branch 'feature/proliferation-refactor' into feature/flystem-me…

cb21bce

…rgebranch

removed PDE-like growth from the NB proliferation module

349df76

renamed differentiation rulesets

bc407dc

added random and apical_gmc differentiation rulesets

529e8d4

changed angle threshold past which mudmuts divide symetrically to 75 …

08ba35d

…degrees in line with experimental data

added functionality for changing the reference vector from which the …

9138cea

…division axis is rotating, defaults to apical axis but can be previous division instead

adding WT_DIVISION_SPLIT_OFFSET_PERCENT_Y to allow WT division defaul…

e9172b0

…t to move according to user specification

adding ability to manually set GMC critical volume

a68698f

added populationCriticalVolume as tracked value in pottsCellFlyStemPr…

02caa77

…oliferationModule and reference it instead of cell-specific critical volume for growth regulation math

added WT early-return in daughterStemDeterministic so that wt divisio…

fc4f7f0

…ns with deterministic division always yield a NB and a GMC

changed default parameter values to align with new defaults

3ff6daa

addressing lint errors

019aad1

Merge branch 'main' into feature/flystem-mergebranch

54d0f2c

kristaphommatha reviewed Aug 25, 2026

View reviewed changes

fix bug in which both mud daughter NBs inherit one of their initial v…

5fbe760

…olumes (instead of each inheriting its own

kristaphommatha reviewed Sep 17, 2026

View reviewed changes

allison-li-1016 reviewed Sep 18, 2026

View reviewed changes

Comment thread src/arcade/potts/agent/module/PottsModuleFlyStemProliferation.java

allison-li-1016 reviewed Sep 18, 2026

View reviewed changes

allison-li-1016 requested changes Sep 18, 2026

View reviewed changes

	* orientation. This is deterministic.
	* orientation of the division plane. This is deterministic.

	* orientation. This is deterministic.
	* orientation of the apical axis. This is deterministic.

Conversation

Jannetty commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kristaphommatha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kristaphommatha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allison-li-1016 Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allison-li-1016 Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allison-li-1016 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Jannetty commented Apr 12, 2026 •

edited

Loading

allison-li-1016 Sep 18, 2026 •

edited

Loading

allison-li-1016 Sep 18, 2026 •

edited

Loading

allison-li-1016 left a comment •

edited

Loading