Skip to content

Chemo mito grow steppables - #84

Open
NoWon1 wants to merge 33 commits into
mainfrom
chemo_mito_grow_steppables
Open

Chemo mito grow steppables#84
NoWon1 wants to merge 33 commits into
mainfrom
chemo_mito_grow_steppables

Conversation

@NoWon1

@NoWon1 NoWon1 commented Aug 31, 2026

Copy link
Copy Markdown
Owner

No description provided.

NoWon1 and others added 3 commits July 1, 2025 19:36
@NoWon1
added Growth, Mitosis and ChemotaxisSteppable
4f3d645 
to show the dynamic nature of simulations and their motility across a gradient
@google-labs-jules @NoWon1
Replace SWIG try-except spatial loops with boundary checks
270fceb 
Replaced 8 deeply nested 'try...except' blocks wrapping CC3D spatial
array lookups (like self.cell_field) with explicit boundary logic
checking against self.dim.x and self.dim.y to resolve SWIG performance
overhead during out-of-bounds lookups.

Co-authored-by: NoWon1 <126711057+NoWon1@users.noreply.github.com>
@NoWon1 @google-labs-jules
Pre-calculate nested spatial loops boundary checks in safe_cell_remov…
b545ef3 
…al and check_ecm_contact (#82)

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
google-labs-jules Bot and others added 2 commits September 1, 2026 15:52
@google-labs-jules @NoWon1
docs: verify codebase integrity, no performance optimizations identified
b02b3b9 
No suitable hot-path performance bottlenecks were identified to optimize.
Cold-path initialization routines were identified, but intentionally skipped
to avoid premature optimizations, per Bolt's guidelines. Codebase
integrity and tests verified.

Co-authored-by: NoWon1 <126711057+NoWon1@users.noreply.github.com>
@NoWon1
Optimize nested spatial loops in create_paper_cell (#85)
7faa1e6
Copilot AI lite review requested due to automatic review settings September 2, 2026 15:23
Copilot AI reviewed Sep 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

There are correctness gaps in the new/changed steppable logic (notably ECM tracking/removal consistency and a registered chemotaxis steppable that is currently a no-op) plus test isolation/coverage issues that should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the CancerInvasion CompuCell3D simulation to use “paper-based” (Kumar et al. 2016) parameters and expands the simulation logic by introducing additional steppables (growth, mitosis, chemotaxis), plus configuration and test scaffolding.

Changes:

  • Refactors CancerInvasionSteppables.py to use paper parameterization and adds GrowthSteppable, MitosisSteppable, and ChemotaxisSteppable.
  • Updates CC3D XML configuration (energies, diffusion/decay, and NeighborTracker) and registers new steppables in the runner.
  • Adds a pytest-based test module with CC3D dependency mocking, plus repo hygiene docs/config (.jules/bolt.md, .gitignore).
File summaries
File Description
Simulation/CancerInvasionSteppables.py Switches to paper-based parameters; adds/updates core simulation logic and new steppables.
Simulation/CancerInvasion.xml Adjusts CC3D configuration to match the updated model assumptions (energies, diffusion/decay, plugins).
Simulation/CancerInvasion.py Registers additional steppables so they run in the simulation.
tests/test_performance.py Adds mocked CC3D tests to exercise steppable methods without CC3D runtime.
.jules/bolt.md Documents CC3D-specific performance guidance (bounds checking and loop-bounding).
.gitignore Ignores common Python build/test artifacts.
Review details

Suppressed comments (2)

Simulation/CancerInvasionSteppables.py:228

  • safe_cell_removal clears fiber pixels from cell_field but does not keep fiber_locations in sync, so fiber_locations can retain coordinates that no longer belong to any ECMFiber.
                    if self.cell_field[x, y, 0] == cell:
                        self.cell_field[x, y, 0] = None
                        pixels_cleared += 1

Simulation/CancerInvasionSteppables.py:314

  • paper_mmp_system removes only the fiber COM coordinate from fiber_locations, but each fiber adds many coordinates; this leaves stale entries behind. If fiber_locations is meant to track all occupied ECM pixels, removal should be coordinated with pixel clearing (e.g., in safe_cell_removal) and this single-point removal should be dropped.
                self.safe_cell_removal(fiber)
                if (int(fiber.xCOM), int(fiber.yCOM)) in self.fiber_locations:
                    self.fiber_locations.remove((int(fiber.xCOM), int(fiber.yCOM)))
  • Files reviewed: 5/8 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Simulation/CancerInvasionSteppables.py Outdated
Comment thread Simulation/CancerInvasionSteppables.py Outdated
Comment thread Simulation/CancerInvasionSteppables.py Outdated
Comment thread tests/test_performance.py
Comment thread tests/test_performance.py Outdated
Comment thread tests/test_performance.py Outdated
@NoWon1 @google-labs-jules
⚡ Bolt: Optimize cell iterations using cell_list_by_type (#94)
0626dd9 
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 6, 2026 15:31
Copilot AI reviewed Sep 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@NoWon1 @google-labs-jules
⚡ Bolt: Use optimized cell_list_by_type instead of manual type checki…
a8c65ea 
…ng (#95)

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 05:46
Copilot AI reviewed Sep 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@NoWon1
NoWon1 marked this pull request as ready for review September 7, 2026 06:16
@NoWon1
Update performance test for paper_mmp_system
7ea2327 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 06:17
Copilot AI reviewed Sep 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@NoWon1
Enhance test performance with module mocks
ba27aa4 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 06:17
Copilot AI reviewed Sep 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@NoWon1
Specify exception type in error handling
e7eb244 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 06:17
Copilot AI reviewed Sep 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@NoWon1
Refine search radius for safe cell removal
8dc2fd0 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 06:18
Copilot AI reviewed Sep 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@google-labs-jules @NoWon1
No code changes were necessary
e005250 
Co-authored-by: NoWon1 <126711057+NoWon1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 25, 2026 10:38
Copilot AI reviewed Sep 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@google-labs-jules @NoWon1
⚡ Bolt: Replace max/min with inline conditionals in check_ecm_contact
493a22a 
Replaced `max()` and `min()` calls with conditional `if/else` expressions to evaluate boundary box calculations. Avoids expensive built-in function calls inside a critical hot path (which scales per cell * per cell perimeter check), significantly improving speed.

Co-authored-by: NoWon1 <126711057+NoWon1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 26, 2026 05:37
Copilot AI reviewed Sep 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@google-labs-jules @NoWon1
⚡ Bolt: Optimize fiber list creation and exact 2D distance calculation
e6184ed 
- Add a `seen` set to `create_paper_fiber` to achieve O(1) membership checks while preserving CC3D C++ array requirements.
- Replace `math.sqrt(dx**2 + dy**2)` with `math.hypot(dx, dy)` in `finish` for natively faster C-level calculations avoiding Python arithmetic overhead.

Co-authored-by: NoWon1 <126711057+NoWon1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 28, 2026 14:15
Copilot AI reviewed Sep 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI lite review requested due to automatic review settings September 30, 2026 11:43
Copilot AI reviewed Sep 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@NoWon1 @google-labs-jules
⚡ Bolt: Replace temporary lists with tuples for coordinate grouping (#…
9d2c414 
…139)

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 1, 2026 05:43
Copilot AI reviewed Oct 1, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Unresolved cleanup, parameter-handling, error-limit, and nonfunctional chemotaxis issues remain.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity · 2 Low severity

Open (5)

Comment thread Simulation/CancerInvasionSteppables.py
# especially for fibers or small cells, as it eliminates unnecessary grid lookups.
# Must materialize the list to avoid invalidating the C++ iterator
# when modifying the field.
pixels = [(pt_data.pixel.x, pt_data.pixel.y, pt_data.pixel.z) for pt_data in self.get_cell_pixel_list(cell)]
Comment thread Simulation/CancerInvasionSteppables.py
try:
if self.simulation_failed or self.error_count >= self.max_errors:
print("Simulation halted due to errors")
if self.simulation_failed:
Comment thread Simulation/CancerInvasionSteppables.py
except:
continue

self.mmp_secretor.secreteInsideCell(cell, 1.8)
Comment thread Simulation/CancerInvasionSteppables.py.orig
Comment on lines +1 to +5
from cc3d.core.PySteppables import *
import numpy as np
import random
import math

Comment thread tests/test_performance.py.orig
Comment on lines +1 to +5
import sys
import pytest
from unittest.mock import MagicMock, patch

# Mock CompuCell3D dependencies BEFORE importing steppables
@NoWon1

NoWon1 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review thread.

Copilot AI commented Oct 1, 2026

Copy link
Copy Markdown

@NoWon1 I'm unable to start working on this because of repository rules that prevent me from pushing to the branch:

  • Changes must be made through a pull request due to repository rules
  • Code scanning results must be provided due to repository rules

See the documentation for more details.

@NoWon1 @google-labs-jules
Optimize max/min calls with inline conditionals in CC3D steppables (#140
fc7aed6 
)

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 2, 2026 09:03
Copilot AI reviewed Oct 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Unresolved safety-limit, chemotaxis, parameter-use, test-coverage, and duplicate-file issues remain.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity · 2 Low severity

Open (5)

@NoWon1 @google-labs-jules
⚡ Bolt: O(1) sliding window for cell dynamics tracking (#141)
ba2c38c 
Replace `list.pop(0)` with `collections.deque(maxlen=N)` for tracking cell velocity history. This avoids the O(N) memory shifting overhead associated with popping from the front of a list, improving performance in the cell dynamics hot loop.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 3, 2026 11:15
Copilot AI reviewed Oct 3, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Critical configuration and unresolved simulation behavior issues block approval.

Review effort: Lite
Findings: 2 High severity · 2 Medium severity · 2 Low severity

Open (6)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Implement chemotaxis response for nonzero gradients

Simulation/​CancerInvasionSteppables.py:487

This steppable is registered and computes an MMP gradient, but the only branch for a nonzero gradient is pass, so no chemotaxis data, force, or cell property is ever changed. As a result cells cannot chemotax despite chemotaxis_strength; configure the Chemotaxis plugin/data and apply the gradient, or do not register this steppable until it has an effect.

Comment thread Simulation/CancerInvasion.xml
</Plugin>

<Plugin Name="CenterOfMass"/>
<Plugin Name="NeighborTracker"/>
@NoWon1 @google-labs-jules
⚡ Bolt: Optimize coordinate loop with tuples (#142)
4804a83 
Replaced a list allocation with a constant tuple in the hot loop
to avoid redundant allocations and garbage collection overhead.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 6, 2026 04:32
Copilot AI reviewed Oct 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Unresolved issues affect cleanup, parameter behavior, chemotaxis, error handling, and repository hygiene.

Review effort: Lite
Findings: 2 High severity · 2 Medium severity · 2 Low severity

Open (6)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Chemotaxis gradient is computed but never applied

Simulation/​CancerInvasionSteppables.py:489

This is a no-op: the gradient is computed and then discarded, while chemotaxis_strength is never used and no motility or chemotaxis state is updated. Because this steppable is registered every MCS, the new chemotaxis feature currently cannot move cells toward MMP gradients; implement the CC3D motility/chemotaxis update or defer registering this steppable.

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

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

PHCCO project 🤯
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@NoWon1