Refactor particleset attributes and particle data for vectorized kernels

[erikvansebille]

Following from #2122, it would be good to rethink the API and refactor the code for particleset data. See below the original comment by @VeckoTheGecko

Hmm, I assume that this change was needed since we are passing the particleset all the way to the kernels now? I think we should at some point pass the particle data itself down - as I touched on f2f, there is a distinction between the particle data and the ParticleSet (e.g., it doesn't make sense to call pset.execute() in a kernel). By passing the particledata down to the kernel it makes this clear. This also means that we don't silently limit users variable names (currently they can't call their variable names execute or anything that is already a method/attr on ParticleSet).

Happy for that to be a future issue.

Perhaps:

• Make ParticleData its own class (dict of arrays under the hood, but allowing .attr notation to get and set values)
  • Only have private variables/methods/names on this dataset (i.e., _data). All public attribute access will be assumed to correspond with particle data -> this results in no shadowing
  • Disallow names starting with _ in parcels/particle.py@Variable as this could collide with PariticleData
• Pass ParticleData down to the kernels
• Update pset._data to be pset.data so that its on our public API.
  • @erikvansebille can we then remove the __getattr__ and __setattr__ methods on the pset? I don't think its too prohibitive to do pset.data.lon if users want to access their lon values, or would you prefer to still have the pset.lon? (a getattr? or also a setattr?)

Also, thoughts on explicitly disallowing object.__setattr__(self, name, value) in that last else clause? I know that its to maintain the default behaviour for Python objects, but I don't see the benefit in supporting it here - and since it works outside of our model it means that that data would fall through the cracks if we write particle data (and also means that if people typo in their kernel it will be a silent bug)

Originally posted by @VeckoTheGecko in #2122 (comment)

[erikvansebille]

Note that with the change in bee5464, we now pass a list of KernelParticles to the Kernels, instead of the entire ParticleSet:
https://github.com/OceanParcels/Parcels/blob/081753dbce1ea327d1c2ed6fcc147d81183caa10/parcels/kernel.py#L255-L258

So this might solve (some) of the problems above?

[VeckoTheGecko]

we now pass a list of KernelParticles to the Kernels, instead of the entire ParticleSet

*not a list of KernelParticle instances, but a KernelParticle instance where the indices is a boolean mask (instead of single indices - 1, 2, 3 etc. - as it was before)

Yes, KernelParticle in #2122 is helping with the problems above. But naming is now a bit confusing as it isn't about a single particle, and would be good to formalise this properly in a particle data class with proper testing. For a future PR - I can work on that once I'm back (also fielding some interesting ideas from people here, which has been helpful)

[erikvansebille]

Another aspect to consider is the implementation of __getitem__:

https://github.com/OceanParcels/Parcels/blob/d4fdc01e9913db51dd18e4eb1afcf2e6c8250b02/parcels/particle.py#L141-L143

See also #2122 (comment)

[erikvansebille]

Note that #2199 (comment) now has a minimal breaking example

[VeckoTheGecko]

@erikvansebille is this the issue most relevant to the KernelParticle discussion? Also, I feel like there was another issue with test cases (repeatedly assigning to variables and that causing broken state) but I can't seem to find it

[VeckoTheGecko]

I feel like there was another issue with test cases (repeatedly assigning to variables and that causing broken state)

Found it #2199 (comment)