Add magmom support in get_phonopy_structure (#3555)

* Add magmom support in get_phonopy_structure

* add test for struct_ph.magnetic_moments

* fix TypeError: 'NoneType' object is not subscriptable

prop = {key: val[idx] for key, val in site_properties.items()}

* fix StructureError stating wrong DISTANCE_TOLERANCE

---------

Co-authored-by: Janosh Riebesell <[email protected]>

Author: tomdemeyere

pymatgen/core/structure.py

@@ -927,7 +927,7 @@ def __init__(
         for idx, specie in enumerate(species):
             prop = None
             if site_properties:
-                prop = {key: val[idx] for key, val in site_properties.items()}
+                prop = {key: val[idx] for key, val in site_properties.items() if val is not None}
 
             label = labels[idx] if labels else None
 
@@ -943,7 +943,7 @@ def __init__(
             sites.append(site)
         self._sites: tuple[PeriodicSite, ...] = tuple(sites)
         if validate_proximity and not self.is_valid():
-            raise StructureError("Structure contains sites that are less than 0.01 Angstrom apart!")
+            raise StructureError(f"sites are less than {self.DISTANCE_TOLERANCE} Angstrom apart!")
         self._charge = charge
         self._properties = properties or {}
 

pymatgen/io/phonopy.py

@@ -34,13 +34,13 @@ def get_pmg_structure(phonopy_structure: PhonopyAtoms) -> Structure:
     frac_coords = phonopy_structure.scaled_positions
     symbols = phonopy_structure.symbols
     masses = phonopy_structure.masses
-    mms = getattr(phonopy_structure, "magnetic_moments", None) or [0] * len(symbols)
+    magmoms = getattr(phonopy_structure, "magnetic_moments", [0] * len(symbols))
 
     return Structure(
         lattice,
         symbols,
         frac_coords,
-        site_properties={"phonopy_masses": masses, "magnetic_moments": mms},
+        site_properties={"phonopy_masses": masses, "magnetic_moments": magmoms},
     )
 
 
@@ -58,6 +58,7 @@ def get_phonopy_structure(pmg_structure: Structure) -> PhonopyAtoms:
         symbols=symbols,
         cell=pmg_structure.lattice.matrix,
         scaled_positions=pmg_structure.frac_coords,
+        magnetic_moments=pmg_structure.site_properties.get("magmom"),
     )
 
 

tests/core/test_structure.py

@@ -73,7 +73,9 @@ def setUp(self):
         assert self.struct.is_ordered
         assert self.struct.ntypesp == 1
         coords = [[0, 0, 0], [0.0, 0, 0.0000001]]
-        with pytest.raises(StructureError, match="Structure contains sites that are less than 0.01 Angstrom apart"):
+        with pytest.raises(
+            StructureError, match=f"sites are less than {self.struct.DISTANCE_TOLERANCE} Angstrom apart"
+        ):
             IStructure(self.lattice, ["Si"] * 2, coords, validate_proximity=True)
         self.propertied_structure = IStructure(
             self.lattice, ["Si"] * 2, coords, site_properties={"magmom": [5, -5]}, properties={"test_property": "test"}
@@ -104,7 +106,7 @@ def test_get_orderings(self):
 
     def test_as_dataframe(self):
         df = self.propertied_structure.as_dataframe()
-        assert df.attrs["Reduced Formula"] == "Si"
+        assert df.attrs["Reduced Formula"] == self.propertied_structure.composition.reduced_formula
         assert df.shape == (2, 8)
 
     def test_equal(self):
@@ -130,7 +132,7 @@ def test_matches(self):
 
     def test_bad_structure(self):
         coords = [[0, 0, 0], [0.75, 0.5, 0.75], [0.75, 0.5, 0.75]]
-        with pytest.raises(StructureError, match="Structure contains sites that are less than 0.01 Angstrom apart"):
+        with pytest.raises(StructureError, match=f"sites are less than {Structure.DISTANCE_TOLERANCE} Angstrom apart"):
             IStructure(self.lattice, ["Si"] * 3, coords, validate_proximity=True)
         # these shouldn't raise an error
         IStructure(self.lattice, ["Si"] * 2, coords[:2], validate_proximity=True)

tests/io/test_ase.py

@@ -147,7 +147,7 @@ def test_get_structure():
     atoms = read(f"{TEST_FILES_DIR}/POSCAR_overlap")
     struct = AseAtomsAdaptor.get_structure(atoms)
     assert [s.species_string for s in struct] == atoms.get_chemical_symbols()
-    with pytest.raises(StructureError, match="Structure contains sites that are less than 0.01 Angstrom apart"):
+    with pytest.raises(StructureError, match=f"sites are less than {struct.DISTANCE_TOLERANCE} Angstrom apart"):
         struct = AseAtomsAdaptor.get_structure(atoms, validate_proximity=True)
 
 

tests/io/test_phonopy.py

@@ -98,22 +98,28 @@ def test_get_complete_dos(self):
 @unittest.skipIf(Phonopy is None, "Phonopy not present")
 class TestStructureConversion(PymatgenTest):
     def test_structure_conversion(self):
-        s_pmg = PymatgenTest.get_structure("LiFePO4")
-        s_ph = get_phonopy_structure(s_pmg)
-        s_pmg2 = get_pmg_structure(s_ph)
-
-        coords_ph = s_ph.get_scaled_positions()
-        symbols_pmg = {e.symbol for e in s_pmg.composition}
-        symbols_pmg2 = {e.symbol for e in s_pmg2.composition}
-
-        assert s_ph.get_cell()[1, 1] == approx(s_pmg.lattice._matrix[1, 1], abs=1e-7)
-        assert s_pmg.lattice._matrix[1, 1] == approx(s_pmg2.lattice._matrix[1, 1], abs=1e-7)
-        assert symbols_pmg == set(s_ph.symbols)
+        struct_pmg = PymatgenTest.get_structure("LiFePO4")
+        # add magmoms to site_properties
+        struct_pmg.add_site_property("magmom", magmoms := [1] * len(struct_pmg))
+        struct_ph = get_phonopy_structure(struct_pmg)
+        struct_pmg_round_trip = get_pmg_structure(struct_ph)
+        assert struct_pmg_round_trip.matches(struct_pmg)
+
+        coords_ph = struct_ph.get_scaled_positions()
+        symbols_pmg = {e.symbol for e in struct_pmg.composition}
+        symbols_pmg2 = {e.symbol for e in struct_pmg_round_trip.composition}
+
+        assert struct_ph.get_cell()[1, 1] == approx(struct_pmg.lattice._matrix[1, 1], abs=1e-7)
+        assert struct_pmg.lattice._matrix[1, 1] == approx(struct_pmg_round_trip.lattice._matrix[1, 1], abs=1e-7)
+        assert symbols_pmg == set(struct_ph.symbols)
         assert symbols_pmg == symbols_pmg2
-        assert_allclose(coords_ph[3], s_pmg.frac_coords[3])
-        assert_allclose(s_pmg.frac_coords[3], s_pmg2.frac_coords[3])
-        assert s_ph.get_number_of_atoms() == len(s_pmg)
-        assert len(s_pmg) == len(s_pmg2)
+        assert_allclose(coords_ph[3], struct_pmg.frac_coords[3])
+        assert_allclose(struct_pmg.frac_coords[3], struct_pmg_round_trip.frac_coords[3])
+        assert struct_ph.get_number_of_atoms() == len(struct_pmg)
+        assert len(struct_pmg) == len(struct_pmg_round_trip)
+
+        # https://github.com/materialsproject/pymatgen/pull/3555
+        assert list(struct_ph.magnetic_moments) == magmoms
 
 
 @unittest.skipIf(Phonopy is None, "Phonopy not present")