`translate2uc` should use boundary conditions (?)

[pfebrer]

Here in Geometry.translate2uc, nsc is used to understand which axes to translate:

https://github.com/zerothi/sisl/blob/8d6da596972b77451376b5492c42ba490f1bec2c/src/sisl/_core/geometry.py#L1741

I think it should check for periodic boundary conditions instead, no?

[zerothi]

Indeed, however, I am not sure whether it should be a default of both merged? Consider a case where a user has done pbc = (T, T, F) but then set nsc = [3, 1, 1]?

However, I can see that the documentation specifies periodic direction, so if this is clarified in the doc-string, that pbc is used, then fine!

[zerothi]

Perhaps there are other places:

~/codes/sisl/src % grep "nsc > 1" **/*.py                                                                                                                                                main
sisl/_core/geometry.py:            axes = (self.lattice.nsc > 1).nonzero()[0]
sisl/_core/geometry.py:        elif np.any(self.nsc > 1):
sisl/_core/geometry.py:        periodic system (where ``self.nsc > 1`` or `periodic` is true).
sisl/_core/geometry.py:            directions are only where ``self.nsc > 1 & self.pbc``.
sisl/_core/geometry.py:            periodic = np.logical_and(self.pbc, self.nsc > 1)
sisl/_core/geometry.py:            pbc=geom.nsc > 1,
sisl/_core/lattice.py:            if changed.any() and (~bc).all() and nsc > 1:
sisl/_core/sparse_geometry.py:            axes = (self.lattice.nsc > 1).nonzero()[0]
sisl/_core/tests/test_geometry.py:        # Even if the geometry has nsc > 1, if we set periodic=False
sisl/io/dftb/realdat.py:            if np.any(nsc > 1):
sisl/io/siesta/binaries.py:            if all(nsc > 1):
sisl/io/siesta/fdf.py:        # periodic = geom.nsc > 1
sisl/io/siesta/tests/test_orb_indx.py:    assert np.all(nsc > 1)
sisl/physics/brillouinzone.py:           The default value is `(self.parent.nsc > 1).nonzero()[0]`.
sisl/physics/brillouinzone.py:            periodic = (self.parent.nsc > 1).nonzero()[0]
sisl/physics/brillouinzone.py:            if self.points.shape[1] != np.sum(self.parent.nsc > 1):
sisl/viz/data/pdos.py:            kgrid = [3 if nsc > 1 else 1 for nsc in H.geometry.nsc]

~/codes/sisl/src % grep "nsc == 1" **/*.py                                                                                                                                               main
sisl/_core/geometry.py:        if all(self.nsc == 1):
sisl/_core/sparse_geometry.py:        # since we may have nsc == 1 and cut it X times.
sisl/_core/sparse_geometry.py:        Untiling structures with ``nsc == 1`` along `axis` are assumed to have periodic boundary
sisl/_core/sparse_geometry.py:        When untiling structures with ``nsc == 1`` along `axis` it is important to
sisl/_core/sparse_geometry.py:        Untiling structures with ``nsc == 1`` along `axis` are assumed to have periodic boundary
sisl/_core/sparse_geometry.py:        When untiling structures with ``nsc == 1`` along `axis` it is important to
sisl/io/siesta/binaries.py:                # we will never have all(nsc == 1) since that is
sisl/io/siesta/tests/test_orb_indx.py:    assert np.all(nsc == 1)
sisl/io/xsf.py:        if all(lattice.nsc == 1):

[pfebrer]

This goes back to the never ending discussion :smile:

My view is that as long as you have periodicity, it makes sense to translate to the unit cell regardless of nsc.

[zerothi]

Ok, lets do that then :)

[pfebrer]

Even more so because nsc is many times [1,1,1] because it is simply unset, but we know that there is periodicity. For example when a geometry is read from a SIESTA file.

[zerothi]

Even more so because nsc is many times [1,1,1] because it is simply unset, but we know that there is periodicity. For example when a geometry is read from a SIESTA file.

But... ;) When PSolver can be used, then we will not have periodicity. And the all hell breaks loose. ;) Half-joke aside. Do we know if all geometries read from Siesta actually have periodicity along all directions?

[pfebrer]

For the moment they should, right? SIESTA assumes that everything is periodic when it calculates things.

But yeah when the PSolver makes it possible to not treat things periodically I can see that this will become more complicated :sweat_smile: Maybe one could check Psolver options then, I don't know.

But that is a problem of how to determine boundary conditions. I think that once you have established that the boundaries are periodic I believe it makes sense to use that fact here.