Fix import from Castep lattice_abc format

CHANGELOG.rst

@@ -4,7 +4,16 @@ Change Log
 Unreleased
 ----------
 
-* Minor docstring updates (@kavanase, https://github.com/SMTG-Bham/sumo/pull/241)
+Documentation:
+
+- Minor docstring updates (@kavanase, https://github.com/SMTG-Bham/sumo/pull/241)
+
+Bugfixes:
+
+- Castep lattice_abc format was broken in many cases; deal with a
+  Pymatgen deprecation and avoid incorrect truncation of data when no
+  units are provided. Where structure is ambiguous due to mixed
+  lattice/cartesian conventions, raise an error. (@jryates, @ajjackson)
 
 v2.3.8
 ------

sumo/io/castep.py

@@ -30,6 +30,8 @@
 
 unsupported_dosplot_args = {"elements", "lm_orbitals", "atoms"}
 
+logger = logging.getLogger("io.castep")
+
 
 class CastepCell:
     """Structure information and more: CASTEP seedname.cell file
@@ -76,14 +78,16 @@ def structure(self):
                 lengths_and_angles = lattice_abc[1:]
             else:
                 unit = "ang"
-                lengths_and_angles = lattice_abc[1:]
+                lengths_and_angles = lattice_abc
             if len(lengths_and_angles) != 2:
                 raise ValueError("lattice_abc should have two rows")
             lengths_and_angles = [list(map(float, row)) for row in lengths_and_angles]
             lengths_and_angles[0] = [
                 x * to_angstrom[unit] for x in lengths_and_angles[0]
             ]
-            lattice = Lattice.from_lengths_and_angles(*lengths_and_angles)
+            lattice = Lattice.from_parameters(
+                *lengths_and_angles[0], *lengths_and_angles[1]
+            )
         else:
             raise ValueError("Couldn't find a lattice in cell file")
 
@@ -95,6 +99,14 @@ def structure(self):
             elements, coords = zip(*elements_coords)
             return Structure(lattice, elements, coords, coords_are_cartesian=False)
         elif "positions_abs" in self.blocks:
+            if "lattice_abc" in self.blocks:
+                raise ValueError(
+                    "Positions are given in Cartesian coordinates and lattice "
+                    "in angles/lengths format. This structure cannot be "
+                    "interpreted reliably. Consider using c2x to convert to "
+                    "fractional coordinates and/or 3x3 lattice matrix."
+                )
+
             positions_abs = self.blocks["positions_abs"].values
             if positions_abs[0][0].lower() in ("ang", "nm", "cm", "m", "bohr", "a0"):
                 unit = positions_abs[0][0].lower()

tests/data/NiO/NiO_abc.cell

@@ -0,0 +1,20 @@
+%BLOCK LATTICE_ABC
+2.983077  2.98308198  5.15994087
+106.80215981  73.19772637 119.99807072
+%ENDBLOCK LATTICE_ABC
+
+%BLOCK POSITIONS_FRAC
+Ni  0.000000  0.000000  0.000000 SPIN=1
+Ni  0.500000  0.500000  0.500000 SPIN=-1
+O  0.250001  0.749999  0.249993  SPIN=0
+O  0.749999  0.250001  0.750007  SPIN=0
+%ENDBLOCK POSITIONS_FRAC
+
+%BLOCK SPECIES_POT
+Ni C19
+O  C19
+%ENDBLOCK SPECIES_POT
+
+KPOINTS_MP_GRID 7 7 7
+SPECTRAL_KPOINTS_MP_GRID 2 2 2
+

tests/data/NiO/NiO_abc_cart.cell

@@ -0,0 +1,20 @@
+%BLOCK LATTICE_ABC
+2.983077  2.98308198  5.15994087
+106.80215981  73.19772637 119.99807072
+%ENDBLOCK LATTICE_ABC
+
+%BLOCK POSITIONS_ABS
+Ni  0.000000  0.000000  0.000000 SPIN=1
+Ni  1.491603  0.861143  2.432002 SPIN=-1
+O  0.000069  1.722313  1.215967  SPIN=0
+O  2.983138 -0.000026  3.648036  SPIN=0 
+%ENDBLOCK POSITIONS_ABS
+
+%BLOCK SPECIES_POT
+Ni C19
+O  C19
+%ENDBLOCK SPECIES_POT
+
+KPOINTS_MP_GRID 7 7 7
+SPECTRAL_KPOINTS_MP_GRID 2 2 2
+

tests/data/NiO/NiO_abc_frac.cell

@@ -0,0 +1,22 @@
+# written by ASE
+
+%BLOCK LATTICE_CART
+ 2.983077  0.000000  0.000000
+-1.491454  2.583475  0.000000
+ 1.491583 -0.861188  4.864003
+%ENDBLOCK LATTICE_CART
+
+%BLOCK POSITIONS_FRAC
+Ni  0.000000  0.000000  0.000000
+Ni  0.500000  0.500000  0.500000
+O  0.250001  0.749999  0.249993
+O  0.749999  0.250001  0.750007
+%ENDBLOCK POSITIONS_FRAC
+
+%BLOCK SPECIES_POT
+Ni C19
+O C19
+%ENDBLOCK SPECIES_POT
+
+KPOINTS_MP_GRID: 7 7 7
+SPECTRAL_KPOINTS_MP_GRID: 2 2 2

tests/data/NiO/NiO_abc_units.cell

@@ -0,0 +1,20 @@
+%BLOCK LATTICE_ABC
+Ang
+2.983077  2.98308198  5.15994087
+106.80215981  73.19772637 119.99807072
+%ENDBLOCK LATTICE_ABC
+
+%BLOCK POSITIONS_FRAC
+Ni  0.000000  0.000000  0.000000 SPIN=1
+Ni  0.500000  0.500000  0.500000 SPIN=-1
+O  0.250001  0.749999  0.249993  SPIN=0
+O  0.749999  0.250001  0.750007  SPIN=0
+%ENDBLOCK POSITIONS_FRAC
+
+%BLOCK SPECIES_POT
+Ni C19
+O  C19
+%ENDBLOCK SPECIES_POT
+
+KPOINTS_MP_GRID 7 7 7
+SPECTRAL_KPOINTS_MP_GRID 2 2 2

tests/tests_io/test_castep.py

@@ -17,6 +17,7 @@
     CastepCell,
     CastepPhonon,
     labels_from_cell,
+    logger,
     read_bands_eigenvalues,
     read_bands_header,
     read_dos,
@@ -31,12 +32,25 @@ def setUp(self):
         self.si_cell_alt = os.path.join(
             ilr_files("tests"), "data", "Si", "Si2-alt.cell"
         )
+        si_structure_file = os.path.join(ilr_files("tests"), "data", "Si", "Si8.json")
+        self.si_structure = Structure.from_file(si_structure_file)
         self.zns_band_cell = os.path.join(ilr_files("tests"), "data", "ZnS", "zns.cell")
         self.zns_singlepoint_cell = os.path.join(
             ilr_files("tests"), "data", "ZnS", "zns-sp.cell"
         )
-        si_structure_file = os.path.join(ilr_files("tests"), "data", "Si", "Si8.json")
-        self.si_structure = Structure.from_file(si_structure_file)
+        self.nio_cart_cell = os.path.join(
+            ilr_files("tests"), "data", "NiO", "NiO.cell"
+        )
+        self.nio_abc_cell = os.path.join(
+            ilr_files("tests"), "data", "NiO", "NiO_abc.cell"
+        )
+        self.nio_abc_units_cell = os.path.join(
+            ilr_files("tests"), "data", "NiO", "NiO_abc_units.cell"
+        )
+        self.nio_abc_cart_cell = os.path.join(
+            ilr_files("tests"), "data", "NiO", "NiO_abc_cart.cell"
+        )
+
 
     def test_castep_cell_null_init(self):
         null_cell = CastepCell()
@@ -72,6 +86,36 @@ def test_castep_cell_from_singlepoint_file(self):
             [[0.0, 2.71, 2.71], [2.71, 0.0, 2.71], [2.71, 2.71, 0.0]],
         )
 
+    def test_castep_cell_abc(self):
+        """Test .cell file using lattice_abc to define unit cell"""
+        for filename in self.nio_abc_cell, self.nio_abc_units_cell:
+            cc = CastepCell.from_file(filename)
+            structure = cc.structure
+
+            assert_array_almost_equal(
+                structure.lattice.abc,
+                [2.983077, 2.98308198, 5.15994087]
+            )
+            assert_array_almost_equal(
+                structure.lattice.angles,
+                [106.80215981, 73.19772637, 119.99807072]
+            )
+
+    def test_castep_cell_consistent_frac_coords(self):
+        """Test fractional positions are consistent between .cell formats"""
+        cartesian_cell = CastepCell.from_file(self.nio_cart_cell)
+        frac_abc_cell = CastepCell.from_file(self.nio_abc_cell)
+
+        assert_array_almost_equal(
+            cartesian_cell.structure.frac_coords,
+            frac_abc_cell.structure.frac_coords
+        )
+
+    def test_castep_abc_cart_raises(self):
+        """Test that error is raised for incompatible vectors and positions"""
+        with self.assertRaisesRegex(ValueError, "Cartesian"):
+            CastepCell.from_file(self.nio_abc_cart_cell).structure
+
     def test_castep_cell_from_structure(self):
         cell = CastepCell.from_structure(self.si_structure)
         self.assertEqual(