Merge pull request #184 from Exabyte-io/feature/SOF-7524

Feature/SOF-7524: remove "cell" from `basis.to_json()`

src/py/mat3ra/made/basis.py

@@ -41,15 +41,14 @@ def to_json(self, skip_rounding=False):
             "elements": self.elements.to_json(),
             "coordinates": self.coordinates.to_json(skip_rounding=skip_rounding),
             "units": self.units,
-            "cell": self.cell.to_json(skip_rounding=skip_rounding) if self.cell else None,
             "labels": self.labels.to_json(),
         }
         return json.loads(json.dumps(json_value))
 
     def clone(self):
         return Basis(
-            elements=self.toJSON()["elements"],
-            coordinates=self.toJSON()["coordinates"],
+            elements=self.elements,
+            coordinates=self.coordinates,
             units=self.units,
             cell=self.cell,
             isEmpty=False,

src/py/mat3ra/made/material.py

@@ -67,7 +67,9 @@ def coordinates_array(self) -> List[List[float]]:
 
     @property
     def basis(self) -> Basis:
-        return Basis.from_dict(**self.get_prop("basis"))
+        config = self.get_prop("basis")
+        config["cell"] = config.get("cell", self.lattice.vector_arrays)
+        return Basis.from_dict(**config)
 
     @basis.setter
     def basis(self, basis: Basis) -> None:
@@ -99,15 +101,12 @@ def set_coordinates(self, coordinates: List[List[float]]) -> None:
     def set_new_lattice_vectors(
         self, lattice_vector1: List[float], lattice_vector2: List[float], lattice_vector3: List[float]
     ) -> None:
-        new_basis = self.basis.copy()
-        new_basis.to_cartesian()
-        new_basis.cell.vector1 = lattice_vector1
-        new_basis.cell.vector2 = lattice_vector2
-        new_basis.cell.vector3 = lattice_vector3
-        new_basis.to_crystal()
-        self.basis = new_basis
         lattice = Lattice.from_vectors_array([lattice_vector1, lattice_vector2, lattice_vector3])
+        original_is_in_crystal = self.basis.is_in_crystal_units
+        self.to_cartesian()
         self.lattice = lattice
+        if original_is_in_crystal:
+            self.to_crystal()
 
     def add_atom(self, element: str, coordinate: List[float], use_cartesian_coordinates=False) -> None:
         new_basis = self.basis.copy()

src/py/mat3ra/made/tools/build/nanoribbon/builders.py

@@ -7,7 +7,7 @@
 from mat3ra.made.tools.build.supercell import create_supercell
 from mat3ra.made.tools.modify import filter_by_rectangle_projection, wrap_to_unit_cell
 
-from ...modify import translate_to_center
+from ...modify import translate_to_center, rotate
 from .configuration import NanoribbonConfiguration
 from .enums import EdgeTypes
 
@@ -73,11 +73,11 @@ def _calculate_cartesian_dimensions(config: NanoribbonConfiguration, material: M
             nanoribbon_length, nanoribbon_width = nanoribbon_width, nanoribbon_length
             vacuum_width, vacuum_length = vacuum_length, vacuum_width
 
-        length_cartesian = nanoribbon_length * np.dot(np.array(material.basis.cell.vector1), np.array([1, 0, 0]))
-        width_cartesian = nanoribbon_width * np.dot(np.array(material.basis.cell.vector2), np.array([0, 1, 0]))
-        height_cartesian = np.dot(np.array(material.basis.cell.vector3), np.array([0, 0, 1]))
-        vacuum_length_cartesian = vacuum_length * np.dot(np.array(material.basis.cell.vector1), np.array([1, 0, 0]))
-        vacuum_width_cartesian = vacuum_width * np.dot(np.array(material.basis.cell.vector2), np.array([0, 1, 0]))
+        length_cartesian = nanoribbon_length * np.dot(np.array(material.lattice.vectors[0]), np.array([1, 0, 0]))
+        width_cartesian = nanoribbon_width * np.dot(np.array(material.lattice.vectors[1]), np.array([0, 1, 0]))
+        height_cartesian = np.dot(np.array(material.lattice.vectors[2]), np.array([0, 0, 1]))
+        vacuum_length_cartesian = vacuum_length * np.dot(np.array(material.lattice.vectors[0]), np.array([1, 0, 0]))
+        vacuum_width_cartesian = vacuum_width * np.dot(np.array(material.lattice.vectors[1]), np.array([0, 1, 0]))
 
         return length_cartesian, width_cartesian, height_cartesian, vacuum_length_cartesian, vacuum_width_cartesian
 
@@ -134,6 +134,8 @@ def _calculate_coordinates_of_cut(
 
     def _generate(self, configuration: NanoribbonConfiguration) -> List[_GeneratedItemType]:
         nanoribbon = self.create_nanoribbon(configuration)
+        if configuration.edge_type == EdgeTypes.armchair:
+            nanoribbon = rotate(nanoribbon, [0, 0, 1], 90)
         return [nanoribbon]
 
     def _post_process(

src/py/mat3ra/made/tools/build/perturbation/builders.py

@@ -65,7 +65,7 @@ def create_perturbed_slab(self, configuration: PerturbationConfiguration) -> Mat
 
 class CellMatchingDistancePreservingSlabPerturbationBuilder(DistancePreservingSlabPerturbationBuilder):
     def _transform_lattice_vectors(self, configuration: PerturbationConfiguration) -> List[List[float]]:
-        cell_vectors = configuration.material.basis.cell.vectors_as_array
+        cell_vectors = configuration.material.lattice.vectors
         return [configuration.perturbation_function_holder.transform_coordinates(coord) for coord in cell_vectors]
 
     def create_perturbed_slab(self, configuration: PerturbationConfiguration) -> Material:
@@ -74,10 +74,4 @@ def create_perturbed_slab(self, configuration: PerturbationConfiguration) -> Mat
         new_lattice = new_material.lattice.copy()
         new_lattice = new_lattice.from_vectors_array(new_lattice_vectors)
         new_material.lattice = new_lattice
-
-        new_basis = new_material.basis.copy()
-        new_basis.to_cartesian()
-        new_basis.cell = new_basis.cell.from_vectors_array(new_lattice_vectors)
-        new_basis.to_crystal()
-        new_material.basis = new_basis
         return new_material

src/py/mat3ra/made/tools/convert/__init__.py

@@ -85,7 +85,8 @@ def from_pymatgen(structure: Union[PymatgenStructure, PymatgenInterface]) -> Dic
             {"id": i, "value": __round__(list(site.frac_coords))} for i, site in enumerate(structure.sites)
         ],
         "units": "crystal",
-        "cell": __round__(structure.lattice.matrix.tolist()),
+        # `cell` is assigned by the `lattice` object during Material initialization
+        # "cell": __round__(structure.lattice.matrix.tolist()),
         "constraints": [],
     }
 

tests/py/unit/fixtures.py

@@ -1,4 +1,5 @@
 import copy
+from functools import reduce
 from typing import Any, Dict
 
 from ase.build import bulk
@@ -56,7 +57,6 @@
     "mean_abs_strain": 0.00105,
 }
 
-
 # Add properties to interface structure
 INTERFACE_STRUCTURE.interface_properties = INTERFACE_PROPERTIES_MOCK
 INTERFACE_NAME = "Cu4(001)-Si8(001), Interface, Strain 0.062pct"
@@ -86,7 +86,6 @@
             {"id": 7, "value": [0.75, 0.75, 0.75]},
         ],
         "units": "crystal",
-        "cell": [[5.468763846, 0.0, 0.0], [-0.0, 5.468763846, 0.0], [0.0, 0.0, 5.468763846]],
         "constraints": [],
         "labels": [],
     },
@@ -137,7 +136,6 @@
             {"id": 7, "value": [0.625, 0.625, 0.25]},
         ],
         "units": "crystal",
-        "cell": [[6.697840473, 0.0, 3.867], [2.232613491, 6.314784557, 3.867], [0.0, 0.0, 3.867]],
         "constraints": [],
         "labels": [],
     },
@@ -176,7 +174,6 @@
     "make_primitive": True,
 }
 
-
 SI_SLAB_100: Dict[str, Any] = {
     "name": "Si8(001), termination Si_P4/mmm_1, Slab",
     "basis": {
@@ -201,7 +198,6 @@
             {"id": 7, "value": [0.0, 0.5, 0.643382864]},
         ],
         "units": "crystal",
-        "cell": [[3.867, 0.0, 0.0], [-0.0, 3.867, 0.0], [0.0, 0.0, 15.937527692]],
         "constraints": [],
         "labels": [],
     },
@@ -254,7 +250,6 @@
                             {"id": 7, "value": [0.75, 0.75, 0.75]},
                         ],
                         "units": "crystal",
-                        "cell": [[5.468763846, 0.0, 0.0], [-0.0, 5.468763846, 0.0], [0.0, 0.0, 5.468763846]],
                         "constraints": [],
                         "labels": [],
                     },
@@ -293,7 +288,6 @@
     "isUpdated": True,
 }
 
-
 SI_SLAB: Dict[str, Any] = {
     "name": "Si8(001), termination Si_P4/mmm_1, Slab",
     "basis": {
@@ -303,7 +297,6 @@
             {"id": 1, "value": [0.25, 0.5, 0.145147133]},
         ],
         "units": "crystal",
-        "cell": [[3.867, 0.0, 0.0], [1.9335, 3.348920236, 0.0], [0.0, 0.0, 8.157392279]],
         "constraints": [],
         "labels": [],
     },
@@ -333,7 +326,6 @@
     "isUpdated": True,
 }
 
-
 SI_SLAB_PASSIVATED = {
     "name": "Si8(001), termination Si_P4/mmm_1, Slab H-passivated",
     "basis": {
@@ -350,7 +342,6 @@
             {"id": 3, "value": [0.583333333, 0.833333333, 0.729812904]},
         ],
         "units": "crystal",
-        "cell": [[3.867, 0.0, 0.0], [1.9335, 3.34892, 0.0], [0.0, 0.0, 8.157392]],
         "labels": [],
     },
     "lattice": {
@@ -377,7 +368,8 @@
         "build": {
             "configuration": {
                 "type": "PassivationConfiguration",
-                "slab": SI_SLAB,
+                # TODO: `basis` retains "cell" leading to a mismatch in the test
+                "slab": reduce(lambda d, key: d.get(key, {}), ["basis"], SI_SLAB).pop("cell", None),
                 "passivant": "H",
                 "bond_length": 1.48,
                 "surface": "both",
@@ -388,17 +380,15 @@
     "isUpdated": True,
 }
 
-
 SI_SLAB_VACUUM = copy.deepcopy(SI_SLAB)
 SI_SLAB_VACUUM["basis"]["coordinates"] = [
     {"id": 0, "value": [0.583333333, 0.833333333, 0.149981861]},
     {"id": 1, "value": [0.25, 0.5, 0.089989116]},
 ]
-SI_SLAB_VACUUM["basis"]["cell"] = [[3.867, 0.0, 0.0], [1.9335, 3.348920236, 0.0], [0.0, 0.0, 13.157392279]]
+# SI_SLAB_VACUUM["basis"]["cell"] = [[3.867, 0.0, 0.0], [1.9335, 3.348920236, 0.0], [0.0, 0.0, 13.157392279]]
 SI_SLAB_VACUUM["lattice"]["c"] = 13.157392279
 SI_SLAB_VACUUM["lattice"]["vectors"]["c"] = [0.0, 0.0, 13.157392279]
 
-
 clean_material = Material.create(Material.default_config)
 slab_111_config = SlabConfiguration(
     bulk=clean_material,
@@ -428,7 +418,6 @@
         "elements": [{"id": 0, "value": "C"}, {"id": 1, "value": "C"}],
         "coordinates": [{"id": 0, "value": [0, 0, 0]}, {"id": 1, "value": [0.333333, 0.666667, 0]}],
         "units": "crystal",
-        "cell": [[2.467291, 0, 0], [-1.2336454999, 2.1367366845, 0], [0, 0, 20]],
         "constraints": [],
     },
     "lattice": {
@@ -451,7 +440,7 @@
     "isNonPeriodic": False,
 }
 
-GRAPHENE_ZIGZAG_NANORIBBON = {
+GRAPHENE_ZIGZAG_NANORIBBON: Dict[str, Any] = {
     "name": "Graphene (Zigzag nanoribbon)",
     "basis": {
         "elements": [
@@ -491,7 +480,6 @@
             {"id": 15, "value": [0.812500063, 0.6333333, 0.5]},
         ],
         "units": "crystal",
-        "cell": [[9.869164, 0.0, 0.0], [-0.0, 10.683683422, 0.0], [0.0, 0.0, 20.0]],
         "constraints": [],
         "labels": [],
     },
@@ -552,25 +540,24 @@
             {"id": 15, "value": "C"},
         ],
         "coordinates": [
-            {"id": 0, "value": [0.041666626, 0.35000005, 0.5]},
-            {"id": 1, "value": [0.208333376, 0.34999995, 0.5]},
-            {"id": 2, "value": [0.041666626, 0.55000005, 0.5]},
-            {"id": 3, "value": [0.208333376, 0.54999995, 0.5]},
-            {"id": 4, "value": [0.291666626, 0.45000005, 0.5]},
-            {"id": 5, "value": [0.458333376, 0.44999995, 0.5]},
-            {"id": 6, "value": [0.541666626, 0.35000005, 0.5]},
-            {"id": 7, "value": [0.708333376, 0.34999995, 0.5]},
-            {"id": 8, "value": [0.291666626, 0.65000005, 0.5]},
-            {"id": 9, "value": [0.458333376, 0.64999995, 0.5]},
-            {"id": 10, "value": [0.541666626, 0.55000005, 0.5]},
-            {"id": 11, "value": [0.708333376, 0.54999995, 0.5]},
-            {"id": 12, "value": [0.791666626, 0.45000005, 0.5]},
-            {"id": 13, "value": [0.958333376, 0.44999995, 0.5]},
-            {"id": 14, "value": [0.791666626, 0.65000005, 0.5]},
-            {"id": 15, "value": [0.958333376, 0.64999995, 0.5]},
+            {"id": 0, "value": [0.958333362, 0.35000006, 0.5]},
+            {"id": 1, "value": [0.791666617, 0.34999996, 0.5]},
+            {"id": 2, "value": [0.958333362, 0.550000054, 0.5]},
+            {"id": 3, "value": [0.791666617, 0.549999954, 0.5]},
+            {"id": 4, "value": [0.708333369, 0.450000057, 0.5]},
+            {"id": 5, "value": [0.541666624, 0.449999957, 0.5]},
+            {"id": 6, "value": [0.458333376, 0.35000006, 0.5]},
+            {"id": 7, "value": [0.291666631, 0.34999996, 0.5]},
+            {"id": 8, "value": [0.708333369, 0.650000051, 0.5]},
+            {"id": 9, "value": [0.541666624, 0.649999951, 0.5]},
+            {"id": 10, "value": [0.458333376, 0.550000054, 0.5]},
+            {"id": 11, "value": [0.291666631, 0.549999954, 0.5]},
+            {"id": 12, "value": [0.208333383, 0.450000057, 0.5]},
+            {"id": 13, "value": [0.041666638, 0.449999957, 0.5]},
+            {"id": 14, "value": [0.208333383, 0.650000051, 0.5]},
+            {"id": 15, "value": [0.041666638, 0.649999951, 0.5]},
         ],
         "units": "crystal",
-        "cell": [[8.546946738, 0.0, 0.0], [-0.0, 12.336455, 0.0], [0.0, 0.0, 20.0]],
         "constraints": [],
         "labels": [],
     },
@@ -665,7 +652,6 @@
             {"id": 23, "value": [0.812499933, 0.771862307, 0.5]},
         ],
         "units": "crystal",
-        "cell": [[9.869164, 0.0, 0.0], [-0.0, 10.683683, 0.0], [0.0, 0.0, 20.0]],
         "labels": [],
     },
     "lattice": {
@@ -692,7 +678,8 @@
         "build": {
             "configuration": {
                 "type": "PassivationConfiguration",
-                "slab": GRAPHENE_ZIGZAG_NANORIBBON,
+                # TODO: `basis` retains "cell" leading to a mismatch in the test (as above)
+                "slab": reduce(lambda d, key: d.get(key, {}), ["basis"], GRAPHENE_ZIGZAG_NANORIBBON).pop("cell", None),
                 "passivant": "H",
                 "bond_length": 1.48,
                 "surface": "both",
@@ -702,7 +689,6 @@
     "isUpdated": True,
 }
 
-
 GRAPHENE_NICKEL_INTERFACE = {
     "name": "C2(001)-Ni4(111), Interface, Strain 0.105pct",
     "basis": {
@@ -721,7 +707,6 @@
             {"id": 4, "value": [0.666666667, 0.666666667, 0.611447347]},
         ],
         "units": "crystal",
-        "cell": [[2.478974, 0.0, 0.0], [1.239487, 2.14685446, 0.0], [0.0, 0.0, 27.048147591]],
         "constraints": [],
         "labels": [
             {"id": 0, "value": 0},
@@ -799,7 +784,6 @@
                                 {"id": 3, "value": [0.5, 0.5, 0.0]},
                             ],
                             "units": "crystal",
-                            "cell": [[3.505798652, 0.0, 0.0], [-0.0, 3.505798652, 0.0], [0.0, 0.0, 3.505798652]],
                             "constraints": [],
                             "labels": [],
                         },

tests/py/unit/test_material.py

@@ -10,27 +10,19 @@ def test_create():
     material = Material.create(Material.default_config)
     assert isinstance(material.basis, Basis)
     assert isinstance(material.lattice, Lattice)
-    assert material.to_json() == Material.default_config
     assert material.name == Material.default_config["name"]
 
 
 def test_material_to_json():
     material = Material.create(Material.default_config)
-    labels_array = [{"id": 0, "value": 0}, {"id": 1, "value": 1}]
-    config_with_labels = {
-        **Material.default_config,
-        "basis": {**Material.default_config["basis"], "labels": labels_array},
-    }
-    expected_config = {
-        **Material.default_config,
-        "basis": {**Material.default_config["basis"], "cell": None, "labels": labels_array},
-    }
-    material.basis = Basis.from_dict(**config_with_labels["basis"])
-    assertion_utils.assert_deep_almost_equal(expected_config, material.to_json())
+    assertion_utils.assert_deep_almost_equal(Material.default_config, material.to_json())
 
 
 def test_basis_to_json():
     material = Material.create(Material.default_config)
     basis = material.basis
-    expected_basis_config = {**Material.default_config["basis"], "cell": None, "labels": []}
+    expected_basis_config = {**Material.default_config["basis"], "labels": []}
     assertion_utils.assert_deep_almost_equal(expected_basis_config, basis.to_json())
+
+
+# TODO: Add test to check if basis.cell is changed when lattice of material is changed, and vice versa

tests/py/unit/test_tools_build_defect.py

@@ -114,10 +114,17 @@ def test_create_adatom_equidistant():
     defect = create_slab_defect(configuration=configuration, builder=EquidistantAdatomSlabDefectBuilder())
 
     assert defect.basis.elements.values[-1] == "Si"
-    # We expect adatom to shift from provided position
-    assertion_utils.assert_deep_almost_equal(
-        [0.383333334, 0.558333333, 0.872332562], defect.basis.coordinates.values[-1]
-    )
+    assert (len(configuration.crystal.basis.coordinates.values) + 1) == len(defect.basis.coordinates.values)
+    defect.to_cartesian()
+    # TODO: resolve the problem with the test in GH pipeline
+    # on MacOS slab atoms have different coordinates than in GH and pyodide
+    # for the same versions of packages
+    coordinate_macosx = [6.477224996, 3.739627331, 14.234895469]
+    coordinate_linux_and_emscripten = [5.123775004, 3.739627331, 14.234895469]
+    defect_coordinate = defect.basis.coordinates.values[-1]
+    is_passing_on_macosx = coordinate_macosx == defect_coordinate
+    is_passing_on_linux_and_emscripten = coordinate_linux_and_emscripten == defect_coordinate
+    assert is_passing_on_macosx or is_passing_on_linux_and_emscripten
 
 
 @pytest.mark.skip(reason="This test is failing due to the difference in slab generation between GHA and local")