Skip to content

Conversation

uellue
Copy link

@uellue uellue commented Oct 7, 2025

A recent CIF file pulled from https://icsd.fiz-karlsruhe.de/ for collection code 163723 contained the atom type symbol "Au0+" instead of just Au. This lead to empty diffraction patterns.

With this change only the first alphabetical
part of the atom type symbol is used as an element, and a warning is emitted if unknown elements are encountered.

See https://www.iucr.org/__data/iucr/cifdic_html/1/cif_core.dic/Iatom_type_symbol.html for the definition of the atom type symbol.

It can be debated if unrecognized atom type symbols should actually be a hard error since the current
code will quietly zero the contribution of sites with unknown atom types, which may lead to plausible but wrong diffraction patterns if recognized and unrecognized atom type symbols are mixed in a CIF file.

Description of the change

Progress of the PR

Minimal example of the bug fix or new feature

EntryWithCollCode163723.cif.txt

from diffpy.structure import loadStructure
from diffsims.generators.simulation_generator import SimulationGenerator
from orix.quaternion import Rotation
from orix.crystal_map import Phase
gen = SimulationGenerator(
    accelerating_voltage=300,
)
rot = Rotation.from_axes_angles(
    [1, 0, 0], 0, degrees=True
)
structure = loadStructure('EntryWithCollCode163723.cif')
p = Phase(structure=structure, space_group=225)
sim = gen.calculate_diffraction2d(phase=p, rotation=rot)
# Previously gave you an empty list
print(sim.coordinates)

For reviewers

  • The PR title is short, concise, and will make sense 1 year later.
  • New functions are imported in corresponding __init__.py.
  • New features, API changes, and deprecations are mentioned in the
    unreleased section in CHANGELOG.rst.
  • Contributor(s) are listed correctly in credits in diffsims/release_info.py and
    in .zenodo.json.

A recent CIF file pulled from https://icsd.fiz-karlsruhe.de/
for collection code 163723 contained the atom type symbol "Au0+"
instead of just Au. This lead to empty diffraction patterns.

With this change only the first alphabetical
part of the atom type symbol is used as an element, and a warning is
emitted if unknown elements are encountered.

See https://www.iucr.org/__data/iucr/cifdic_html/1/cif_core.dic/Iatom_type_symbol.html
for the definition of the atom type symbol.

It can be debated if unrecognized atom type symbols should actually
be a hard error since the current
code will quietly zero the contribution of sites with unknown atom types, which may
lead to plausible but wrong diffraction patterns if recognized and
unrecognized atom type symbols are mixed in a CIF file.
@uellue uellue changed the title Support CIF files from ICSD Support CIF files from ICSD with atom type symbols like Au0+ Oct 7, 2025
@uellue
Copy link
Author

uellue commented Oct 7, 2025

pre-commit.ci autofix

@uellue
Copy link
Author

uellue commented Oct 7, 2025

The docs failure seems to be an issue with Sphinx or an extension?

@hakonanes hakonanes added the enhancement New feature, request, or improvement label Oct 7, 2025
@hakonanes
Copy link
Member

Good fix, @uellue!

The doc fail is related to pyxem/orix#570, I think.

@CSSFrancis, how about a patch release v0.7.1 with this fix? Nothing on main since v0.7.0.

Signed-off-by: Håkon Wiik Ånes <hwaanes@gmail.com>
@CSSFrancis
Copy link
Member

@hakonanes Sounds good to me :)

uellue added 3 commits October 8, 2025 09:28
This should be about right?
Specify more clearly what kind of strings are accepted, update to
new feature of supporting more valid atom type symbols found
in CIF files in the wild.
@uellue
Copy link
Author

uellue commented Oct 8, 2025

About __init__.py I am unsure: Should get_element() be included there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, request, or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants