Clarify argument shift for SlabGenerator.get_slab

pymatgen/core/surface.py

@@ -62,12 +62,12 @@
 
 class Slab(Structure):
     """Class to hold information for a Slab, with additional
-    attributes pertaining to slabs, but the init method does not
-    actually create a slab. Also has additional methods that returns other information
-    about a Slab such as the surface area, normal, and atom adsorption.
+    attributes pertaining to slabs, but does not actually create a slab.
+    Also has additional methods for a Slab such as the surface area,
+    normal, and adsorbate atoms.
 
-    Note that all Slabs have the surface normal oriented perpendicular to the a
-    and b lattice vectors. This means the lattice vectors a and b are in the
+    Note that all Slabs have the surface normal oriented perpendicular to the
+    a and b lattice vectors. This means the lattice vectors a and b are in the
     surface plane and the c vector is out of the surface plane (though not
     necessarily perpendicular to the surface).
     """
@@ -214,14 +214,52 @@ def surface_area(self) -> float:
         matrix = self.lattice.matrix
         return np.linalg.norm(np.cross(matrix[0], matrix[1]))
 
+    def is_symmetric(self, symprec: float = 0.1) -> bool:
+        """Check if Slab is symmetric, i.e., contains inversion, mirror on (hkl) plane,
+            or screw axis (rotation and translation) about [hkl].
+
+        Args:
+            symprec (float): Symmetry precision used for SpaceGroup analyzer.
+
+        Returns:
+            bool: Whether surfaces are symmetric.
+        """
+        spg_analyzer = SpacegroupAnalyzer(self, symprec=symprec)
+        symm_ops = spg_analyzer.get_point_group_operations()
+
+        # Check for inversion symmetry. Or if sites from surface (a) can be translated
+        # to surface (b) along the [hkl]-axis, surfaces are symmetric. Or because the
+        # two surfaces of our slabs are always parallel to the (hkl) plane,
+        # any operation where there's an (hkl) mirror plane has surface symmetry
+        return (
+            spg_analyzer.is_laue()
+            or any(op.translation_vector[2] != 0 for op in symm_ops)
+            or any(np.all(op.rotation_matrix[2] == np.array([0, 0, -1])) for op in symm_ops)
+        )
+
+    def is_polar(self, tol_dipole_per_unit_area: float = 1e-3) -> bool:
+        """Check if the Slab is polar by computing the normalized dipole per unit area.
+        Normalized dipole per unit area is used as it is more reliable than
+        using the absolute value, which varies with surface area.
+
+        Note that the Slab must be oxidation state decorated for this to work properly.
+        Otherwise, the Slab will always have a dipole moment of 0.
+
+        Args:
+            tol_dipole_per_unit_area (float): A tolerance above which the Slab is
+                considered polar.
+        """
+        dip_per_unit_area = self.dipole / self.surface_area
+        return np.linalg.norm(dip_per_unit_area) > tol_dipole_per_unit_area
+
     @classmethod
     def from_dict(cls, dct: dict[str, Any]) -> Self:  # type: ignore[override]
         """
         Args:
             dct: dict.
 
         Returns:
-            Creates slab from dict.
+            Slab: Created from dict.
         """
         lattice = Lattice.from_dict(dct["lattice"])
         sites = [PeriodicSite.from_dict(sd, lattice) for sd in dct["sites"]]
@@ -279,44 +317,6 @@ def copy(self, site_properties: dict[str, Any] | None = None) -> Slab:  # type:
             reorient_lattice=self.reorient_lattice,
         )
 
-    def is_symmetric(self, symprec: float = 0.1) -> bool:
-        """Check if Slab is symmetric, i.e., contains inversion, mirror on (hkl) plane,
-            or screw axis (rotation and translation) about [hkl].
-
-        Args:
-            symprec (float): Symmetry precision used for SpaceGroup analyzer.
-
-        Returns:
-            bool: Whether surfaces are symmetric.
-        """
-        spg_analyzer = SpacegroupAnalyzer(self, symprec=symprec)
-        symm_ops = spg_analyzer.get_point_group_operations()
-
-        # Check for inversion symmetry. Or if sites from surface (a) can be translated
-        # to surface (b) along the [hkl]-axis, surfaces are symmetric. Or because the
-        # two surfaces of our slabs are always parallel to the (hkl) plane,
-        # any operation where there's an (hkl) mirror plane has surface symmetry
-        return (
-            spg_analyzer.is_laue()
-            or any(op.translation_vector[2] != 0 for op in symm_ops)
-            or any(np.all(op.rotation_matrix[2] == np.array([0, 0, -1])) for op in symm_ops)
-        )
-
-    def is_polar(self, tol_dipole_per_unit_area: float = 1e-3) -> bool:
-        """Check if the Slab is polar by computing the normalized dipole per unit area.
-        Normalized dipole per unit area is used as it is more reliable than
-        using the absolute value, which varies with surface area.
-
-        Note that the Slab must be oxidation state decorated for this to work properly.
-        Otherwise, the Slab will always have a dipole moment of 0.
-
-        Args:
-            tol_dipole_per_unit_area (float): A tolerance above which the Slab is
-                considered polar.
-        """
-        dip_per_unit_area = self.dipole / self.surface_area
-        return np.linalg.norm(dip_per_unit_area) > tol_dipole_per_unit_area
-
     def get_surface_sites(self, tag: bool = False) -> dict[str, list]:
         """Returns the surface sites and their indices in a dictionary.
         Useful for analysis involving broken bonds and for finding adsorption sites.
@@ -329,8 +329,8 @@ def get_surface_sites(self, tag: bool = False) -> dict[str, list]:
         site as well. This will only work for single-element systems for now.
 
         Args:
-            tag (bool): Option to adds site attribute "is_surfsite" (bool)
-                to all sites of slab. Defaults to False
+            tag (bool): Add attribute "is_surf_site" (bool)
+                to all sites of the Slab. Defaults to False.
 
         Returns:
             A dictionary grouping sites on top and bottom of the slab together.
@@ -638,8 +638,8 @@ def symmetrically_add_atom(
         specie: str | Element | Species | None = None,
         coords_are_cartesian: bool = False,
     ) -> None:
-        """Add a species at a specified site in a slab. Will also add an
-        equivalent site on the other side of the slab to maintain symmetry.
+        """Add a species at a selected site in a Slab. Will also add an
+        equivalent site on the other side to maintain symmetry.
 
         TODO (@DanielYang59): use "site" over "point" as arg name for consistency
 
@@ -649,8 +649,6 @@ def symmetrically_add_atom(
             specie: Deprecated argument name in #3691. Use 'species' instead.
             coords_are_cartesian (bool): If the site is in Cartesian coordinates.
         """
-        # For now just use the species of the surface atom as the element to add
-
         # Check if deprecated argument is used
         if specie is not None:
             warnings.warn("The argument 'specie' is deprecated. Use 'species' instead.", DeprecationWarning)
@@ -1075,7 +1073,8 @@ def get_slab(self, shift: float = 0, tol: float = 0.1, energy: float | None = No
             intended for other generation methods.
 
         Args:
-            shift (float): The shift value along the lattice c direction in Angstrom.
+            shift (float): The shift value along the lattice c direction
+                in fractional coordinates.
             tol (float): Tolerance to determine primitive cell.
             energy (float): The energy to assign to the slab.
 
@@ -1104,10 +1103,8 @@ def get_slab(self, shift: float = 0, tol: float = 0.1, energy: float | None = No
         species = self.oriented_unit_cell.species_and_occu
 
         # Shift all atoms
-        # DEBUG(@DanielYang59): shift value in Angstrom inconsistent with frac_coordis
         frac_coords = self.oriented_unit_cell.frac_coords
-        # DEBUG(@DanielYang59): suspicious shift direction (positive for downwards shift)
-        frac_coords = np.array(frac_coords) + np.array([0, 0, -shift])[None, :]
+        frac_coords = np.array(frac_coords) + np.array([0, 0, shift])[None, :]
         frac_coords -= np.floor(frac_coords)  # wrap frac_coords to the [0, 1) range
 
         # Scale down z-coordinate by the number of layers
@@ -1231,8 +1228,7 @@ def gen_possible_shifts(ftol: float) -> list[float]:
 
             # Clustering does not work when there is only one atom
             if n_atoms == 1:
-                # TODO (@DanielYang59): why this magic number 0.5?
-                shift = frac_coords[0][2] + 0.5
+                shift = -frac_coords[0][2] + 0.5  # shift to center
                 return [shift - math.floor(shift)]
 
             # Compute a Cartesian z-coordinate distance matrix