support gather/scatter #42
Conversation
src/LAMMPS.jl
Outdated
| elseif T === Float64 | ||
| dtype = 1 | ||
| """ | ||
| gather_atoms(lmp::LMP, name::String, T::Union{Type{Int32}, Type{Float64}}, count::Integer, ids::Union{Nothing, Array{Int32}}=nothing) |
There was a problem hiding this comment.
Expanding the docstring a bit would be nice.
src/LAMMPS.jl
Outdated
| if ids === nothing | ||
| natoms = get_natoms(lmp) | ||
| data = Array{T, 2}(undef, (count, natoms)) | ||
| API.lammps_gather(lmp, name, dtype, count, data) |
There was a problem hiding this comment.
What's the difference between gather and gather_atomis?
There was a problem hiding this comment.
It seems like gather_atoms is basically just a more restrictive version of gather. If I've understood it correctlly, gather_atoms only works on normal atom data, so the data you could get by calling extract_atoms while gather works on any per-atom property such as computes and fixes.
I've left gather_atoms in because I didn't want to remove any already existing methods
There was a problem hiding this comment.
we could use the same logic from extract_atom in gather_atom to determine the datatype and count of the property, wich would make it a more convenient version of gather where the datatype and count are determined through the C-API
There was a problem hiding this comment.
Yeah I think I would be okay with @deprecate gather_atoms gather
where the datatype and count are determined through the C-API
One downside of this is that the resulting Julia code is type-unstable.
So having the user provide the eltype of the output is probably a better API design,
but we should safety check that the LAMMPS eltype corresponds.
|
I still have to implement automated tests. Hopefully I'll get to that soon. During testing I've noticed that LAMMPS crashed multiple times, taking julia with it, probably due to illegal memory access. I've added a ton of safety checks and it should now be hopefully impossible (or at least considerably more difficult) to crash LAMMPS due to bad input. |
Adresses #41
I've decided to roll
lammps_gather(),lammps_gather_subset()into one function. I've done the same forgather_atoms()andscatter!()