P3M Merge

[tischwab0911]

Pull request to merge the P3M codes into the main repository.

[aaadelmann]

Why did you add the plot's? In general pdfs of results should not be in the repo. Are these plots used in your thesis?

[aliemen]

I skimmed a bit over the code (I won't have time to actually understand the solver itself until in two weeks...). But here are a view comments:

• I agree @aaadelmann, I think we should remove the plots/... and python_scripts/... folder for the merge., since they are not part of the solver itself?
• The files under test/p3m/... seem nice, and I think they could also easily be converted into unit tests.
• I added a view suggestions above concerning the code/file structure. I think, it might make sense to make an effort in keeping it consistent with the existing structure (mainly regarding Alpine mini apps and manager classes):
  1. Remove the P3M3DManager, since it is basically the same as PicManager (just a suggestion, there might be room for debate here).
  2. Remove src/p3m/ and put P3MParticleContainer together with the high level P3M manager(s) into a alpine/p3m/... folder.

I have not yet looked into the alpine/[...]P3M[...] (or src/PoissonSolvers/P3MSolver[...]) classes yet, since (as mentioned) I don't fully understand the algorithm yet... But I suppose it is probably not necessary to look too much into it just yet for this merge?

[aliemen]

Is this manager really necessary? If I am not wrong, then this is exactly the same as PicManager without the load balancer and with an additional par2par() method.

Also: this method seems to be a function that calculates actual interaction, so maybe, conceptually, it should not be part of a "manager" parent class anyways?

[0xBachmann]

P3M3DManager has its particle container fixed to P3MParticleContainer instead of having a template parameter or it. does this make it reason enough to have a separate class for it?

[aliemen]

This should be inside the class.

[aliemen]

Maybe I did not find it, but is phi even used anywhere?

[aliemen]

Should neighbors_m be a smart pointer instead of a regular pointer?

[0xBachmann]

this should be an array of bools. what is the best datatype to do this? is a std::vector possible or should it be a Kokkos::View?

[aliemen]

This could be measured using IpplTimings.

[aliemen]

2. Remove src/p3m/ and put P3MParticleContainer together with the high level P3M manager(s) into a alpine/p3m/... folder.

Update after todays IPPL Meeting: Let's not extend "alpine", but rather create a separate directory. Also: I think it would improve clarity if we created a new "examples" directory, where we put all these mini apps in. Maybe a folder structure like the following:

examples/
├─ alpine/
│  ├─ AlpineManager.h
│  ├─ ...
├─ collision/
│  ├─ P3M[...]Manager.hpp
│  ├─ P3MParticleContainer.hpp
│  ├─ datatypes.h
│  ├─ [...].cpp 
│  ├─ ...
├─ cosmology/
│  ├─ ...

Also I just notices you removed P3MParticleContainer.hpp (which is not what I meant above). I was thinking of removing P3M3DManager.h, since it is basically the same as the PicManager.h, but with a fixed particle container type. Since PicManager is templated on the container type, one can easily use it together with P3MParticleContainer.hpp.

@0xBachmann be carefule with the difference between a manager class and a container class ;):

• Container only saves particle information (e.g. attributes like position, charge, mass) and their layout (e.g. "where on the machine"). On a high level, they should inherit from e.g. ParticleBase.
• Managers manage the simulation. They hold the containers (particle/field) and all other tools (like the solver instance) and define what happens in the simulation every timestep. On a high level, they should inherit from PicManager for particle simulations.

Therefore, removing the only P3MContainer is not a good idea, it should be high level part of the "P3M mini apps". Removing the code duplication introduced by the P3M3DManager, which does not need to be a high level specialization, however, should improve clarity.