Closed lmiq closed 2 years ago
Merging #45 (5400dfa) into master (48a9713) will increase coverage by
0.45%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #45 +/- ##
==========================================
+ Coverage 72.05% 72.50% +0.45%
==========================================
Files 24 24
Lines 1839 1833 -6
==========================================
+ Hits 1325 1329 +4
+ Misses 514 504 -10
Impacted Files | Coverage Δ | |
---|---|---|
src/neighbors.jl | 91.39% <100.00%> (+2.85%) |
:arrow_up: |
src/types.jl | 72.05% <0.00%> (+7.35%) |
:arrow_up: |
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 48a9713...5400dfa. Read the comment docs.
Great, thanks for doing this.
Dear @jgreener64, I have updated the interface of the CellListMap neighbor finder to the 0.7.0 version, which has some breaking changes (hopefully the last ones until 1.0).
The changes are:
Now CellListMap propagates units properly through the code, so no need anymore of stripping units before passing the data to it. Because of that, one less type parameter is necessary in the constructor as well. (see here). I have added an
number_of_batches
option to the constructor, although I don't think it will be used in practice. Since the number of batches is not anymore necessarily equal to the number of threads, the preallocated output arrays do not have lengthnthreads()
, and thus their resetting is performed directly on their length (actually this is better and can cooperate with nested multithreading much more).The number of batches used in parallel calculations are defined by an heuristic (or can be set by the user), to improve the performance particularly if many threads are being used. The new approach avoids some latency problems, uses less memory and avoids some huge GC workloads particularly when parallelizing over many threads. (see here)
Tests are passing here.
Performance should not be significantly affected, but for specific, particularly small (~10^4 particles) you may experience small slowdowns, maybe, but nothing really important. Anyway, the bottlenecks are now much more clear and any improvement on this side will be performed from v0.7 on.
The code, particularly because the units can be propagated through CellListMap, is simpler now.
Happy new year.