Follow-up on #304, which introduced these quantities. Belongs under #219.
In schema_packages/model_system.py, the description of site_multiplicities (L 472-488) says the value is the multiplicity in the conventional cell and that it "remains the same" in primitive cells and supercells, being an intrinsic property of the Wyckoff position.
_compute_site_multiplicities (lines 858-893) instead counts, per atom, how many atoms share its equivalent_atoms index in whatever cell it was handed. Its own docstring (865-866) states this, so the disagreement is internal to the file.
Call sites diverge:
- Line 967 (
resolve_analyzed_cell) is shared by both branches. The conventional branch gets get_equivalent_atoms_conventional() and therefore matches the description; the primitive branch does not.
- Line 1064 (
resolve_bulk_symmetry) uses get_equivalent_atoms_original(), so ModelSystem.local_symmetry reflects whatever cell the parser produced.
Observed
fcc Cu, space group 225, single site 4a, three input cells:
| input cell |
original |
primitive |
conventional |
| primitive (1 atom) |
1 |
1 |
4 |
| conventional (4 atoms) |
4 |
1 |
4 |
| 2x2x2 supercell (32 atoms) |
32 |
1 |
4 |
Only the conventional column is invariant under the choice of input cell. The supercell reports 32, which is not a Wyckoff multiplicity of any position in space group 225.
Reproducer (spglib 2.7.0, matid 2.2.0, ASE 3.29.0)
from ase.build import bulk
from matid.symmetry import SymmetryAnalyzer
def site_multiplicities(equivalent_atoms): # current implementation
equiv = list(equivalent_atoms)
return [equiv.count(equiv[i]) for i in range(len(equiv))]
for label, atoms in [('primitive', bulk('Cu', 'fcc', a=3.6)),
('conventional', bulk('Cu', 'fcc', a=3.6, cubic=True)),
('2x2x2', bulk('Cu', 'fcc', a=3.6, cubic=True) * (2, 2, 2))]:
sa = SymmetryAnalyzer(atoms, symmetry_tol=0.1)
print(label, site_multiplicities(sa.get_equivalent_atoms_original())[0])
# primitive 1 / conventional 4 / 2x2x2 32
Question
Which is intended?
Intrinsic (what the description promises): no extra spglib call needed. Multiplicity is fixed per Wyckoff letter within a space group, independent of the free parameters of a parametric position, so the map can be built once from the conventional cell:
conv_letters = [str(x) for x in sa.get_wyckoff_letters_conventional()]
conv_equiv = list(sa.get_equivalent_atoms_conventional())
mult_by_letter = {l: conv_equiv.count(e) for l, e in zip(conv_letters, conv_equiv)}
site_mults = [mult_by_letter[l] for l in letters_of_this_cell]
This gives 4a for both the primitive Cu cell and the supercell, 4a/4b for NaCl primitive, 2c for hcp Ti.
Per-cell orbit size (what the code computes): the description needs rewriting, and the quantity arguably wants a name that does not say "multiplicity", since equivalent_atoms already encodes the grouping.
Impact
test_compute_site_multiplicities (tests/test_model_system.py 1350-1375) uses synthetic equivalent_atoms arrays only, so it passes under either reading and does not pin the behaviour.
One dependency related minor question: if #241 (PyXtal for symmetry analysis) is on the near roadmap, the intrinsic multiplicity would come from PyXtal's Wyckoff tables rather than from a second pass over the conventional cell, so that decision may want to come first
Follow-up on #304, which introduced these quantities. Belongs under #219.
In
schema_packages/model_system.py, the description ofsite_multiplicities(L 472-488) says the value is the multiplicity in the conventional cell and that it "remains the same" in primitive cells and supercells, being an intrinsic property of the Wyckoff position._compute_site_multiplicities(lines 858-893) instead counts, per atom, how many atoms share itsequivalent_atomsindex in whatever cell it was handed. Its own docstring (865-866) states this, so the disagreement is internal to the file.Call sites diverge:
resolve_analyzed_cell) is shared by both branches. Theconventionalbranch getsget_equivalent_atoms_conventional()and therefore matches the description; theprimitivebranch does not.resolve_bulk_symmetry) usesget_equivalent_atoms_original(), soModelSystem.local_symmetryreflects whatever cell the parser produced.Observed
fcc Cu, space group 225, single site
4a, three input cells:originalprimitiveconventionalOnly the
conventionalcolumn is invariant under the choice of input cell. The supercell reports 32, which is not a Wyckoff multiplicity of any position in space group 225.Reproducer (spglib 2.7.0, matid 2.2.0, ASE 3.29.0)
Question
Which is intended?
Intrinsic (what the description promises): no extra spglib call needed. Multiplicity is fixed per Wyckoff letter within a space group, independent of the free parameters of a parametric position, so the map can be built once from the conventional cell:
This gives
4afor both the primitive Cu cell and the supercell,4a/4bfor NaCl primitive,2cfor hcp Ti.Per-cell orbit size (what the code computes): the description needs rewriting, and the quantity arguably wants a name that does not say "multiplicity", since
equivalent_atomsalready encodes the grouping.Impact
test_compute_site_multiplicities(tests/test_model_system.py1350-1375) uses syntheticequivalent_atomsarrays only, so it passes under either reading and does not pin the behaviour.One dependency related minor question: if #241 (PyXtal for symmetry analysis) is on the near roadmap, the intrinsic multiplicity would come from PyXtal's Wyckoff tables rather than from a second pass over the conventional cell, so that decision may want to come first