Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increasing number of batches causes an error in map_pairwise! #106

Open
efaulhaber opened this issue Dec 2, 2024 · 2 comments
Open

Increasing number of batches causes an error in map_pairwise! #106

efaulhaber opened this issue Dec 2, 2024 · 2 comments

Comments

@efaulhaber
Copy link
Contributor

Say we have a cell list.

x = zeros(3, 1)
search_radius = 0.1
box = Box(limits(x), search_radius)
cell_list = CellList(x, box)

Then we get some more particles and update our cell list accordingly:

x2 = rand(3, 100)
box = Box(limits(x2), search_radius)
UpdateCellList!(x2, box, cell_list)

The cell list is now using only a single thread if we used too few particles at the beginning:

julia> cell_list.nbatches
  Number of batches for cell list construction: 1
  Number of batches for function mapping: 1

With #87 fixed, we can set the number of batches as follows.

CellListMap.set_number_of_batches!(cell_list)

But now, we get an error.

julia> map_pairwise!(0, box, cell_list, parallel=true) do x, y, i, j, d2, output
           return output
       end
ERROR: TaskFailedException

    nested task error: BoundsError: attempt to access 0-element Vector{CellListMap.ProjectedParticle{3, Float64}} at index [1]
    Stacktrace:
     [1] throw_boundserror(A::Vector{CellListMap.ProjectedParticle{3, Float64}}, I::Tuple{Int64})
       @ Base ./essentials.jl:14
     [2] setindex!
       @ ./array.jl:975 [inlined]
     [3] project_particles!(projected_particles::Vector{…}, cellⱼ::CellListMap.Cell{…}, cellᵢ::CellListMap.Cell{…}, Δc::SVector{…}, Δc_norm::Float64, box::Box{…})
       @ CellListMap ~/.julia/packages/CellListMap/0BWXe/src/CoreComputing.jl:357
     [4] _vicinal_cell_interactions!(f::var"#249#250", box::Box{…}, cellᵢ::CellListMap.Cell{…}, cellⱼ::CellListMap.Cell{…}, cl::CellList{…}, output::Int64, ibatch::Int64)
       @ CellListMap ~/.julia/packages/CellListMap/0BWXe/src/CoreComputing.jl:272
     [5] inner_loop!(f::var"#249#250", _neighbor_cells::typeof(CellListMap.neighbor_cells_forward), box::Box{…}, cellᵢ::CellListMap.Cell{…}, cl::CellList{…}, output::Int64, ibatch::Int64)
       @ CellListMap ~/.julia/packages/CellListMap/0BWXe/src/CoreComputing.jl:220
     [6] inner_loop!
       @ ~/.julia/packages/CellListMap/0BWXe/src/CoreComputing.jl:200 [inlined]
     [7] batch(f::var"#249#250", ibatch::Int64, nbatches::Int64, n_cells_with_real_particles::Int64, output_threaded::Vector{…}, box::Box{…}, cl::CellList{…}, p::Nothing)
       @ CellListMap ~/.julia/packages/CellListMap/0BWXe/src/CoreComputing.jl:124
     [8] (::CellListMap.var"#111#113"{var"#249#250", Int64, Int64, Int64, Vector{}, Box{}, CellList{}, Nothing})()
       @ CellListMap ~/.julia/packages/CellListMap/0BWXe/src/CoreComputing.jl:147
    Some type information was truncated. Use `show(err)` to see complete types.

...and 4 more exceptions.

Stacktrace:
 [1] sync_end(c::Channel{Any})
   @ Base ./task.jl:466
 [2] macro expansion
   @ ./task.jl:499 [inlined]
 [3] map_pairwise_parallel!(f::var"#249#250", output::Int64, box::Box{…}, cl::CellList{…}; output_threaded::Nothing, reduce::typeof(CellListMap.reduce), show_progress::Bool)
   @ CellListMap ~/.julia/packages/CellListMap/0BWXe/src/CoreComputing.jl:146
 [4] map_pairwise_parallel!
   @ ~/.julia/packages/CellListMap/0BWXe/src/CoreComputing.jl:133 [inlined]
 [5] #map_pairwise!#124
   @ ~/.julia/packages/CellListMap/0BWXe/src/CellListMap.jl:112 [inlined]
 [6] top-level scope
   @ REPL[96]:1
Some type information was truncated. Use `show(err)` to see complete types.
@efaulhaber
Copy link
Contributor Author

efaulhaber commented Dec 2, 2024

Okay, I see that the update uses the number of batches. So one cannot just change the number of batches without updating afterward.

@lmiq
Copy link
Member

lmiq commented Dec 2, 2024

Thanks for reporting. I have seen your PR, thanks. I'm thinking about the best interface to this, maybe I'll just update the number of batches by default except if they are set manually.

Anyway, since this can introduce correctness issues, I'll only release after very careful testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants