Resolve requested changes:

- Add `@inbounds` in `push_cell_atomic!`
- Improved type dispatch for `supported_update_strategies`
- Clarified and cleaned up cell list initialization and emptying,
- General code cleanup.

Author: RubberLanding

src/cell_lists/cell_lists.jl

@@ -12,7 +12,7 @@ end
 
 # We need the prod() because FullGridCellList's size is a tuple of cells per dimension whereas
 # SpatialHashingCellList's size is an Integer for the number of cells in total.
-function construct_backend(::Type{<:AbstractCellList}, ::Type{Vector{Vector{T}}},
+function construct_backend(::Type{Vector{Vector{T}}},
                            max_outer_length,
                            max_inner_length) where {T}
     return [T[] for _ in 1:max_outer_length]
@@ -39,7 +39,15 @@ function construct_backend(cell_list::Type{<:AbstractCellList},
                              max_inner_length)
 end
 
+function max_points_per_cell(cells::DynamicVectorOfVectors)
+    return size(cells.backend, 1)
+end
+
+# Fallback when backend is a `Vector{Vector{T}}`. Only used for copying the cell list.
+function max_points_per_cell(cells)
+    return 100
+end
+
 include("dictionary.jl")
 include("full_grid.jl")
 include("spatial_hashing.jl")
-include("cell_lists_util.jl")

src/cell_lists/full_grid.jl

@@ -192,15 +192,6 @@ function copy_cell_list(cell_list::FullGridCellList, search_radius, periodic_box
                             max_points_per_cell = max_points_per_cell(cell_list.cells))
 end
 
-function max_points_per_cell(cells::DynamicVectorOfVectors)
-    return size(cells.backend, 1)
-end
-
-# Fallback when backend is a `Vector{Vector{T}}`. Only used for copying the cell list.
-function max_points_per_cell(cells)
-    return 100
-end
-
 @inline function check_cell_bounds(cell_list::FullGridCellList{<:DynamicVectorOfVectors{<:Any,
                                                                                         <:Array}},
                                    cell::Tuple)

src/cell_lists/spatial_hashing.jl

@@ -40,12 +40,15 @@ end
 
 @inline Base.ndims(::SpatialHashingCellList{NDIMS}) where {NDIMS} = NDIMS
 
-function supported_update_strategies(::SpatialHashingCellList{<:DynamicVectorOfVectors})
+function supported_update_strategies(::SpatialHashingCellList{NDIMS, CL, CI, CF}) where {NDIMS,
+                                                                                         CL <:
+                                                                                         DynamicVectorOfVectors,
+                                                                                         CI,
+                                                                                         CF}
     return (ParallelUpdate, SerialUpdate)
 end
-
 function supported_update_strategies(::SpatialHashingCellList)
-    return (SerialUpdate)
+    return (SerialUpdate;)
 end
 
 function SpatialHashingCellList{NDIMS}(list_size,
@@ -60,15 +63,15 @@ function SpatialHashingCellList{NDIMS}(list_size,
 end
 
 function Base.empty!(cell_list::SpatialHashingCellList)
-    (; list_size) = cell_list
+    (; cells) = cell_list
     NDIMS = ndims(cell_list)
 
     # `Base.empty!.(cells)`, but for all backends
-    for i in eachindex(cell_list.cells)
-        emptyat!(cell_list.cells, i)
+    @threaded default_backend(cells) for i in eachindex(cells)
+        emptyat!(cells, i)
     end
 
-    cell_list.coords .= [ntuple(_ -> typemin(Int), NDIMS) for _ in 1:list_size]
+    fill!(cell_list.coords, ntuple(_->typemin(Int), NDIMS))
     cell_list.collisions .= false
     return cell_list
 end
@@ -102,15 +105,15 @@ function push_cell_atomic!(cell_list::SpatialHashingCellList, cell, point)
     @boundscheck check_cell_bounds(cell_list, hash_key)
     @inbounds pushat_atomic!(cells, hash_key, point)
 
-    cell_coord = coords[hash_key]
+    cell_coord = @inbounds coords[hash_key]
     if cell_coord == ntuple(_ -> typemin(Int), NDIMS)
+        # Throws `bitcast: value not a primitive type`-error
+        # @inbounds Atomix.@atomic coords[hash_key] = cell
         # If this cell is not used yet, set cell coordinates
-        # Atomix.@atomic coords[hash_key] = cell
-        coords[hash_key] = cell
+        @inbounds coords[hash_key] = cell
     elseif cell_coord != cell
         # If it is already used by a different cell, mark as collision
-        # Atomix.@atomic collisions[hash_key] = true
-        collisions[hash_key] = true
+        @inbounds Atomix.@atomic collisions[hash_key] = true
     end
 end
 
@@ -125,8 +128,6 @@ function copy_cell_list(cell_list::SpatialHashingCellList, search_radius,
     (; list_size) = cell_list
     NDIMS = ndims(cell_list)
 
-    # Here I'm using max_points_per_cell which is defined in src/cell_lists/full_grid.jl 
-    # Think about putting it somewhere all cell list can access it or copying it here
     return SpatialHashingCellList{NDIMS}(list_size, typeof(cell_list.cells),
                                          max_points_per_cell(cell_list.cells))
 end

src/nhs_grid.jl

@@ -379,8 +379,8 @@ end
 
 # Fully parallel incremental update with atomic push.
 # TODO `cell_list.cells.lengths` and `cell_list.cells.backend` are hardcoded
-# for `FullGridCellList` and `SpatialHashingCellList`, which are currently
-# the only implementations supporting this update strategy.
+# for `FullGridCellList`, which is currently the only implementation
+# supporting this update strategy.
 function update_grid!(neighborhood_search::GridNeighborhoodSearch{<:Any,
                                                                   ParallelIncrementalUpdate},
                       y::AbstractMatrix; parallelization_backend = default_backend(y),

test/cell_lists/spatial_hashing.jl

@@ -52,7 +52,7 @@
             points1 = nhs.cell_list[cell1]
             points2 = nhs.cell_list[cell2]
 
-            @test points1 == points2 == [1, 2]
+            @test sort(points1) == sort(points2) == [1, 2]
             @test cell1_hash == cell2_hash
         end