Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: Fixed find_current_cell #2410

Merged
merged 7 commits into from
Dec 12, 2018
Merged

core: Fixed find_current_cell #2410

merged 7 commits into from
Dec 12, 2018

Conversation

fweik
Copy link
Contributor

@fweik fweik commented Dec 7, 2018

The old implementation could potentially return a ghost cell or no cell as cell for a local particle,
but neighbor search on ghost cells is no possible. (They do not have neighbors set).
It's correct to always use pos_old to find the cell: this is the position the particle
was last sorted with, and hence will find the cell the particle is actually in. From my understanding
of the collision detection this meant that in some corner cases pairs were missed, but
I am not sure about that.

@fweik fweik added this to the Espresso 4.0.1 milestone Dec 7, 2018
@fweik fweik requested a review from RudolfWeeber December 7, 2018 19:36
@codecov
Copy link

codecov bot commented Dec 7, 2018

Codecov Report

Merging #2410 into python will increase coverage by <1%.
The diff coverage is 97%.

Impacted file tree graph

@@           Coverage Diff           @@
##           python   #2410    +/-   ##
=======================================
+ Coverage      72%     72%   +<1%     
=======================================
  Files         397     397            
  Lines       18659   18670    +11     
=======================================
+ Hits        13484   13498    +14     
+ Misses       5175    5172     -3
Impacted Files Coverage Δ
src/core/particle_data.hpp 97% <ø> (ø) ⬆️
src/core/cells.hpp 100% <ø> (ø) ⬆️
src/core/cells.cpp 89% <100%> (-1%) ⬇️
src/core/particle_data.cpp 94% <97%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3525b9d...1262edc. Read the comment docs.

@RudolfWeeber
Copy link
Contributor

Can be merged

@fweik fweik merged commit aee3439 into espressomd:python Dec 12, 2018
@fweik fweik deleted the find_cell branch December 12, 2018 16:04
@RudolfWeeber
Copy link
Contributor

@fweik, for the 401 branch, I just cherry-picked
a0ad77b
because otherwise some of the resort stuff leaks in.
Can you please check the 401test branch on my repo?

@fweik
Copy link
Contributor Author

fweik commented Dec 12, 2018 via email

RudolfWeeber added a commit to RudolfWeeber/espresso that referenced this pull request Jan 15, 2019
RudolfWeeber added a commit that referenced this pull request Jan 16, 2019
Core: Backport particle deletion code from pr #2410
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