Incorrect SpaceGroup symbol attribute for 16 space group types
Python version
3.10
Pymatgen version
2024.5.1 or master
Operating system version
No response
Current behavior
output of minimal example:
48 Pnnn1
50 Pban1
59 Pmmn1
68 Ccce1
70 Fddd1
85 P4/n1
125 P4/nbm1
126 P4/nnc1
129 P4/nmm1
130 P4/ncc1
201 Pn-31
203 Fd-31
222 Pn-3n1
224 Pn-3m1
227 Fd-3m1
228 Fd-3c1
16
Expected Behavior
For the minimal example provided below I would expect len(err) to be zero, meaning that every SpaceGroup symbol attribute provides a valid string that can be used to instantiate again a SpaceGroup object.
However, this is not the case for 16 space group types (see above). They all differ from their correct Hermann Mauguin symbol by an additionally appended "1" (resulting even in an impossible fourth symmetry direction / blickrichtung for most of them).
The symbol attribute is set here https://github.com/materialsproject/pymatgen/blob/74e692b0186ec77881acd3adbf1bbefeea8ca096/pymatgen/symmetry/groups.py#L226
I am not sure what the "universal" Hermann Mauguin symbol ("universal_h_m" key) and the additional :1 represents and why the key "hermann_mauguin" is not used instead, but the current code gives wrong space group symbols in 16 cases.
Minimal example
from pymatgen.symmetry.groups import SpaceGroup
err = []
for sg_num in range(1, 231):
sg = SpaceGroup.from_int_number(sg_num)
sg_symbol = sg.symbol
try:
SpaceGroup(sg_symbol)
except ValueError as e:
print(sg_num, sg_symbol)
err.append(sg_num)
print(len(err))
Relevant files to reproduce this bug
No response
It affects the Materials Project website as well.
Before @kaueltzen starts to correct it, we would like to ask for feedback on this as this will be a breaking change with potentially major effects. The symbol is clearly wrong. Nevertheless, people work with the wrong symbol.
@shyuep @janosh, thoughts?
Any thoughs from MP side: @munrojm @tschaume @yang-ruoxi ?
Thanks in advance.
@shyuep ?
We will start fixing it tomorrow as it is a major bug. Please let us know by end of this day if you object.
@JaGeo thanks for tagging us. We're good with the change from MP's side.
Actually, there are still open questions and I would appreciate feedback on them by @shyuep or @janosh :
If we decide on the standard Hermann-Mauguin symbol as the symbol attribute, the seven rhombohedral space group types would not contain information on their setting anymore (R-centered hexagonal cell or primitive rhombohedral cell). In this case, the current implementation of the is_compatible() method would not behave as previously as the appended ":H"/":R" is not part of the Hermann-Mauguin symbol and all rhombohedral space group types would always be evaluated as in the rhombohedral setting.
One solution could be to add a hexagonal bool parameter to the init of SpaceGroup as it is already done in the classmethod from_int_number(). What do you think?
Also, I have a question regarding the init:
https://github.com/materialsproject/pymatgen/blob/74e692b0186ec77881acd3adbf1bbefeea8ca096/pymatgen/symmetry/groups.py#L232
The point_group attribute may be set as spg["schoenflies"]. However, this key seems generally to refer to the Schoenflies space group (not point group) notation. By removing the superscript, one would arrive at the corresponding point group symbol and I would suggest this change.
Also, regarding SYMMOPS.json: the key "crystal_class" does not refer to the actual crystal class but for some entries to the crystal system, for some entries to the lattice system (or both if identical). Although this key is not used right now, I would highly recommend correcting it in case it is going to be used in the future and decide on whether to include a crystal system or lattice system key. What are your thoughts?
Thanks in advance (:
Hi all,
We are also happy to just correct these issues according to @kaueltzen 's suggestions. It will, however, be a breaking change. But as this seems wrong, we should correct it.
question for @kaueltzen and @JaGeo: the SpaceGroup.symbol is documented to return the full HM symbol but as reported in https://github.com/spglib/moyo/issues/83, it actually returns the short symbol for all but the 13 triclinic space groups. did you also notice this inconsistency? is there any reason for keeping that?
either way, @shyuep the docs need correcting and/or the attribute changed to actually return the full HM symbol