From e2a48db2b72527c8af2bf87dbef26ac3b3362e69 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Wed, 6 Nov 2024 12:31:41 +0000 Subject: [PATCH] Raise an error when faced with ambiguous CASTEP .cell CASTEP and Pymatgen orientation are likely to be different from the same lattice vectors. To avoid building incorrect structures, refuse to work with lattice_abc and positions_abs simultaneously. --- sumo/io/castep.py | 8 ++++--- tests/data/NiO/NiO_abc.cell | 10 ++++---- tests/data/NiO/NiO_abc_units.cell | 12 ++++------ tests/tests_io/test_castep.py | 40 +++++++++++++++++++++++-------- 4 files changed, 45 insertions(+), 25 deletions(-) diff --git a/sumo/io/castep.py b/sumo/io/castep.py index b297e7d5..c8416349 100644 --- a/sumo/io/castep.py +++ b/sumo/io/castep.py @@ -100,9 +100,11 @@ def structure(self): return Structure(lattice, elements, coords, coords_are_cartesian=False) elif "positions_abs" in self.blocks: if "lattice_abc" in self.blocks: - logger.warning( - "Positions are given in absolute coordinates and lattice " - "in angles/lengths format. Structure may not be consistent." + 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 diff --git a/tests/data/NiO/NiO_abc.cell b/tests/data/NiO/NiO_abc.cell index 46a84eee..e63cb4bc 100644 --- a/tests/data/NiO/NiO_abc.cell +++ b/tests/data/NiO/NiO_abc.cell @@ -3,12 +3,12 @@ 106.80215981 73.19772637 119.99807072 %ENDBLOCK LATTICE_ABC -%BLOCK POSITIONS_ABS +%BLOCK POSITIONS_FRAC 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 +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 diff --git a/tests/data/NiO/NiO_abc_units.cell b/tests/data/NiO/NiO_abc_units.cell index d89d422d..bfa79556 100644 --- a/tests/data/NiO/NiO_abc_units.cell +++ b/tests/data/NiO/NiO_abc_units.cell @@ -4,13 +4,12 @@ Ang 106.80215981 73.19772637 119.99807072 %ENDBLOCK LATTICE_ABC -%BLOCK POSITIONS_ABS -Ang +%BLOCK POSITIONS_FRAC 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 +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 @@ -19,4 +18,3 @@ O C19 KPOINTS_MP_GRID 7 7 7 SPECTRAL_KPOINTS_MP_GRID 2 2 2 - diff --git a/tests/tests_io/test_castep.py b/tests/tests_io/test_castep.py index 70404b4c..9f3e78e3 100644 --- a/tests/tests_io/test_castep.py +++ b/tests/tests_io/test_castep.py @@ -38,12 +38,19 @@ def setUp(self): self.zns_singlepoint_cell = os.path.join( ilr_files("tests"), "data", "ZnS", "zns-sp.cell" ) + 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() @@ -82,20 +89,33 @@ def test_castep_cell_from_singlepoint_file(self): 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: - with self.assertLogs(logger) as log: - cc = CastepCell.from_file(filename) - structure = cc.structure - self.assertIn("Structure may not be consistent.", log.output[-1]) + cc = CastepCell.from_file(filename) + structure = cc.structure assert_array_almost_equal( - structure.lattice.matrix, - [ - [2.855724, 0.0, 0.862317], - [-1.297582, 2.54391, -0.862313], - [0.0, 0.0, 5.159941], - ], + 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(