added warning for length-0 vectors and offset argument in add_vacuum

Enabled offset argument in add_vacuum of geometries, can be useful.

Since adding a warning to the 0-length vectors of Lattice
we have moved away from using geometry.add in the surface stuff.
Better to explicitly use add_vacuum (also for clarity).

Made the understanding in the BandStructure code much better
my moving things around and streamlining it.

Added some more tests.

Signed-off-by: Nick Papior <nickpapior@gmail.com>

CHANGELOG.md

@@ -15,12 +15,16 @@ we hit release version 1.0.0.
 - Creation of chiral GNRs (`kind=chiral` in `sisl.geom.nanoribbon`/`sisl.geom.graphene_nanoribbon`
   as well as `sisl.geom.cgnr`)
 - Creation of [n]-triangulenes (`sisl.geom.triangulene`)
+- added `offset` argument in `Geometry.add_vacuum` to enable shifting atomic coordinates
 
 ### Fixed
+- `Lattice` objects now issues a warning when created with 0-length vectors
 - HSX file reads should respect input geometry arguments
 - enabled slicing in matrix assignments, #650
 - changed `Shape.volume()` to `Shape.volume` 
 - growth direction for zigzag heteroribbons
+- `BandStructure` points can now automatically add the `nsc == 1` axis as would
+  be done for assigning matrix elements (it fills with 0's).
 
 ### Changed
 - `vacuum` is now an optional parameter for all ribbon structures

src/sisl/geom/surfaces.py

@@ -254,8 +254,8 @@ def iter_func(key, layer):
             **kwargs,
         )
         # add vacuum
-        vacuum = Lattice([0, 0, vacuums.pop(0)])
-        out = out.add(vacuum, offset=(0, 0, vacuum.cell[2, 2]))
+        vacuum = vacuums.pop(0)
+        out = out.add_vacuum(vacuum, 2, offset=(0, 0, vacuum))
         ivacuum += 1
         islab += 1
 
@@ -275,9 +275,9 @@ def iter_func(key, layer):
         if layer is None:
             dx = out.cell[2, 2] - out.xyz[:, 2].max()
             # this ensures the vacuum is exactly vacuums[iv]
-            vacuum = Lattice([0, 0, vacuums.pop(0) - dx])
+            vacuum = vacuums.pop(0) - dx
             ivacuum += 1
-            out = out.add(vacuum)
+            out = out.add_vacuum(vacuum, 2)
         else:
             geom = func(
                 *args,

src/sisl/geometry.py

@@ -3040,15 +3040,25 @@ def add(
         other :
             Other geometry class which is added
         offset :
-            offset in geometry of `other` when adding the atoms. Only if `other` is
-            of instance `Geometry`.
+            offset in geometry of `other` when adding the atoms.
+            Otherwise it is the offset of the `self` atoms.
 
         See Also
         --------
         append : appending geometries
         prepend : prending geometries
         attach : attach a geometry
         insert : insert a geometry
+
+        Examples
+        --------
+        >>> first = Geometry(...)
+        >>> second = Geometry(...)
+        >>> lattice = Lattice(...)
+        >>> added = first.add(second, offset=(0, 0, 2))
+        >>> assert np.allclose(added.xyz[:len(first)], first.xyz)
+        >>> assert np.allclose(added.xyz[len(first):] - [0, 0, 2], second.xyz)
+
         """
         if isinstance(other, Lattice):
             xyz = self.xyz.copy() + _a.arrayd(offset)
@@ -3063,7 +3073,9 @@ def add(
             names = self._names.merge(other._names, offset=len(self))
         return self.__class__(xyz, atoms=atoms, lattice=lattice, names=names)
 
-    def add_vacuum(self, vacuum: float, axis: int) -> Geometry:
+    def add_vacuum(
+        self, vacuum: float, axis: int, offset: Sequence[float] = (0, 0, 0)
+    ) -> Geometry:
         """Add vacuum along the `axis` lattice vector
 
         When the vacuum is bigger than the maximum orbital ranges the
@@ -3076,12 +3088,15 @@ def add_vacuum(self, vacuum: float, axis: int) -> Geometry:
            amount of vacuum added, in Ang
         axis :
            the lattice vector to add vacuum along
+        offset :
+            offset in geometry when adding the vacuum.
 
         Returns
         -------
         Geometry : a new geometry with added vacuum
         """
         new = self.copy()
+        new.xyz += _a.arrayd(offset)
         new.set_lattice(self.lattice.add_vacuum(vacuum, axis))
         if vacuum > self.maxR() + 0.001:
             # only overwrite along axis

src/sisl/lattice.py

@@ -25,7 +25,7 @@
 from ._internal import set_module
 from ._lattice import cell_invert, cell_reciprocal
 from ._math_small import cross3, dot3
-from .messages import SislError, deprecate, deprecate_argument, deprecation, info
+from .messages import SislError, deprecate, deprecate_argument, deprecation, info, warn
 from .quaternion import Quaternion
 from .shape.prism4 import Cuboid
 from .utils.mathematics import fnorm
@@ -130,6 +130,11 @@ def __init__(
         # If the length of cell is 6 it must be cell-parameters, not
         # actual cell coordinates
         self.cell = self.tocell(cell)
+        if np.any(self.length < 1e-7):
+            warn(
+                f"{self.__class__.__name__} got initialized with one or more "
+                "lattice vector(s) with 0 length. Use with care."
+            )
 
         if origin is None:
             self._origin = _a.zerosd(3)

src/sisl/physics/brillouinzone.py

@@ -307,12 +307,19 @@ def set_parent(self, parent):
 
     def __str__(self):
         """String representation of the BrillouinZone"""
+        parent = str(self._parent_lattice()).replace("\n", "\n ")
+        return f"{self.__class__.__name__}{{nk: {len(self)},\n {parent}\n}}"
+
+    def _parent_lattice(self):
+        """Ensures to return the lattice of the parent, however it may be nested"""
         parent = self.parent
         if isinstance(parent, Lattice):
-            parent = str(parent).replace("\n", "\n ")
-        else:
-            parent = str(parent.lattice).replace("\n", "\n ")
-        return f"{self.__class__.__name__}{{nk: {len(self)},\n {parent}\n}}"
+            return parent
+        if isinstance(parent.lattice, Lattice):
+            return parent.lattice
+        raise ValueError(
+            f"{self.__class__.__name__} could not extract the lattice object, it must be located elsewhere."
+        )
 
     def __getstate__(self):
         """Return dictionary with the current state"""
@@ -877,10 +884,7 @@ def displacement(self):
 
     def __str__(self):
         """String representation of `MonkhorstPack`"""
-        if isinstance(self.parent, Lattice):
-            p = self.parent
-        else:
-            p = self.parent.lattice
+        p = self._parent_lattice()
         return (
             "{cls}{{nk: {nk:d}, size: [{size[0]:.5f} {size[1]:.5f} {size[0]:.5f}], trs: {trs},"
             "\n diagonal: [{diag[0]:d} {diag[1]:d} {diag[2]:d}], displacement: [{disp[0]:.5f} {disp[1]:.5f} {disp[2]:.5f}],"
@@ -1217,7 +1221,8 @@ class BandStructure(BrillouinZone):
        An object with associated `parent.cell` and `parent.rcell` or
        an array of floats which may be turned into a `Lattice`
     points : array_like of float
-       a list of points that are the *corners* of the path
+       a list of points that are the *corners* of the path.
+       Define a discontinuity in the points by adding a `None` in the list.
     divisions : int or array_like of int
        number of divisions in each segment.
        If a single integer is passed it is the total number
@@ -1243,9 +1248,9 @@ class BandStructure(BrillouinZone):
     >>> bs = BandStructure(lattice, [[0] * 3, [0.5] * 3, [1.] * 3], 200)
     >>> bs = BandStructure(lattice, [[0] * 3, [0.5] * 3, [1.] * 3], 200, ['Gamma', 'M', 'Gamma'])
 
-    A disconnected band structure may be created by either having a point of 0 length, or None.
+    A disconnected band structure may be created by having None as the element.
     Note that the number of names does not contain the empty points (they are simply removed).
-    Such a band-structure may be useful when one is not interested in a fully connected band structure.
+    Such a band-structure may be useful when one is interested in a discontinuous band structure.
 
     >>> bs = BandStructure(lattice, [[0, 0, 0], [0, 0.5, 0], None, [0.5, 0, 0], [0.5, 0.5, 0]], 200)
     """
@@ -1259,6 +1264,7 @@ class BandStructure(BrillouinZone):
     def __init__(self, parent, *args, **kwargs):
         # points, divisions, names=None):
         super().__init__(parent)
+        lattice = self._parent_lattice()
 
         points = kwargs.pop("points", None)
         if points is None:
@@ -1267,6 +1273,31 @@ def __init__(self, parent, *args, **kwargs):
             else:
                 raise ValueError(f"{self.__class__.__name__} 'points' argument missing")
 
+        # Extract the jump indices
+        # In that case it is a disconnected path
+        def is_empty(p):
+            try:
+                return len(p) == 0
+            except Exception:
+                return p is None
+
+        jump_idx = []
+        _points = []
+        for i, p in enumerate(points):
+            if is_empty(p):
+                if i > 0:
+                    # we can't have a jump at the first index
+                    jump_idx.append(i)
+            else:
+                _points.append(lattice._fill(p, dtype=np.float64))
+
+        # convert to exact array and correct for removed indices
+        jump_idx = _a.arrayi(jump_idx) - _a.arangei(len(jump_idx))
+
+        # fill with correct points
+        self.points = _a.arrayd(_points)
+        del _points  # clean-up for clarity
+
         divisions = kwargs.pop("divisions", None)
         if divisions is None:
             if len(args) > 0:
@@ -1294,26 +1325,9 @@ def __init__(self, parent, *args, **kwargs):
                 f"{self.__class__.__name__} unknown keyword arguments after parsing [points, divisions, names, jump_dk]: {list(kwargs.keys())}"
             )
 
-        # Copy over points
-        # Check if any of the points is None or has length 0
-        # In that case it is a disconnected path
-        def is_empty(ix):
-            try:
-                return len(ix[1]) == 0
-            except Exception:
-                return ix[1] is None
-
-        # filter out jump directions
-        jump_idx = _a.arrayi([i for i, _ in filter(is_empty, enumerate(points))])
-
-        # store only *valid* points
-        self.points = _a.arrayd([p for i, p in enumerate(points) if i not in jump_idx])
-
         # remove erroneous jumps
-        if len(points) - 1 in jump_idx:
+        if len(self.points) in jump_idx:
             jump_idx = jump_idx[:-1]
-        if 0 in jump_idx:
-            jump_idx = jump_idx[1:]
 
         if self._jump_dk.size > 1 and jump_idx.size != self._jump_dk.size:
             raise ValueError(
@@ -1322,8 +1336,6 @@ def is_empty(ix):
 
         # The jump-idx is equal to using np.split(self.points, jump_idx)
         # which then returns continuous sections
-        # correct for removed indices
-        jump_idx -= np.arange(len(jump_idx))
         self._jump_idx = jump_idx
 
         # If the array has fewer points we try and determine

src/sisl/physics/tests/test_brillouinzone.py

@@ -28,15 +28,12 @@ class t:
         def __init__(self):
             self.s1 = Lattice(1, nsc=[3, 3, 1])
             self.s2 = Lattice([2, 2, 10, 90, 90, 60], [5, 5, 1])
+            self.s3 = Lattice(1, nsc=[3, 1, 3])
 
     return t()
 
 
 class TestBrillouinZone:
-    def setUp(self, setup):
-        setup.s1 = Lattice(1, nsc=[3, 3, 1])
-        setup.s2 = Lattice([2, 2, 10, 90, 90, 60], [5, 5, 1])
-
     def test_bz1(self, setup):
         bz = BrillouinZone(1.0)
         str(bz)
@@ -250,10 +247,6 @@ def test_merge_scales_scalar(self):
 
 @pytest.mark.monkhorstpack
 class TestMonkhorstPack:
-    def setUp(self, setup):
-        setup.s1 = Lattice(1, nsc=[3, 3, 1])
-        setup.s2 = Lattice([2, 2, 10, 90, 90, 60], [5, 5, 1])
-
     def test_class(self, setup):
         class Test(LatticeChild):
             def __init__(self, lattice):
@@ -736,10 +729,6 @@ def test_in_primitive(self):
 
 @pytest.mark.bandstructure
 class TestBandStructure:
-    def setUp(self, setup):
-        setup.s1 = Lattice(1, nsc=[3, 3, 1])
-        setup.s2 = Lattice([2, 2, 10, 90, 90, 60], [5, 5, 1])
-
     def test_pbz1(self, setup):
         bz = BandStructure(setup.s1, [[0] * 3, [0.5] * 3], 300)
         assert len(bz) == 300
@@ -762,6 +751,11 @@ def test_pbs_divisions(self, setup):
         bz = BandStructure(setup.s1, [[0] * 3, [0.25] * 3, [0.5] * 3], [10, 10])
         assert len(bz) == 21
 
+    def test_pbc_fill(self, setup):
+        bz = BandStructure(setup.s3, [[0] * 2, [0.25] * 2, [0.5] * 2], [10, 10])
+        assert np.allclose(bz.k[:, 1], 0)
+        assert len(bz) == 21
+
     def test_pbs_missing_arguments(self, setup):
         with pytest.raises(ValueError):
             bz = BandStructure(setup.s1, divisions=[10, 10])

src/sisl/tests/test_geometry.py

@@ -709,6 +709,21 @@ def test_add(self, setup):
         assert len(double) == len(setup.g) * 2
         assert np.allclose(setup.g.cell * 2, double.cell)
 
+    def test_add_vacuum(self, setup):
+        g = setup.g
+        added = setup.g.add_vacuum(g.cell[2, 2], 2)
+        assert len(added) == len(g)
+        assert np.allclose(
+            added.lattice.length, g.lattice.length + [0, 0, g.cell[2, 2]]
+        )
+        assert np.allclose(added.xyz, g.xyz)
+        added = g.add_vacuum(g.cell[2, 2], 2, offset=(1, 2, 3))
+        assert len(added) == len(g)
+        assert np.allclose(
+            added.lattice.length, g.lattice.length + [0, 0, g.cell[2, 2]]
+        )
+        assert np.allclose(added.xyz, g.xyz + [1, 2, 3])
+
     def test_insert(self, setup):
         double = setup.g.insert(0, setup.g)
         assert len(double) == len(setup.g) * 2

src/sisl/tests/test_lattice.py

@@ -423,6 +423,11 @@ def test_set_lattice_off_wrong_size(self, setup):
             lattice.sc_off = np.zeros([10000, 3])
 
 
+def test_zero_length():
+    with pytest.warns(sisl.SislWarning):
+        Lattice([1, 0, 0])
+
+
 def _dot(u, v):
     """Dot product u . v"""
     return u[0] * v[0] + u[1] * v[1] + u[2] * v[2]