Fixed double-counting of periopdic image pairs

[Luthaf]

This was found by @liam-o-marsh in #240, and I cherry-picked it to merge the fix quicker.

Ping @rosecers who wants to use this!

---

📚 Documentation preview 📚: https://rascaline--260.org.readthedocs.build/en/260/

[rosecers]

Much appreciated on my end! Code looks great to this noob -- let me know if you want a formal review from me.

[Luthaf]

I think the NeighborsList calculator is now wrong with these changes, around here: https://github.com/Luthaf/rascaline/blob/2851d47ab3471057f3794ac2b48b0417add2f748/rascaline/src/calculators/neighbor_list.rs#L568-L571

The condition should account for the cell shift as well.

[github-actions]

Here is a pre-built version of the code in this pull request: wheels.zip, you can install it locally by unzipping wheels.zip and using pip to install the file matching your system

[PicoCentauri]

Good to have this fixed.

[PicoCentauri]

I have the feeling that this cell is this to small to check for more then one unit cell?

[Luthaf]

It's 3.6 in one direction, you should get pairs with a shift of 2 with a cutoff of 9

[PicoCentauri]

I understand what is going on here but maybe a picture would help. But I agree and accept, if this is too much work.

[liam-o-marsh]

does this work?

      ┼─────┼─────┼─────┼─────┼
     ╭╯    ╭╯    ╭╯    ╭╯    ╭╯
     │   × │   × │   × │   × │
     │     │     │  ╱  │     │
    ╭╯    ╭╯    ╭╯ ╱  <- first pair
    ┼─────┼─────┼─────┼─────┼
   ╭╯    ╭╯   ╱╭╯    ╭╯    ╭╯
   │   × │   × │   × │   × │
   │     │  ╱  │     │     │
  ╭╯    ╭╯ ╱  <- second pair, redundant
  ┼─────┼─────┼─────┼─────┼
 ╭╯   ╱╭╯    ╭╯    ╭╯    ╭╯
 │   × │   × │   × │   × │
 │     │     │     │     │
╭╯    ╭╯    ╭╯    ╭╯    ╭╯
┼─────┼─────┼─────┼─────┼

[Luthaf]

We looked at the code very carefully again, and I think we both understand it now! I might do a diagram of the 2D version of it, but trying to represent the 3D version in ASCII is a bit clunky

[liam-o-marsh]

I think I understand what the hard part is in the understanding.
it's the condition that starts the block at https://github.com/Luthaf/rascaline/blob/3b0178213c13032e2f2d0b6c31f6ba5d8621ae84/rascaline/src/systems/neighbors.rs#L260, right?
to build this condition, my reasoning is:

• for two same-atom pairs characterised by (h,j,k) and (-h,-j,-k), one of them has to be ignored. how do you choose?
• first try to divide space along the h+j+k=0 plane.
• if it doesn't work, divide by the k=0 plane
• if it still doesn't work, divide by the j=0 plane.
• if the two pairs cannot be separated by any of those planes, then they are both at (h=0,j=0,k=0), which isn't possible

[Luthaf]

Yes, that was the issue. I split the condition in two and added a small illustration of the 2D case