Skip to content

Fix/heberlr/pr neighbor list for non movable cells - #430

Open
vincent-noel wants to merge 2 commits into
MathCancer:developmentfrom
vincent-noel:fix/heberlr/pr-neighbor-list-for-non-movable-cells
Open

Fix/heberlr/pr neighbor list for non movable cells#430
vincent-noel wants to merge 2 commits into
MathCancer:developmentfrom
vincent-noel:fix/heberlr/pr-neighbor-list-for-non-movable-cells

Conversation

@vincent-noel

Copy link
Copy Markdown
Collaborator

New pr for #410

@vincent-noel
vincent-noel changed the base branch from master to development August 14, 2026 16:16
Comment thread core/PhysiCell_standard_models.cpp Outdated
Comment on lines +650 to +655
if( pCell->is_movable == false )
{
pCell->velocity = {0.0, 0.0, 0.0};
return;
}

@drbergman drbergman Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This differs from #410. @heberlr made a change in core/PhysiCell_cell.cpp.

It looks like the choice here is to let the axpy( &velocity , temp_r , displacement ); update the velocity field. Then in this function, sets that velocity to 0 and short circuits the motility updates.

So I think this does the same thing for the most part:

  • add_potentials will now update the cell's velocity. Heber's version won't.
  • This version skips the motility vector update and sets velocity to 0; Heber's does the motility vector update. Looking more closely, I think it is technically possible for a cell to have is_motile==true but is_movable==false. So this particular PR better protects against that.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @vincent-noel! I believe you got the first commits where I used this approach, but later I changed it to do exactly what @drbergman mentioned (see commit 7ffa1a6: 7ffa1a6), avoiding the extra calculation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, so let's put that guard inside add_potentials back and then we'll be ready to merge once we've updated the test suite. Sound good @vincent-noel ?

@drbergman drbergman assigned drbergman and unassigned drbergman Aug 14, 2026
@vincent-noel

Copy link
Copy Markdown
Collaborator Author

Thanks for catching this @drbergman and @heberlr.
I added the missing commit. All good now, right ?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants