fix: allow StructureData without specified cell

aiida/orm/nodes/data/structure.py

@@ -29,8 +29,8 @@
 _MASS_THRESHOLD = 1.e-3
 # Threshold to check if the sum is one or not
 _SUM_THRESHOLD = 1.e-6
-# Threshold used to check if the cell volume is not zero.
-_VOLUME_THRESHOLD = 1.e-6
+# Default cell
+_DEFAULT_CELL = ((0, 0, 0), (0, 0, 0), (0, 0, 0))
 
 _valid_symbols = tuple(i['symbol'] for i in elements.values())
 _atomic_masses = {el['symbol']: el['mass'] for el in elements.values()}
@@ -52,9 +52,6 @@ def _get_valid_cell(inputcell):
     except (IndexError, ValueError, TypeError):
         raise ValueError('Cell must be a list of three vectors, each defined as a list of three coordinates.')
 
-    if abs(calc_cell_volume(the_cell)) < _VOLUME_THRESHOLD:
-        raise ValueError('The cell volume is zero. Invalid cell.')
-
     return the_cell
 
 
@@ -138,25 +135,13 @@ def has_spglib():
 
 def calc_cell_volume(cell):
     """
-    Calculates the volume of a cell given the three lattice vectors.
-
-    It is calculated as cell[0] . (cell[1] x cell[2]), where . represents
-    a dot product and x a cross product.
-
-    :param cell: the cell vectors; the must be a 3x3 list of lists of floats,
-            no other checks are done.
+    Compute the three-dimensional cell volume in Angstrom^3.
 
+    :param cell: the cell vectors; the must be a 3x3 list of lists of floats
     :returns: the cell volume.
     """
-    # pylint: disable=invalid-name
-    # returns the volume of the primitive cell: |a1.(a2xa3)|
-    a1 = cell[0]
-    a2 = cell[1]
-    a3 = cell[2]
-    a_mid_0 = a2[1] * a3[2] - a2[2] * a3[1]
-    a_mid_1 = a2[2] * a3[0] - a2[0] * a3[2]
-    a_mid_2 = a2[0] * a3[1] - a2[1] * a3[0]
-    return abs(a1[0] * a_mid_0 + a1[1] * a_mid_1 + a1[2] * a_mid_2)
+    import numpy as np
+    return np.abs(np.dot(cell[0], np.cross(cell[1], cell[2])))
 
 
 def _create_symbols_tuple(symbols):
@@ -761,45 +746,23 @@ def __init__(
 
         else:
             if cell is None:
-                cell = [[1., 0., 0.], [0., 1., 0.], [0., 0., 1.]]
+                cell = _DEFAULT_CELL
+            self.set_cell(cell)
 
             if pbc is None:
                 pbc = [True, True, True]
-
-            self.set_cell(cell)
             self.set_pbc(pbc)
 
     def get_dimensionality(self):
         """
-        This function checks the dimensionality of the structure and
-        calculates its length/surface/volume
-        :return: returns the dimensionality and length/surface/volume
-        """
-
-        import numpy as np
-
-        retdict = {}
+        Return the dimensionality of the structure and its length/surface/volume.
 
-        cell = np.array(self.cell)
-        pbc = np.array(self.pbc)
-        dim = len(pbc[pbc])
+        Zero-dimensional structures are assigned "volume" 0.
 
-        retdict['dim'] = dim
-        retdict['label'] = self._dimensionality_label[dim]
-
-        if dim == 0:
-            pass
-        elif dim == 1:
-            retdict['value'] = np.linalg.norm(cell[pbc])
-        elif dim == 2:
-            vectors = cell[pbc]
-            retdict['value'] = np.linalg.norm(np.cross(vectors[0], vectors[1]))
-        elif dim == 3:
-            retdict['value'] = np.dot(cell[0], np.cross(cell[1], cell[2]))
-        else:
-            raise ValueError(f'Dimensionality {dim} must be <= 3')
-
-        return retdict
+        :return: returns a dictionary with keys "dim" (dimensionality integer), "label" (dimensionality label)
+            and "value" (numerical length/surface/volume).
+        """
+        return _get_dimensionality(self.pbc, self.cell)
 
     def set_ase(self, aseatoms):
         """
@@ -948,6 +911,8 @@ def _validate(self):
         except ValueError as exc:
             raise ValidationError(f'Invalid periodic boundary conditions: {exc}')
 
+        _validate_dimensionality(self.pbc, self.cell)
+
         try:
             # This will try to create the kinds objects
             kinds = self.kinds
@@ -1124,9 +1089,7 @@ def _parse_xyz(self, inputstring):
 
     def _adjust_default_cell(self, vacuum_factor=1.0, vacuum_addition=10.0, pbc=(False, False, False)):
         """
-        If the structure was imported from an xyz file, it lacks a defined cell,
-        and the default cell is taken ([[1,0,0], [0,1,0], [0,0,1]]),
-        leading to an unphysical definition of the structure.
+        If the structure was imported from an xyz file, it lacks a cell.
         This method will adjust the cell
         """
         # pylint: disable=invalid-name
@@ -1138,10 +1101,6 @@ def get_extremas_from_positions(positions):
             """
             return list(zip(*[(min(values), max(values)) for values in zip(*positions)]))
 
-        # First, set PBC
-        # All the checks are done in get_valid_pbc called by set_pbc, no need to check anything here
-        self.set_pbc(pbc)
-
         # Calculating the minimal cell:
         positions = np.array([site.position for site in self.sites])
         position_min, _ = get_extremas_from_positions(positions)
@@ -1162,6 +1121,11 @@ def get_extremas_from_positions(positions):
         newcell = np.diag(minimal_orthorhombic_cell_dimensions)
         self.set_cell(newcell.tolist())
 
+        # Now set PBC (checks are done in set_pbc, no need to check anything here)
+        self.set_pbc(pbc)
+
+        return self
+
     def get_description(self):
         """
         Returns a string with infos retrieved from StructureData node's properties
@@ -1753,7 +1717,9 @@ def has_vacancies(self):
 
     def get_cell_volume(self):
         """
-        Returns the cell volume in Angstrom^3.
+        Returns the three-dimensional cell volume in Angstrom^3.
+
+        Use the `get_dimensionality` method in order to get the area/length of lower-dimensional cells.
 
         :return: a float.
         """
@@ -2490,5 +2456,57 @@ def __str__(self):
         return f"kind name '{self.kind_name}' @ {self.position[0]},{self.position[1]},{self.position[2]}"
 
 
-# get_structuredata_from_qeinput has been moved to:
-# aiida.tools.codespecific.quantumespresso.qeinputparser
+def _get_dimensionality(pbc, cell):
+    """
+    Return the dimensionality of the structure and its length/surface/volume.
+
+    Zero-dimensional structures are assigned "volume" 0.
+
+    :return: returns a dictionary with keys "dim" (dimensionality integer), "label" (dimensionality label)
+        and "value" (numerical length/surface/volume).
+    """
+
+    import numpy as np
+
+    retdict = {}
+
+    pbc = np.array(pbc)
+    cell = np.array(cell)
+
+    dim = len(pbc[pbc])
+
+    retdict['dim'] = dim
+    retdict['label'] = StructureData._dimensionality_label[dim]  # pylint: disable=protected-access
+
+    if dim not in (0, 1, 2, 3):
+        raise ValueError(f'Dimensionality {dim} must be one of 0, 1, 2, 3')
+
+    if dim == 0:
+        # We have no concept of 0d volume. Let's return a value of 0 for a consistent output dictionary
+        retdict['value'] = 0
+    elif dim == 1:
+        retdict['value'] = np.linalg.norm(cell[pbc])
+    elif dim == 2:
+        vectors = cell[pbc]
+        retdict['value'] = np.linalg.norm(np.cross(vectors[0], vectors[1]))
+    elif dim == 3:
+        retdict['value'] = calc_cell_volume(cell)
+
+    return retdict
+
+
+def _validate_dimensionality(pbc, cell):
+    """
+    Check whether the given pbc and cell vectors are consistent.
+    """
+    dim = _get_dimensionality(pbc, cell)
+
+    # 0-d structures put no constraints on the cell
+    if dim['dim'] == 0:
+        return
+
+    # finite-d structures should have a cell with finite volume
+    if dim['value'] == 0:
+        raise ValueError(f'Structure has periodicity {pbc} but {dim["dim"]}-d volume 0.')
+
+    return

tests/orm/test_querybuilder.py

@@ -427,6 +427,7 @@ def test_operators_eq_lt_gt(self):
     def test_subclassing(self):
         s = orm.StructureData()
         s.set_attribute('cat', 'miau')
+        s.set_pbc(False)
         s.store()
 
         d = orm.Data()
@@ -795,6 +796,8 @@ def test_round_trip_append(self):
         for cls in (orm.StructureData, orm.Dict, orm.Data):
             obj = cls()
             obj.set_attribute('foo-qh2', 'bar')
+            if cls is orm.StructureData:
+                obj.set_pbc(False)
             obj.store()
             g.add_nodes(obj)
 

tests/restapi/conftest.py

@@ -86,9 +86,9 @@ def populate_restapi_database(aiida_profile_clean):
     # pylint: disable=unused-argument
     from aiida import orm
 
-    struct_forcif = orm.StructureData().store()
-    orm.StructureData().store()
-    orm.StructureData().store()
+    struct_forcif = orm.StructureData(pbc=False).store()
+    orm.StructureData(pbc=False).store()
+    orm.StructureData(pbc=False).store()
 
     orm.Dict().store()
     orm.Dict().store()

tests/test_dataclasses.py

@@ -955,14 +955,14 @@ def test_cell_zero_vector(self):
         Wrong cell (one vector has zero length)
         """
         with pytest.raises(ValueError):
-            StructureData(cell=((0., 0., 0.), (0., 1., 0.), (0., 0., 1.)))
+            StructureData(cell=((0., 0., 0.), (0., 1., 0.), (0., 0., 1.))).store()
 
     def test_cell_zero_volume(self):
         """
         Wrong cell (volume is zero)
         """
         with pytest.raises(ValueError):
-            StructureData(cell=((1., 0., 0.), (0., 1., 0.), (1., 1., 0.)))
+            StructureData(cell=((1., 0., 0.), (0., 1., 0.), (1., 1., 0.))).store()
 
     def test_cell_ok_init(self):
         """
@@ -1729,8 +1729,9 @@ def test_xyz_parser(self):
             assert s.sites
             assert s.kinds
             assert s.cell
+
             # The default cell is given in these cases:
-            assert s.cell == np.diag([1, 1, 1]).tolist()
+            assert s.cell == np.diag([0, 0, 0]).tolist()
 
         # Testing a case where 1
         xyz_string4 = """
@@ -1934,6 +1935,26 @@ def test_ase(self):
 
         assert round(abs(c[1].mass - 110.2), 7) == 0
 
+    @skip_ase
+    def test_ase_molecule(self):  # pylint: disable=no-self-use
+        """Tests that importing a molecule from ASE works."""
+        from ase.build import molecule
+        s = StructureData(ase=molecule('H2O'))
+
+        assert s.pbc == (False, False, False)
+        retdict = s.get_dimensionality()
+        assert retdict['value'] == 0
+        assert retdict['dim'] == 0
+
+        with pytest.raises(ValueError):
+            # A periodic cell requires a nonzero volume in periodic directions
+            s.set_pbc(True)
+            s.store()
+
+        # after setting a cell, we should be able to store
+        s.set_cell([[5, 0, 0], [0, 5, 0], [0, 0, 5]])
+        s.store()
+
     @skip_ase
     def test_conversion_of_types_1(self):
         """
@@ -2002,7 +2023,7 @@ def test_conversion_of_types_3(self):
         """
         Tests StructureData -> ASE, with all sorts of kind names
         """
-        a = StructureData()
+        a = StructureData(cell=[[1, 0, 0], [0, 1, 0], [0, 0, 1]])
         a.append_atom(position=(0., 0., 0.), symbols='Ba', name='Ba')
         a.append_atom(position=(0., 0., 0.), symbols='Ba', name='Ba1')
         a.append_atom(position=(0., 0., 0.), symbols='Cu', name='Cu')
@@ -2294,7 +2315,7 @@ class TestPymatgenFromStructureData:
     def test_1(self):
         """Tests the check of periodic boundary conditions."""
         struct = StructureData()
-
+        struct.set_cell([[1, 0, 0], [0, 1, 2], [3, 4, 5]])
         struct.pbc = [True, True, True]
         struct.get_pymatgen_structure()
 

tests/tools/archive/orm/test_groups.py

@@ -27,7 +27,7 @@ def test_nodes_in_group(tmp_path, aiida_profile_clean, aiida_localhost):
     user.store()
 
     # Create a structure data node that has a calculation as output
-    sd1 = orm.StructureData()
+    sd1 = orm.StructureData(pbc=False)
     sd1.user = user
     sd1.label = 'sd1'
     sd1.store()
@@ -73,7 +73,7 @@ def test_group_export(tmp_path, aiida_profile_clean):
     user.store()
 
     # Create a structure data node
-    sd1 = orm.StructureData()
+    sd1 = orm.StructureData(pbc=False)
     sd1.user = user
     sd1.label = 'sd1'
     sd1.store()
@@ -118,7 +118,7 @@ def test_group_import_existing(tmp_path, aiida_profile_clean):
     user.store()
 
     # Create a structure data node
-    sd1 = orm.StructureData()
+    sd1 = orm.StructureData(pbc=False)
     sd1.user = user
     sd1.label = 'sd'
     sd1.store()