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

Add _atom_type.mass_number #400

Merged
merged 4 commits into from
May 31, 2023

Conversation

rowlesmr
Copy link
Collaborator

Will close #393

Now an atom_type can have a specific isotope.

This will eventually allow for default scattering lengths to be enumerated, if #399 is accepted.

@vaitkus
Copy link
Collaborator

vaitkus commented May 22, 2023

I am not sure if we should have a default value here as if may clash with the general dictionary design policy (see discussion in https://www.iucr.org/__data/iucr/lists/coredmg/msg00460.html).

cif_core.dic Outdated Show resolved Hide resolved
Co-authored-by: Antanas Vaitkus <[email protected]>
@merkys
Copy link
Contributor

merkys commented May 23, 2023

I like the implementation, but share @vaitkus's concern about default values. Even if the default values get accepted, there should be a reference to their source.

@rowlesmr
Copy link
Collaborator Author

rowlesmr commented May 24, 2023

The enumeration defaults are taken from ITC table 4.4.4.1.

The default value of _atom_type.mass_number being defined as . is meant as a signal that an element of natural abundance is used. If the creator of a CIF skips this, and instead directly defines _atom_type_scat.length_neutron, then the default value doesn't imact on the numerical value of that data name. But there would be a semantic clash. Could the default value be downgraded to ? to give a value for enumeration defaults to latch on to while removing a sematic clash? (or is that already the default default value...? I don't have my references handy to check...)

In general, templ_enum is missing references for all of its values: is there a tag to give a reference to the source of a value?

@vaitkus
Copy link
Collaborator

vaitkus commented May 24, 2023

The problem with using '.' as the default is that the natural abundance would be automatically assumed even if the author did not actually mean it (the explicit mass number data item might be missing for multiple reasons). While this default assumption is likely to be correct most of the time, the new safer approach adopted by the IUCr is to not assume any default values unless they can be easily derived from other values (see [1]). Therefore, changing the default to ? (unknown) seems like the most truthful representation.

That being said, I would probably remove the _enumeration.default attribute altogether since it seems to be logically equivalent to having an attribute with an unknown value.

References:
[1] https://www.iucr.org/__data/iucr/lists/coredmg/msg00460.html

@rowlesmr
Copy link
Collaborator Author

Roger that. That all makes sense.

I'll remove the default value altogether, but keep the special value of . meaning natural abundance.

@jamesrhester
Copy link
Contributor

I'm still amazed that we didn't have this is cif core before. Even mmCIF only has it for nmr work. Anyway, once the default is removed happy to merge.

rowlesmr and others added 2 commits May 28, 2023 16:19
Improved wording for _atom_type.mass_number definition.
@jamesrhester jamesrhester merged commit 0999a11 into COMCIFS:master May 31, 2023
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.

How to represent isotopes?
4 participants