From a4d726d86d1a9df881c089c4423f1d1e2931e45e Mon Sep 17 00:00:00 2001 From: Nick Papior Date: Wed, 23 Oct 2024 22:19:33 +0200 Subject: [PATCH 1/3] trying to fix the within_inf problem Basically the isc did not respect the initial placement of the atomic coordinates, so if atoms were already outside of the unit-cell, then isc would be wrong. Signed-off-by: Nick Papior --- src/sisl/_core/geometry.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/sisl/_core/geometry.py b/src/sisl/_core/geometry.py index d6f1a1446b..0d0475cf78 100644 --- a/src/sisl/_core/geometry.py +++ b/src/sisl/_core/geometry.py @@ -3603,7 +3603,10 @@ def within_inf( if periodic is None: periodic = self.pbc.nonzero()[0] elif isinstance(periodic, bool): - periodic = (0, 1, 2) + if periodic: + periodic = (0, 1, 2) + else: + periodic = () else: try: periodic = map(direction, listify(periodic)) | listify @@ -3623,23 +3626,22 @@ def within_inf( # 1. Number of times each lattice vector must be expanded to fit # inside the "possibly" larger `lattice`. - idx = dot(lattice.cell, self.icell.T) + idx = lattice.cell @ self.icell.T tile_min = floor(idx.min(0)) tile_max = ceil(idx.max(0)).astype(dtype=int32) # Intrinsic offset (when atomic coordinates are outside primary unit-cell) - idx = self.fxyz - tmp = floor(idx.min(0)) - tile_min = np.where(tile_min < tmp, tile_min, tmp).astype(dtype=int32) - tmp = ceil(idx.max(0)) - tile_max = np.where(tmp < tile_max, tile_max, tmp).astype(dtype=int32) - del idx, tmp + fxyz = self.move(1e-8).fxyz + fxyz_floor = floor(fxyz).astype(dtype=int32) + tile_min = np.minimum(tile_min, fxyz_floor.min(0)).astype(dtype=int32) + tile_max = np.maximum(tile_max, ceil(idx.max(0))).astype(dtype=int32) + del idx # 1a) correct for origin displacement - idx = floor(dot(lattice.origin, self.icell.T)) - tile_min = np.where(tile_min < idx, tile_min, idx).astype(dtype=int32) - idx = floor(dot(origin, self.icell.T)) - tile_min = np.where(tile_min < idx, tile_min, idx).astype(dtype=int32) + idx = floor(lattice.origin @ self.icell.T) + tile_min = np.minimum(tile_min, idx).astype(dtype=int32) + idx = floor(origin @ self.icell.T) + tile_min = np.minimum(tile_min, idx).astype(dtype=int32) # 2. Reduce tiling along non-periodic directions tile_min[non_periodic] = 0 @@ -3673,7 +3675,7 @@ def within_inf( # Figure out supercell connections in the smaller indices # Since we have shifted all coordinates into the primary unit cell we # are sure that these fxyz are [0:1[ - fxyz = dot(xyz, self.icell.T) + fxyz = xyz @ self.icell.T # Since there are numerical errors for the above operation # we *have* to account for possible sign-errors @@ -3692,7 +3694,8 @@ def within_inf( # Convert indices to unit-cell indices and also return coordinates and # infinite supercell indices - return self.asc2uc(idx), xyz, isc + ia = self.asc2uc(idx) + return ia, xyz, isc - fxyz_floor[ia] def _orbital_values( self, grid_shape: tuple[int, int, int], truncate_with_nsc: bool = False From a3323784922298c727e08327a243e2b75958d41c Mon Sep 17 00:00:00 2001 From: Nick Papior Date: Thu, 24 Oct 2024 05:52:08 +0200 Subject: [PATCH 2/3] rounded fxyz instead of moving Skewed cells will deliberatily move them to outside supercells, rounding should be preferred. Signed-off-by: Nick Papior --- src/sisl/_core/_ufuncs_geometry.py | 5 +++-- src/sisl/_core/geometry.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sisl/_core/_ufuncs_geometry.py b/src/sisl/_core/_ufuncs_geometry.py index 4fe48fbbeb..7297b41542 100644 --- a/src/sisl/_core/_ufuncs_geometry.py +++ b/src/sisl/_core/_ufuncs_geometry.py @@ -1014,9 +1014,10 @@ def translate( """ g = geometry.copy() if atoms is None: - g.xyz += np.asarray(v, g.xyz.dtype) + g.xyz += v + np.asarray(v, g.xyz.dtype) else: - g.xyz[geometry._sanitize_atoms(atoms).ravel(), :] += np.asarray(v, g.xyz.dtype) + g.xyz[geometry._sanitize_atoms(atoms).ravel(), :] += v return g diff --git a/src/sisl/_core/geometry.py b/src/sisl/_core/geometry.py index 0d0475cf78..b0ecf710e6 100644 --- a/src/sisl/_core/geometry.py +++ b/src/sisl/_core/geometry.py @@ -3631,11 +3631,11 @@ def within_inf( tile_max = ceil(idx.max(0)).astype(dtype=int32) # Intrinsic offset (when atomic coordinates are outside primary unit-cell) - fxyz = self.move(1e-8).fxyz + fxyz = np.round(self.fxyz, decimals=5) fxyz_floor = floor(fxyz).astype(dtype=int32) tile_min = np.minimum(tile_min, fxyz_floor.min(0)).astype(dtype=int32) tile_max = np.maximum(tile_max, ceil(idx.max(0))).astype(dtype=int32) - del idx + del idx, fxyz # 1a) correct for origin displacement idx = floor(lattice.origin @ self.icell.T) From e774c5abdcca3a43ee6e72014e20855d39c86ec0 Mon Sep 17 00:00:00 2001 From: Nick Papior Date: Thu, 24 Oct 2024 13:48:54 +0200 Subject: [PATCH 3/3] final correction that passes tests Signed-off-by: Nick Papior --- src/sisl/_core/geometry.py | 25 +++++++++++++++---------- src/sisl/_core/tests/test_geometry.py | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/sisl/_core/geometry.py b/src/sisl/_core/geometry.py index b0ecf710e6..38a0b6675a 100644 --- a/src/sisl/_core/geometry.py +++ b/src/sisl/_core/geometry.py @@ -3627,14 +3627,16 @@ def within_inf( # 1. Number of times each lattice vector must be expanded to fit # inside the "possibly" larger `lattice`. idx = lattice.cell @ self.icell.T - tile_min = floor(idx.min(0)) + tile_min = floor(idx.min(0)).astype(dtype=int32) tile_max = ceil(idx.max(0)).astype(dtype=int32) # Intrinsic offset (when atomic coordinates are outside primary unit-cell) fxyz = np.round(self.fxyz, decimals=5) - fxyz_floor = floor(fxyz).astype(dtype=int32) - tile_min = np.minimum(tile_min, fxyz_floor.min(0)).astype(dtype=int32) - tile_max = np.maximum(tile_max, ceil(idx.max(0))).astype(dtype=int32) + # We don't collapse this as it is necessary for correcting isc further below + fxyz_ifloor = floor(fxyz).astype(dtype=int32) + fxyz_iceil = ceil(fxyz).max(0).astype(dtype=int32) + tile_min = np.minimum(tile_min, fxyz_ifloor.min(0)) + tile_max = np.maximum(tile_max, fxyz_iceil) del idx, fxyz # 1a) correct for origin displacement @@ -3662,7 +3664,14 @@ def within_inf( # Make sure that full_geom doesn't return coordinates outside the unit cell # for non periodic directions - nsc = full_geom.nsc.copy() + nsc = full_geom.nsc.copy() // 2 + + # If we have atoms outside the primary unit-cell in the original + # cell, then we should consider an nsc large enough to encompass this + nsc = np.maximum(nsc, fxyz_iceil) + nsc = np.maximum(nsc, -fxyz_ifloor.min(0)) + nsc = nsc * 2 + 1 + nsc[non_periodic] = 1 full_geom.set_nsc(nsc) @@ -3695,7 +3704,7 @@ def within_inf( # Convert indices to unit-cell indices and also return coordinates and # infinite supercell indices ia = self.asc2uc(idx) - return ia, xyz, isc - fxyz_floor[ia] + return ia, xyz, isc - fxyz_ifloor[ia] def _orbital_values( self, grid_shape: tuple[int, int, int], truncate_with_nsc: bool = False @@ -3736,10 +3745,6 @@ def _orbital_values( IA, XYZ, ISC = self.within_inf(lattice, periodic=self.pbc) XYZ -= self.lattice.origin.reshape(1, 3) - # within_inf translates atoms to the unit cell to compute - # supercell indices. Here we revert that - ISC -= np.floor(self.fxyz[IA]).astype(int32) - # Don't consider atoms that are outside of the geometry's auxiliary cell. if truncate_with_nsc: mask = (abs(ISC) <= self.nsc // 2).all(axis=1) diff --git a/src/sisl/_core/tests/test_geometry.py b/src/sisl/_core/tests/test_geometry.py index 257b9583b8..081cba296a 100644 --- a/src/sisl/_core/tests/test_geometry.py +++ b/src/sisl/_core/tests/test_geometry.py @@ -982,6 +982,22 @@ def test_within_inf_duplicates(self, setup): lattice_3x3 = g.lattice.tile(3, 0).tile(3, 1) assert len(g.within_inf(lattice_3x3)[0]) == 25 + def test_within_inf_gh649(self): + # see https://github.com/zerothi/sisl/issues/649 + + # Create a geometry with an atom outside of the unit cell + geometry = Geometry([-0.5, 0, 0], lattice=np.diag([2, 10, 10])) + + search = Lattice(np.diag([3, 10, 10])) + ia, xyz, isc = geometry.within_inf(search, periodic=True) + assert np.allclose(ia, 0) + assert np.allclose(isc, [1, 0, 0]) + + search = Lattice(np.diag([2, 10, 10])) + ia, xyz, isc = geometry.within_inf(search, periodic=True) + assert np.allclose(ia, 0) + assert np.allclose(isc, [1, 0, 0]) + def test_close_sizes(self, setup): point = 0