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

CifParser check() method does not account for different site multiplicities #3626

Closed
kaueltzen opened this issue Feb 14, 2024 · 1 comment · Fixed by #3628
Closed

CifParser check() method does not account for different site multiplicities #3626

kaueltzen opened this issue Feb 14, 2024 · 1 comment · Fixed by #3628
Labels

Comments

@kaueltzen
Copy link
Contributor

Python version

3.10.0

Pymatgen version

2024.2.8 or master

Operating system version

No response

Current behavior

The check() method of the CifParser class performs checks on the compositions reported in both cif and Structure object parsed from cif. Besides other things, it is checked if the relative stochiometry from cif and structure are the same.

If the cif file is missing both the keys _chemical_formula_sum and _chemical_formula_structural, cif_formula is constructed from the atom types of all crystallographically unique sites in the structure:

cif_formula = " ".join(cif_as_dict[head_key]["_atom_site_type_symbol"])

However, if the site multiplicities differ within the structure, the formula of the Structure object will have a different relative stochiometry. This leads to a UserWarning: Incorrect stoichiometry even if the cif file is not faulty.

Expected Behavior

No UserWarning to be raised for the below mentioned example.

In my opinion, checking the relative stochiometry should be skipped if both _chemical_formula_sum and _chemical_formula_structural are not present in the cif. To obtain the true stochiometry (including multiplicities) from the cif without these keys, its symmetry operations would need to be applied as it is also done for constructing the Structure object - therefore, this would no longer be a sanity check (?).

Minimal example

from pymatgen.io.cif import CifParser
import warnings

warnings.filterwarnings("error")

struct = CifParser("path-to-example-cif").parse_structures(primitive=True)[0]

Relevant files to reproduce this bug

The bug can be for example reproduced with the mcif file of MAGNDATA entry 0.296:
https://www.cryst.ehu.es/magndata/index.php?this_label=0.296

This structure contains three crystallographically unique Cu sites, out of which two each have a site multiplicity of 2 and one has a multiplicity of 4. The site multiplicity of all unique anion sites is also 4. This results in a warning:
UserWarning: Incorrect stoichiometry: CIF={'Cu': 3.0, 'Cl': 1.0, 'O': 3.0} PMG={'Cu': 8.0, 'Cl': 4.0, 'O': 12.0} ratios={'Cl': 4.0, 'O': 4.0, 'Cu': 2.6666666666666665}

@kaueltzen
Copy link
Contributor Author

Mentioned this in PR #3628

janosh added a commit that referenced this issue Feb 22, 2024
…3628)

* Modified check method of CifParser as one possible way to fix issue 3626.

* Reincluded all CifParser check() checks but for relative stoichiometry in case of missing formula keys.

* Added CifParser test for skipping relative stoichiometry check in case of missing formula keys and for appending a warning to self.warnings in this case.

* Refactor CifParser.check

* tighter warning test

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant