LEoPart-project / leopart-x

Implementation of Lagrangian-Eulerian Particle tracking with DOLFINx
GNU Lesser General Public License v3.0
8 stars 1 forks source link

Store "per field" or "per particle"? #11

Closed jmmaljaars closed 4 years ago

jmmaljaars commented 4 years ago

The proposed particle lay-out stores the particle fields separately rather than stacking the particles into one long std::vector<double>, see

https://github.com/LEoPart-X/LEoPart/blob/0e5188805e6ecba9ce75ac8f2573859baba6db8f/src/particles.h#L50

and

https://github.com/LEoPart-X/LEoPart/blob/0e5188805e6ecba9ce75ac8f2573859baba6db8f/src/particles.h#L55

What are the pros/cons of the proposed lay-out over stacking the particles into one long std::vector<double>?

(Just as an aside, I presume "the same length as the numbers of particles" in

https://github.com/LEoPart-X/LEoPart/blob/0e5188805e6ecba9ce75ac8f2573859baba6db8f/src/particles.h#L53

should be "the same length as the number of particles * _field_shape" ?)

chrisrichardson commented 4 years ago

I thought a bit about this. I think there are advantages to keeping each field separate. For example, it is then very easy to add or delete fields. In a way, it is quite similar to the Function::vector() of DOLFIN - and the cell->particle list is like the dofmap.

There is not really any advantage into storing them all in one array. You are right about the comment, of course - it should be * _field_shape...

One issue which may come up, is that when we delete particles, there could be some unused entries. But I think that is easy to deal with.

chrisrichardson commented 4 years ago

Another way would be to have std::vector<std::tuple<std::vector<int>, std::vector<double>, std::string>> _fields where all the data for each field is collected together. Or even, we could have a Field class, then std::vector<Field>.

jmmaljaars commented 4 years ago

Agreed that separating fields allows to add/delete fields conveniently. And there certainly are use cases for that (e.g. multi-step schemes / particle field updating).

It only seems that extracting/deleting/communicating a particle becomes a bit more challenging (and/or inefficient)? For instance, pulling out a single particle requires a loop over the different fields, rather than just extracting a range of entries from one array. Not sure if this would be a serious drawback?

chrisrichardson commented 4 years ago

I don't think it will be a problem - anyway, we should be able to hide this implementation from the user, so we can change it later if necessary.

jmmaljaars commented 4 years ago

And how about computational efficiency, do you expect any difference between a "per field" or "per particle" implementation?

chrisrichardson commented 4 years ago

I don't feel there will be any performance difference, but we could do some benchmarks.

jmmaljaars commented 4 years ago

Let's give the "per field" implementation a try :+1: