Picongpu: Incorrect naming of particle shapes in PIConGPU?

[steindev]

I think the shapes of order m>=3 (PCS, P4S) are named incorrectly in PIConGPU.
Hockney (Computer Simulation Using Particles, p. 144) names the 3rd order shape PQS, which makes sense as it could mean "Piecewise Quadratic Spline" which exactly describes the form of the cloud shape, and is in line with the other schemes NGP, CIC, and TSC, which describe the form of the cloud shape, too.
The 4th order could then be named PCS="Piecewise Cubic Spline" which again describes exactly the cloud shape.
Therefore, I propose to rename

PCS => PQS
P4S => PCS

in PIConGPU.

Shall I prepare a pull request?

[psychocoderHPC]

Yes they are named wrongly @ax3l showed it also in his PHD theses. Could you have a look therebefore you change the naming.
IMO feel free to fix the naming.

[sbastrakov]

I would say not wrongly, but inconsistently betwen different schemes. Btw I've recently added comments to implementations of each shape so that there is explicitly noted what is the cloud shape and what is the assigment function. Also noted the inconsistencies in the comments. It was PR #3376

[sbastrakov]

Just as a note, I agree that your proposed renaming would be correct. However, it can also be that old setups using PCS would wtill compile as it would still be a valid name, but would mean differently.

[steindev]

Just as a note, I agree that your proposed renaming would be correct. However, it can also be that old setups using PCS would wtill compile as it would still be a valid name, but would mean differently.

Indeed. But this is acceptable, isn't it?

[sbastrakov]

I think it is. We probably need to document all the user interface changes somewhere, especially since we are planning to move some parameters to be run time, and compile time

[steindev]

It will be listed in the changelog. If users read it, they will know. :grin:

[sbastrakov]

Sure, we already do the changelog. I just feel sometimes it is too high level and maybe hard to understand for an infrequent PIConGPU user. However, this thought is of course not connected to this particular change.

[psychocoderHPC]

Should we maybe rename the shapes and add a using to keep also the old names? The only problem is the name PCS.

[sbastrakov]

I would find it even more confusing tbh even regardless of the PCS collision.

[Anton-Le]

Since P4S -> PCS would be an increase in the order of the approximating polynomial I'd say it is a substitution that is acceptable (the other way around would be bad).

Perhaps it would be wort adding a warning comment to the shape definitions that the names have been changed (at least for a while, until v0.6.0 or so).

[sbastrakov]

I think that's a good idea @Anton-Le . We already have a list of valid names in the corresponding .param, for the changed shapes could easily add the old name there.

[PrometheusPi]

I agree with @Anton-Le and @sbastrakov - making the renaming consistently and for a time as verbose as possible would be, in my opinion, the best way to go. Thus a disagree with @psychocoderHPC suggestion of having a usingto keep the old naming - this would be highly confusing.

[steindev]

Yes they are named wrongly @ax3l showed it also in his PHD theses. Could you have a look therebefore you change the naming.
IMO feel free to fix the naming.

Checked with ax3l's thesis, p. 26. My proposal above is in line with his presented notation.

PR is open, see #3431.