refactor: neighbours are now exposed by the neighbour lists themselves

[V-Francois]

Summary
This PR moves all neighbour-list related logic to the neighour subpackage, so that all other modules are agnostic of the underlying implementation.

This is accomplished by making each neighbour list type callable (with system and i are arguments). The call to the instances return generators, that when iterated upon return the indices of all the particles adjacent to i.
The methods to compute energies leverage these newly defined methods instead of encoding the neighbour list logic themselves.

Implementation discussion
For linked lists, I wasn't able to return a simple generator in the dedicated calling function. It looks like Julia doesn't have some of the features that would be required to create custom generators (something like the yield keyword in Python).
Instead, I created a new struct that is used to iterate over.
This adds some complexity and makes the iteration process less clear, which is opposite to the goal of this PR.
I believe this complexity is inherent to the linked list algorithm, and is now fully contained in the implementation of this list (rather than spread to other parts), so I'd argue that this PR still improves overall clarity.

Test plan
CI, which contains tests that rely on these lists.
Locally, tests on a polymer chain.
Currently, unit tests on molecules fail - need to be fixed.

[codecov]

Codecov Report

❌ Patch coverage is 82.50000% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/neighbours.jl	79.41%	7 Missing ⚠️

Files with missing lines	Coverage Δ
src/atoms.jl	96.87% <100.00%> (+19.68%)	⬆️
src/molecules.jl	71.11% <100.00%> (-0.08%)	⬇️
src/neighbours.jl	73.07% <79.41%> (+0.44%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[V-Francois]

@romainljsimon

[V-Francois]

I did some tests, and for LinkedList there is actually a huge performance hit from this implementation.
Maybe it's the call to push! that does this?
I'll investigate and try to come up with a better solution.

[V-Francois]

I updated the implementation, which removed the performance bottleneck arising from using push!.
It is however now more complex. I updated the description accordingly.

Feel free to challenge the content of this PR!

[romainljsimon]

I like the idea of using Base.Iterator to go through the neighbours, which is guess the most Julia way of doing this.
Here it is used only for LinkedList, couldn't we create an iterator for each type of list for consistency?
I'm thinking that maybe get_neighbour_indices is not even necessary as we could for j in listiterator that would go through all neighbours, even though it is not straightforward how to do that.

[V-Francois]

I like the idea of using Base.Iterator to go through the neighbours, which is guess the most Julia way of doing this. Here it is used only for LinkedList, couldn't we create an iterator for each type of list for consistency? I'm thinking that maybe get_neighbour_indices is not even necessary as we could for j in listiterator that would go through all neighbours, even though it is not straightforward how to do that.

I went with a mix of your suggestion and what was previously done: neighbour lists are now callable, and return an iterator.
For EmptyList and CellList, the iterator is hardcoded in the return value, whereas for LinkedList it is a secondary class with a Base.iterate method.
I initially started out by returning iterabale structus for the others, but implementing Base.iterate for nested loop (needed for CellList) results in a lot of boilerplate code and nested if/else to check for return values.

[V-Francois]

I finally found the bug that caused the unit test to fail.
The fix I did slowed down polymer simulations by ~3x.

Commit be8c121 fixed that. I'm shocked that such a small change affects the run time by a factor 3.

I find the linked list iterator extremely complicated for what is essentially a while loop in a for loop.

[romainljsimon]

Thanks a lot @V-Francois for this nice refactor :)