Skip to content

Fix neighbor cache not being cleared when search radius increases#464

Open
atharrva01 wants to merge 2 commits intoBioDynaMo:masterfrom
atharrva01:fix/neighbor-cache-corruption
Open

Fix neighbor cache not being cleared when search radius increases#464
atharrva01 wants to merge 2 commits intoBioDynaMo:masterfrom
atharrva01:fix/neighbor-cache-corruption

Conversation

@atharrva01
Copy link
Copy Markdown

Summary

I found a bug in the neighbor caching code inside ForEachNeighbor. When you have cache_neighbors turned on, the cache gets reused across multiple calls. That's fine until someone requests a bigger search radius than before—the cache becomes stale and needs to be rebuilt.

The problem is that the code wasn't clearing out the old cache entries before adding the new ones. So you'd end up with the old neighbors plus the new ones, which means duplicates.


Impact

If the cache has duplicate entries, the same neighbor can get processed multiple times in later queries. This can cause:

  • Double-counting forces
  • Wrong neighbor counts
  • Simulations that give different results depending on whether caching is on or off

The tricky part is nothing actually crashes—it just quietly produces wrong results.


Fix

Just clear the cache before refilling it:

cached_squared_search_radius_ = squared_radius;
neighbor_cache_.clear();  // wipe old data

Now the cache always has the right neighbors when it gets rebuilt.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>
@atharrva01 atharrva01 changed the title Fix neighbor cache corruption due to missing clear before re-population Fix neighbor cache not being cleared when search radius increases Mar 8, 2026
@atharrva01
Copy link
Copy Markdown
Author

hi @johnpapad24 this fixes a bug where the neighbor cache wasn't cleared before being rebuilt with a larger search radius, causing duplicate entries. This led to neighbors being processed multiple times and producing silent incorrect simulation results.

@sportokalidis
Copy link
Copy Markdown
Contributor

Hi @atharrva01,

Thanks for the contribution! I’ll review the changes and get back to you.

In the meantime: the CI issue that previously prevented macos-system-ci and ubuntu-system-ci from running has been fixed on main.

Could you please update your PR branch with the latest main (merge main into your branch, or rebase onto main) and push? That should trigger the missing system CI workflows on this PR.

@sportokalidis sportokalidis self-requested a review March 11, 2026 15:01
@atharrva01
Copy link
Copy Markdown
Author

Thanks for the update @sportokalidis!

I've merged the latest main into my branch and pushed the changes. The PR should now pick up the macos-system-ci and ubuntu-system-ci workflows.

Let me know if you notice anything else that needs adjustment.

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.

2 participants