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

Remove proton/neutron Number #192

Open
wants to merge 1 commit into
base: upcoming-2.0.0
Choose a base branch
from

Conversation

RemiLehe
Copy link
Member

@RemiLehe RemiLehe commented Apr 3, 2018

This removes the proton/neutron number from the ED-PIC extension, since it is now possible to specify it with SpeciesType.

Implements issue: Close #191

Affected Components

  • EXT_ED-PIC

Logic Changes

Expressing ions in ED-PIC can now be done by using the SpeciesType extension.

Writer Changes

Data writers that were writing the proton/neutron number should now use the SpeciesType extension.

This might require a lookup of an element name in the periodic table for the naming. Previously, one could just define two numbers without knowing how an ion/isotope is abbreviated in the periodic table.

Reader Changes

I am not aware of any reader that currently checks this attribute, but in theory data readers should now look at the SpeciesType information.

Data Converter

Data converters should switch to using the SpeciesType.

@RemiLehe RemiLehe requested a review from ax3l April 3, 2018 15:28
@RemiLehe RemiLehe added EXT: ED-PIC electro-magnetic particle-in-cell extrension EXT: SpeciesType physical particle species extension labels Apr 3, 2018
@ax3l ax3l self-assigned this Apr 3, 2018
@ax3l
Copy link
Member

ax3l commented Apr 3, 2018

I am not aware of any reader that currently checks this attribute

We do in manual post-processing from time to time.

Does the openPMD validator check the proton number?

Yes, it does check its type. We need to remove these two lines: openPMD/openPMD-validator#39

Data converters should switch to using the SpeciesType.

I would like to hold this PR until we have a data converter PR for it. This way of working we will only merge changes to the standard that we can also convert to :)

(The data conversion will need to implement a periodic table lookup, that's fun! :-p )

Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor typo.

@@ -226,6 +226,10 @@ Particle Records (Macroparticles)
The following additional attributes are defined in this extension.
The individual requirement is given in `scope`.

Note that the attributes below to not include information about the species
Copy link
Member

@ax3l ax3l Apr 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to -> do

Suggested change
Note that the attributes below to not include information about the species
Note that the attributes below do not include information about the species

@ax3l ax3l changed the title Close #191 Remove proton/neutron Number Remove proton/neutron Number Nov 8, 2018
@DavidSagan DavidSagan mentioned this pull request Nov 5, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EXT: ED-PIC electro-magnetic particle-in-cell extrension EXT: SpeciesType physical particle species extension
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

2 participants