Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented propagation of input particle ID in the List modus #54

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

szymonharabasz
Copy link

Particle ID (next-to-last column of input file in the Oscar2013 format) was up to now ignored when reading the file. It is now saved in ParticleData. In addition, the method copy_to of ParticleData now copies particle ID if it is larger or equal 0. This was needed for a student. If you find it useful for the official version, that would be great.

Particle ID (next-to-last column of input file in the Oscar2013 format)
was up to now ignored when reading the file. It is now saved in
ParticleData. In addition, the method copy_to of ParticleData now copies
particle ID if it is not -1.
@AxelKrypton
Copy link
Member

@szymonharabasz Thanks for opening this PR. We have been thinking about the changes for a while and we agreed that we need to spend a bit more time on it. Although your change seem straightforward, we are not yet entirely sure that the implementation details of the Particles class remain valid, in particular the fact that each particle is always guaranteed to have a unique ID. For example, the Particles::insert method used in ListModus::try_create_particle uses in its implementation the Particles::copy_in method, which in turn calls ParticleData::copy_to that you modified. In many places in the documentation there is written that the ID is not taken over. For instance, the ParticleData::copy_to description states so and so does ParticleData::insert:

* \param[in] p The data to be added. The id will not be copied. Instead a new
* unique id is generated by the function.

If we want to set the ID in list modus reading in and using the user input file information, we need to ensure that the initialisation is correctly done. For instance, the same ID should not be repeated and invalid IDs should be e.g. somehow corrected (with a warning).

Therefore we decided to leave this on hold for a moment and we'll likely open a PR in our private development repository to add this feature. Maybe you could help us trying to describe your use case and what exactly was needed (e.g. why was it relevant to read in and store the particles ID?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants