m3g / CellListMap.jl

Flexible implementation of cell lists to map the calculations of particle-pair dependent functions, such as forces, energies, neighbor lists, etc.
https://m3g.github.io/CellListMap.jl/
MIT License
87 stars 4 forks source link

Neighborlists contain repeated elements #84

Closed anhi closed 1 year ago

anhi commented 1 year ago

Hi,

I've noticed that sometimes, the results of neighborlist are non-unique:

using StaticArrays
using CellListMap

l = SVector{3, Float32}[[-1.45, 0.0, 0.0], [0.0, 0.0, 0.0], [0.495, 0.0, 1.437], [1.235, -0.911, 1.838], [-1.788, 0.918, 0.25], [1.642, 1.124, -0.864], [-1.801, -0.26, -0.911], [0.154, 1.136, -1.827], [-1.749, -0.665, 0.704], [0.258, 2.118, -0.351], [0.554, 1.175, -0.813], [0.341, -0.928, -0.46], [0.133, 0.98, 2.268], [0.605, 0.98, 3.639], [0.414, -0.408, 4.228], [-0.476, 1.73, 1.938], [0.325, 2.106, 5.472], [0.065, 3.039, 3.988], [-1.16, 1.868, 4.519], [-0.089, 2.07, 4.463], [1.676, 1.185, 3.63], [0.531, -1.358, 3.421], [0.462, -0.512, 5.473]]

nl1 = neighborlist(l, 10.0) # returns a unique 253-element list
nl2 = neighborlist(l, 7.0) # returns a non-unique 260-element list; unique(nl2) has 250 elements
lmiq commented 1 year ago

Thanks for reporting, I'm tracking down the problem.

MWE:

    l = SVector{2, Float64}[[0.0, 0.0], [-1, 0.0]]
    unitcell = [14.01, 14.02]
    neighborlist(l, 5.0; unitcell=unitcell)

This is a regression, the bug is not present in v0.8.0.

The issue is a combination of the exact particle coordinates being [0.0, 0.0] and the way the computing cells are filled up, a particle is getting to be copied into two computing cells because of falling at the exact boundary.

The fix will be released asap.

lmiq commented 1 year ago

I will release a new version 0.8.15 which will be identical to 0.8.10, which was fine.

The bug was introduced in 0.8.11 and the fix is not that easy, I need some time.

I recommend using version 0.8.10 for now if this affects you, until version 0.8.15 and greater are released.

lmiq commented 1 year ago

The issue will be solved in version 0.8.15 that will be released at any moment.

However, the fix includes performance regression, which I will have to deal with later, requiring some careful thought about the general improvements introduced in version 0.8.11, in particular.

Thus, I am not closing the issue now, but changing its label from "bug" to "enhancement".

lmiq commented 1 year ago

fixed in 0.8.16 to be released at any moment. I thank you again for reporting, and please let me know of any other issue you face using the package.