pymatgen icon indicating copy to clipboard operation
pymatgen copied to clipboard

Site property indexing in `SymmetrizedStructure`

Open kaueltzen opened this issue 1 year ago • 1 comments

Python version

3.10

Pymatgen version

master or 2024.7.18

Operating system version

Debian GNU/Linux 11

Current behavior

The current implementation of __str__ of SymmetrizedStructure appends site properties as per the index of self.equivalent_sites, not of the actual site index. https://github.com/materialsproject/pymatgen/blob/660ba7a584fae5b5d8bb1095614f0104c9d89049/src/pymatgen/symmetry/structure.py#L120

This means that a structure with the species string as a test property

Full Formula (La4 Mn4 O12)
Reduced Formula: LaMnO3
abc   :   5.746100   7.663700   5.533300
angles:  90.000000  90.000000  90.000000
pbc   :       True       True       True
Sites (20)
  #  SP          a       b       c  test_property
---  ----  -------  ------  ------  ---------------
  0  La    0.0513   0.25    0.9905  La
  1  La    0.9487   0.75    0.0095  La
  2  La    0.5513   0.25    0.5095  La
  3  La    0.4487   0.75    0.4905  La
  4  Mn    0        0       0.5     Mn
  5  Mn    0        0.5     0.5     Mn
  6  Mn    0.5      0.5     0       Mn
  7  Mn    0.5      0       0       Mn
  8  O     0.48493  0.25    0.0777  O
  9  O     0.51507  0.75    0.9223  O
 10  O     0.98493  0.25    0.4223  O
 11  O     0.01507  0.75    0.5777  O
 12  O     0.3085   0.0408  0.7227  O
 13  O     0.6915   0.5408  0.2773  O
 14  O     0.6915   0.9592  0.2773  O
 15  O     0.3085   0.4592  0.7227  O
 16  O     0.8085   0.4592  0.7773  O
 17  O     0.1915   0.9592  0.2227  O
 18  O     0.1915   0.5408  0.2227  O
 19  O     0.8085   0.0408  0.7773  O

will give the following SymmetrizedStructure string:

SymmetrizedStructure
Full Formula (La4 Mn4 O12)
Reduced Formula: LaMnO3
Spacegroup: Pnma (62)
abc   :   5.746100   7.663700   5.533300
angles:  90.000000  90.000000  90.000000
Sites (20)
  #  SP          a       b       c  Wyckoff    test_property
---  ----  -------  ------  ------  ---------  ---------------
  0  La    0.0513   0.25    0.9905  4c         La
  1  Mn    0        0       0.5     4b         La
  2  O     0.48493  0.25    0.0777  4c         La
  3  O     0.3085   0.0408  0.7227  8d         La

This can be corrected by replacing the respective line with row.append(site.properties[key]) and I can raise a PR with that.

However, the whole approach does not ensure that the site property has the same symmetry as the site itself as only the atomic coordinates and numbers are mapped onto each other. Therefore, it may not make a lot of sense to give site properties here at all or at least a warning should be raised that the symmetry of the property was not taken into account in the symmetry search.

Expected Behavior

A symmetrized structure like

SymmetrizedStructure
Full Formula (La4 Mn4 O12)
Reduced Formula: LaMnO3
Spacegroup: Pnma (62)
abc   :   5.746100   7.663700   5.533300
angles:  90.000000  90.000000  90.000000
Sites (20)
  #  SP          a       b       c  Wyckoff    test_property
---  ----  -------  ------  ------  ---------  ---------------
  0  La    0.0513   0.25    0.9905  4c         La
  1  Mn    0        0       0.5     4b         Mn
  2  O     0.48493  0.25    0.0777  4c         O
  3  O     0.3085   0.0408  0.7227  8d         O

or preferably without site properties (see above).

Minimal example

No response

Relevant files to reproduce this bug

No response

kaueltzen avatar Aug 01 '24 09:08 kaueltzen

excellent catch!

Therefore, it may not make a lot of sense to give site properties here at all or at least a warning

thanks for pointing that out. i'm leaning towards not trying to translate site properties from Structure to SymmetrizedStructure at all in order to not mislead users. a warning might be missed and doesn't sound like a great user experience anyway. but i defer to @mkhorton if he wants to weigh in

janosh avatar Aug 01 '24 11:08 janosh