materialsproject / pymatgen

Python Materials Genomics (pymatgen) is a robust materials analysis code that defines classes for structures and molecules with support for many electronic structure codes. It powers the Materials Project.
https://pymatgen.org
Other
1.53k stars 868 forks source link

possible bug in HighSymmKpath #609

Closed albalu closed 1 year ago

albalu commented 7 years ago

System

Summary

Example code

from pymatgen import MPRester
from pymatgen.symmetry.bandstructure import HighSymmKpath

api = MPRester()
s = api.get_structure_by_material_id('mp-773965', final=True)
ibz = HighSymmKpath(s)
print ibz.kpath["path"]

Error message

print ibz.kpath["path"]
TypeError: 'NoneType' object has no attribute '__getitem__'
gpetretto commented 7 years ago

Hi, I want to point out that this is related to the issue #374. The problem originates from the fact that spglib at some point started to use and return the symbol of the a-centered structure for some of the base-centered orthorhombic lattices (spacegroups 38 to 41. Notice that mp-773965 has spacegroup 40). See https://arxiv.org/pdf/1506.01455.pdf section 3 for more details. The init in HighSymmKpath, that checks the space group symbol expecting a c-centered orthorhombic symbol, fails to recognize the space group.

It should also be noted that, since the same kind of check is used in the SpaceGroupAnalyzer (i.e. if "C" in self.get_space_group_symbol()), also the primitive and conventional standard structures can have some problems in these cases.

hautierg commented 7 years ago

Hi Guido,

Thanks for the follow-up. Maybe we should at least return an error or warning then?

Could you implement this?

Thanks!

Geoffroy

On 08 Mar 2017, at 12:22, Guido Petretto notifications@github.com wrote:

Hi, I want to point out that this is related to the issue #374 https://github.com/materialsproject/pymatgen/issues/374. The problem originates from the fact that spglib at some point started to use and return the symbol of the a-centered structure for some of the base-centered orthorhombic lattices (spacegroups 38 to 41. Notice that mp-773965 has spacegroup 40). See https://arxiv.org/pdf/1506.01455.pdf https://arxiv.org/pdf/1506.01455.pdf section 3 for more details. The init in HighSymmKpath, that checks the space group symbol expecting a c-centered orthorhombic symbol, fails to recognize the space group.

It should also be noted that, since the same kind of check is used in the SpaceGroupAnalyzer (i.e. if "C" in self.get_space_group_symbol()), also the primitive and conventional standard structures can have some problems in these cases.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/materialsproject/pymatgen/issues/609#issuecomment-285016103, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3l92L-DB_8p-9U2IDhknj_5mb0tI6Gks5rjo-IgaJpZM4MV4wA.

gpetretto commented 7 years ago

Hi Geoffroy,

actually the warning is already present in the HighSymmKpath, when it cannot determine the path. When running the example I get the following warning:

warnings.warn("The input structure does not match the expected standard primitive! " pymatgen/pymatgen/symmetry/bandstructure.py:112: UserWarning: Unexpected value for spg_symbol: Ama2

On the other hand, adding a warning or an error on the SpacegroupAnalyzer would require first to check explicitly in which cases the problem will show up. It should be for the spacegroups that I mentioned, but I didn't do any extensive test.

I believe that the a proper solution of the problem will require the analysis of the output produced by spglib in order to update the SpaceggroupAnalyzer and the HighSymmKpath objects. This is probably not so straightforward though.

shyuep commented 7 years ago

In my opinion, the way to determine the path shouldn't depend on the crystal being in a specific setting - C or B or A. I would very much prefer a robust solution that will handle all crystal structures properly based on the symmetry operations that exist, rather than to have special case handling for A setting, C setting, B setting....

hjjvandam commented 6 years ago

I have the same problem with space group 169 for FePS3 (ICSD 633085).

shyuep commented 1 year ago

Seems like this issue has been defunct for many years. Closing.... Feel free to reopen if still valid.