diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 96eb3ef3236..02c5ce3921b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -78,6 +78,13 @@ jobs: for pkg in cmd_line/*; do echo "$(pwd)/cmd_line/$pkg/Linux_64bit" >> "$GITHUB_PATH"; done + - name: Install Bader + if: runner.os == 'Linux' + run: | + wget http://theory.cm.utexas.edu/henkelman/code/bader/download/bader_lnx_64.tar.gz + tar xvzf bader_lnx_64.tar.gz + sudo mv bader /usr/local/bin/ + continue-on-error: true - name: Install dependencies run: | python -m pip install --upgrade pip wheel diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cc856204f92..68133967a56 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -8,7 +8,7 @@ ci: repos: - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.0.279 + rev: v0.0.281 hooks: - id: ruff args: [--fix] diff --git a/pymatgen/analysis/chemenv/coordination_environments/chemenv_strategies.py b/pymatgen/analysis/chemenv/coordination_environments/chemenv_strategies.py index 7293600d4c9..3b570279805 100644 --- a/pymatgen/analysis/chemenv/coordination_environments/chemenv_strategies.py +++ b/pymatgen/analysis/chemenv/coordination_environments/chemenv_strategies.py @@ -2024,7 +2024,6 @@ def __init__( self.area_weight = self.w_area_has_intersection elif weight_type == "has_intersection_smoothstep": raise NotImplementedError - # self.area_weight = self.w_area_has_intersection_smoothstep else: raise ValueError(f'Weight type is {weight_type!r} while it should be "has_intersection"') self.surface_definition = surface_definition @@ -2064,28 +2063,6 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No additional_info=additional_info, ) - def w_area_has_intersection_smoothstep(self, nb_set, structure_environments, cn_map, additional_info): - """Get intersection of the neighbors set area with the surface. - - :param nb_set: Neighbors set. - :param structure_environments: Structure environments. - :param cn_map: Mapping index of the neighbors set. - :param additional_info: Additional information. - :return: Area intersection between neighbors set and surface. - """ - w_area = self.w_area_intersection_specific( - nb_set=nb_set, - structure_environments=structure_environments, - cn_map=cn_map, - additional_info=additional_info, - ) - if w_area > 0: - if self.smoothstep_distance is not None: - w_area = w_area - if self.smoothstep_angle is not None: - w_area = w_area - return w_area - def w_area_has_intersection(self, nb_set, structure_environments, cn_map, additional_info): """Get intersection of the neighbors set area with the surface. diff --git a/pymatgen/analysis/gb/grain.py b/pymatgen/analysis/gb/grain.py index 233556290d9..fdcc6c3659a 100644 --- a/pymatgen/analysis/gb/grain.py +++ b/pymatgen/analysis/gb/grain.py @@ -21,6 +21,7 @@ if TYPE_CHECKING: from numpy.typing import ArrayLike + from pymatgen.core.trajectory import Vector3D from pymatgen.util.typing import CompositionLike # This module implements representations of grain boundaries, as well as @@ -55,10 +56,10 @@ def __init__( lattice: np.ndarray | Lattice, species: Sequence[CompositionLike], coords: Sequence[ArrayLike], - rotation_axis: tuple[float, float, float], + rotation_axis: Vector3D, rotation_angle: float, - gb_plane: tuple[float, float, float], - join_plane: tuple[float, float, float], + gb_plane: Vector3D, + join_plane: Vector3D, init_cell: Structure, vacuum_thickness: float, ab_shift: tuple[float, float], diff --git a/pymatgen/analysis/wulff.py b/pymatgen/analysis/wulff.py index 62a779c1629..de16c0776e5 100644 --- a/pymatgen/analysis/wulff.py +++ b/pymatgen/analysis/wulff.py @@ -462,7 +462,6 @@ def get_plot( ax.set_zlim([-r_range * 1.1, r_range * 1.1]) # pylint: disable=E1101 # add legend if legend_on: - color_proxy = color_proxy if show_area: ax.legend( color_proxy, diff --git a/pymatgen/apps/battery/conversion_battery.py b/pymatgen/apps/battery/conversion_battery.py index d1d8a2ce0c8..e187382b1c8 100644 --- a/pymatgen/apps/battery/conversion_battery.py +++ b/pymatgen/apps/battery/conversion_battery.py @@ -72,7 +72,6 @@ def from_composition_and_pd(cls, comp, pd, working_ion_symbol="Li", allow_unstab profile.reverse() if len(profile) < 2: return None - working_ion_entry = working_ion_entry working_ion = working_ion_entry.composition.elements[0].symbol normalization_els = {} for el, amt in comp.items(): @@ -83,7 +82,7 @@ def from_composition_and_pd(cls, comp, pd, working_ion_symbol="Li", allow_unstab framework.pop(working_ion) framework = Composition(framework) - vpairs = [ + v_pairs = [ ConversionVoltagePair.from_steps( profile[i], profile[i + 1], @@ -94,7 +93,7 @@ def from_composition_and_pd(cls, comp, pd, working_ion_symbol="Li", allow_unstab ] return ConversionElectrode( # pylint: disable=E1123 - voltage_pairs=vpairs, + voltage_pairs=v_pairs, working_ion_entry=working_ion_entry, initial_comp_formula=comp.reduced_formula, framework_formula=framework.reduced_formula, @@ -338,26 +337,24 @@ def from_steps(cls, step1, step2, normalization_els, framework_formula): sum(curr_rxn.all_comp[i].weight * abs(curr_rxn.coeffs[i]) for i in range(len(curr_rxn.all_comp))) / 2 ) mass_charge = prev_mass_dischg - mass_discharge = mass_discharge vol_discharge = sum( abs(curr_rxn.get_coeff(e.composition)) * e.structure.volume for e in step2["entries"] if e.composition.reduced_formula != working_ion ) - totalcomp = Composition({}) + total_comp = Composition({}) for comp in prev_rxn.products: if comp.reduced_formula != working_ion: - totalcomp += comp * abs(prev_rxn.get_coeff(comp)) - frac_charge = totalcomp.get_atomic_fraction(Element(working_ion)) + total_comp += comp * abs(prev_rxn.get_coeff(comp)) + frac_charge = total_comp.get_atomic_fraction(Element(working_ion)) - totalcomp = Composition({}) + total_comp = Composition({}) for comp in curr_rxn.products: if comp.reduced_formula != working_ion: - totalcomp += comp * abs(curr_rxn.get_coeff(comp)) - frac_discharge = totalcomp.get_atomic_fraction(Element(working_ion)) + total_comp += comp * abs(curr_rxn.get_coeff(comp)) + frac_discharge = total_comp.get_atomic_fraction(Element(working_ion)) - rxn = rxn entries_charge = step1["entries"] entries_discharge = step2["entries"] diff --git a/pymatgen/command_line/bader_caller.py b/pymatgen/command_line/bader_caller.py index 0574d3e8a74..2ef1dc30245 100644 --- a/pymatgen/command_line/bader_caller.py +++ b/pymatgen/command_line/bader_caller.py @@ -23,7 +23,6 @@ from tempfile import TemporaryDirectory import numpy as np -from monty.dev import requires from monty.io import zopen from pymatgen.io.common import VolumetricData @@ -37,66 +36,28 @@ __status__ = "Beta" __date__ = "4/5/13" + BADEREXE = which("bader") or which("bader.exe") class BaderAnalysis: """ - Bader analysis for Cube files and VASP outputs. - - .. attribute: data - - Atomic data parsed from bader analysis. Essentially a list of dicts - of the form:: - - [ - { - "atomic_vol": 8.769, - "min_dist": 0.8753, - "charge": 7.4168, - "y": 1.1598, - "x": 0.0079, - "z": 0.8348 - }, - ... - ] - - .. attribute: vacuum_volume - - Vacuum volume of the Bader analysis. - - .. attribute: vacuum_charge - - Vacuum charge of the Bader analysis. - - .. attribute: nelectrons - - Number of electrons of the Bader analysis. - - .. attribute: chgcar - - Chgcar object associated with input CHGCAR file. - - .. attribute: atomic_densities - - list of charge densities for each atom centered on the atom - excess 0's are removed from the array to reduce the size of the array - the charge densities are dicts with the charge density map, - the shift vector applied to move the data to the center, and the original dimension of the charge density map - charge: - { - "data": charge density array - "shift": shift used to center the atomic charge density - "dim": dimension of the original charge density map - } + Performs Bader analysis for Cube files and VASP outputs. + + Attributes: + data (list[dict]): Atomic data parsed from bader analysis. Each dictionary in the list has the keys: + "atomic_vol", "min_dist", "charge", "x", "y", "z". + vacuum_volume (float): Vacuum volume of the Bader analysis. + vacuum_charge (float): Vacuum charge of the Bader analysis. + nelectrons (int): Number of electrons of the Bader analysis. + chgcar (Chgcar): Chgcar object associated with input CHGCAR file. + atomic_densities (list[dict]): List of charge densities for each atom centered on the atom. + Excess 0's are removed from the array to reduce its size. Each dictionary has the keys: + "data", "shift", "dim", where "data" is the charge density array, + "shift" is the shift used to center the atomic charge density, and + "dim" is the dimension of the original charge density map. """ - @requires( - which("bader") or which("bader.exe"), - "BaderAnalysis requires the executable bader to be in the path." - " Please download the library at http://theory.cm.utexas" - ".edu/vasp/bader/ and compile the executable.", - ) def __init__( self, chgcar_filename=None, @@ -104,6 +65,7 @@ def __init__( chgref_filename=None, parse_atomic_densities=False, cube_filename=None, + bader_exe_path: str | None = BADEREXE, ): """ Initializes the Bader caller. @@ -112,16 +74,18 @@ def __init__( chgcar_filename (str): The filename of the CHGCAR. potcar_filename (str): The filename of the POTCAR. chgref_filename (str): The filename of the reference charge density. - parse_atomic_densities (bool): Optional. turns on atomic partition of the charge density + parse_atomic_densities (bool, optional): turns on atomic partition of the charge density charge densities are atom centered - cube_filename (str): Optional. The filename of the cube file. + cube_filename (str, optional): The filename of the cube file. + bader_exe_path (str, optional): The path to the bader executable. """ - if not BADEREXE: + if not BADEREXE and not os.path.isfile(bader_exe_path or ""): raise RuntimeError( - "BaderAnalysis requires the executable bader to be in the path." - " Please download the library at http://theory.cm.utexas" - ".edu/vasp/bader/ and compile the executable." + "BaderAnalysis requires the executable bader be in the PATH or the full path " + f"to the binary to be specified via {bader_exe_path=}. Download the binary at " + "https://theory.cm.utexas.edu/henkelman/code/bader." ) + assert isinstance(BADEREXE, str) # mypy type narrowing if not (cube_filename or chgcar_filename): raise ValueError("You must provide either a cube file or a CHGCAR") @@ -136,7 +100,7 @@ def __init__( self.structure = self.chgcar.structure self.potcar = Potcar.from_file(potcar_filename) if potcar_filename is not None else None self.natoms = self.chgcar.poscar.natoms - chgrefpath = os.path.abspath(chgref_filename) if chgref_filename else None + chgref_path = os.path.abspath(chgref_filename) if chgref_filename else None self.reference_used = bool(chgref_filename) # List of nelects for each atom from potcar @@ -152,16 +116,16 @@ def __init__( self.is_vasp = False self.cube = VolumetricData.from_cube(fpath) self.structure = self.cube.structure - self.nelects = None - chgrefpath = os.path.abspath(chgref_filename) if chgref_filename else None + self.nelects = None # type: ignore + chgref_path = os.path.abspath(chgref_filename) if chgref_filename else None self.reference_used = bool(chgref_filename) tmpfile = "CHGCAR" if chgcar_filename else "CUBE" with zopen(fpath, "rt") as f_in, open(tmpfile, "w") as f_out: shutil.copyfileobj(f_in, f_out) - args = [BADEREXE, tmpfile] + args: list[str] = [BADEREXE, tmpfile] if chgref_filename: - with zopen(chgrefpath, "rt") as f_in, open("CHGCAR_ref", "w") as f_out: + with zopen(chgref_path, "rt") as f_in, open("CHGCAR_ref", "w") as f_out: shutil.copyfileobj(f_in, f_out) args += ["-ref", "CHGCAR_ref"] if parse_atomic_densities: @@ -224,35 +188,35 @@ def __init__( shift = (np.divide(chg.dim, 2) - index).astype(int) # Shift the data so that the atomic charge density to the center for easier manipulation - shifted_data = np.roll(data, shift, axis=(0, 1, 2)) + shifted_data = np.roll(data, shift, axis=(0, 1, 2)) # type: ignore # Slices a central window from the data array - def slice_from_center(data, xwidth, ywidth, zwidth): + def slice_from_center(data, x_width, y_width, z_width): x, y, z = data.shape - startx = x // 2 - (xwidth // 2) - starty = y // 2 - (ywidth // 2) - startz = z // 2 - (zwidth // 2) + start_x = x // 2 - (x_width // 2) + start_y = y // 2 - (y_width // 2) + start_z = z // 2 - (z_width // 2) return data[ - startx : startx + xwidth, - starty : starty + ywidth, - startz : startz + zwidth, + start_x : start_x + x_width, + start_y : start_y + y_width, + start_z : start_z + z_width, ] # Finds the central encompassing volume which holds all the data within a precision def find_encompassing_vol(data): total = np.sum(data) - for i in range(np.max(data.shape)): - sliced_data = slice_from_center(data, i, i, i) + for idx in range(np.max(data.shape)): + sliced_data = slice_from_center(data, idx, idx, idx) if total - np.sum(sliced_data) < 0.1: return sliced_data return None - d = { + dct = { "data": find_encompassing_vol(shifted_data), "shift": shift, "dim": self.chgcar.dim, } - atomic_densities.append(d) + atomic_densities.append(dct) self.atomic_densities = atomic_densities def get_charge(self, atom_index): @@ -519,56 +483,61 @@ def bader_analysis_from_objects(chgcar, potcar=None, aeccar0=None, aeccar2=None) :param aeccar2: (optional) Chgcar object from aeccar2 file :return: summary dict """ - with TemporaryDirectory() as tmp_dir: - if aeccar0 and aeccar2: - # construct reference file - chgref = aeccar0.linear_add(aeccar2) - chgref_path = os.path.join(tmp_dir, "CHGCAR_ref") - chgref.write_file(chgref_path) - else: - chgref_path = None - - chgcar.write_file("CHGCAR") - chgcar_path = os.path.join(tmp_dir, "CHGCAR") - - if potcar: - potcar.write_file("POTCAR") - potcar_path = os.path.join(tmp_dir, "POTCAR") - else: - potcar_path = None - - ba = BaderAnalysis( - chgcar_filename=chgcar_path, - potcar_filename=potcar_path, - chgref_filename=chgref_path, - ) - - summary = { - "min_dist": [d["min_dist"] for d in ba.data], - "charge": [d["charge"] for d in ba.data], - "atomic_volume": [d["atomic_vol"] for d in ba.data], - "vacuum_charge": ba.vacuum_charge, - "vacuum_volume": ba.vacuum_volume, - "reference_used": bool(chgref_path), - "bader_version": ba.version, - } + orig_dir = os.getcwd() + try: + with TemporaryDirectory() as tmp_dir: + os.chdir(tmp_dir) + if aeccar0 and aeccar2: + # construct reference file + chgref = aeccar0.linear_add(aeccar2) + chgref_path = os.path.join(tmp_dir, "CHGCAR_ref") + chgref.write_file(chgref_path) + else: + chgref_path = None - if potcar: - charge_transfer = [ba.get_charge_transfer(i) for i in range(len(ba.data))] - summary["charge_transfer"] = charge_transfer + chgcar.write_file("CHGCAR") + chgcar_path = os.path.join(tmp_dir, "CHGCAR") - if chgcar.is_spin_polarized: - # write a CHGCAR containing magnetization density only - chgcar.data["total"] = chgcar.data["diff"] - chgcar.is_spin_polarized = False - chgcar.write_file("CHGCAR_mag") + if potcar: + potcar.write_file("POTCAR") + potcar_path = os.path.join(tmp_dir, "POTCAR") + else: + potcar_path = None - chgcar_mag_path = os.path.join(tmp_dir, "CHGCAR_mag") ba = BaderAnalysis( - chgcar_filename=chgcar_mag_path, + chgcar_filename=chgcar_path, potcar_filename=potcar_path, chgref_filename=chgref_path, ) - summary["magmom"] = [d["charge"] for d in ba.data] - return summary + summary = { + "min_dist": [d["min_dist"] for d in ba.data], + "charge": [d["charge"] for d in ba.data], + "atomic_volume": [d["atomic_vol"] for d in ba.data], + "vacuum_charge": ba.vacuum_charge, + "vacuum_volume": ba.vacuum_volume, + "reference_used": bool(chgref_path), + "bader_version": ba.version, + } + + if potcar: + charge_transfer = [ba.get_charge_transfer(i) for i in range(len(ba.data))] + summary["charge_transfer"] = charge_transfer + + if chgcar.is_spin_polarized: + # write a CHGCAR containing magnetization density only + chgcar.data["total"] = chgcar.data["diff"] + chgcar.is_spin_polarized = False + chgcar.write_file("CHGCAR_mag") + + chgcar_mag_path = os.path.join(tmp_dir, "CHGCAR_mag") + ba = BaderAnalysis( + chgcar_filename=chgcar_mag_path, + potcar_filename=potcar_path, + chgref_filename=chgref_path, + ) + summary["magmom"] = [d["charge"] for d in ba.data] + finally: + os.chdir(orig_dir) + + return summary diff --git a/pymatgen/command_line/tests/test_bader_caller.py b/pymatgen/command_line/tests/test_bader_caller.py index 4324c13cb11..57652b38534 100644 --- a/pymatgen/command_line/tests/test_bader_caller.py +++ b/pymatgen/command_line/tests/test_bader_caller.py @@ -3,15 +3,18 @@ import os import unittest import warnings +from shutil import which +from unittest.mock import patch import numpy as np +import pytest from pytest import approx -from pymatgen.command_line.bader_caller import BaderAnalysis, bader_analysis_from_path, which +from pymatgen.command_line.bader_caller import BaderAnalysis, bader_analysis_from_path from pymatgen.util.testing import PymatgenTest -@unittest.skipIf(not which("bader"), "bader executable not present.") +@unittest.skipIf(not which("bader"), "bader executable not present") class BaderAnalysisTest(unittest.TestCase): _multiprocess_shared_ = True @@ -25,9 +28,9 @@ def tearDown(self): def test_init(self): # test with reference file analysis = BaderAnalysis( - chgcar_filename=os.path.join(PymatgenTest.TEST_FILES_DIR, "CHGCAR.Fe3O4"), - potcar_filename=os.path.join(PymatgenTest.TEST_FILES_DIR, "POTCAR.Fe3O4"), - chgref_filename=os.path.join(PymatgenTest.TEST_FILES_DIR, "CHGCAR.Fe3O4_ref"), + chgcar_filename=f"{PymatgenTest.TEST_FILES_DIR}/CHGCAR.Fe3O4", + potcar_filename=f"{PymatgenTest.TEST_FILES_DIR}/POTCAR.Fe3O4", + chgref_filename=f"{PymatgenTest.TEST_FILES_DIR}/CHGCAR.Fe3O4_ref", ) assert len(analysis.data) == 14 assert analysis.data[0]["charge"] == approx(6.6136782, abs=1e-3) @@ -65,7 +68,7 @@ def test_init(self): assert len(analysis.data) == 9 def test_from_path(self): - test_dir = os.path.join(PymatgenTest.TEST_FILES_DIR, "bader") + test_dir = f"{PymatgenTest.TEST_FILES_DIR}/bader" analysis = BaderAnalysis.from_path(test_dir) chgcar = os.path.join(test_dir, "CHGCAR.gz") chgref = os.path.join(test_dir, "_CHGCAR_sum.gz") @@ -77,9 +80,8 @@ def test_from_path(self): os.remove("CHGREF") def test_automatic_runner(self): - test_dir = os.path.join(PymatgenTest.TEST_FILES_DIR, "bader") - - summary = bader_analysis_from_path(test_dir) + pytest.skip("raises RuntimeError: bader exited with return code 24") + summary = bader_analysis_from_path(f"{PymatgenTest.TEST_FILES_DIR}/bader") """ Reference summary dict (with bader 1.0) summary_ref = { @@ -113,9 +115,9 @@ def test_automatic_runner(self): def test_atom_parsing(self): # test with reference file analysis = BaderAnalysis( - chgcar_filename=os.path.join(PymatgenTest.TEST_FILES_DIR, "CHGCAR.Fe3O4"), - potcar_filename=os.path.join(PymatgenTest.TEST_FILES_DIR, "POTCAR.Fe3O4"), - chgref_filename=os.path.join(PymatgenTest.TEST_FILES_DIR, "CHGCAR.Fe3O4_ref"), + chgcar_filename=f"{PymatgenTest.TEST_FILES_DIR}/CHGCAR.Fe3O4", + potcar_filename=f"{PymatgenTest.TEST_FILES_DIR}/POTCAR.Fe3O4", + chgref_filename=f"{PymatgenTest.TEST_FILES_DIR}/CHGCAR.Fe3O4_ref", parse_atomic_densities=True, ) @@ -124,3 +126,11 @@ def test_atom_parsing(self): assert np.sum(analysis.chgcar.data["total"]) == approx( np.sum([np.sum(d["data"]) for d in analysis.atomic_densities]) ) + + def test_missing_file_bader_exe_path(self): + pytest.skip("doesn't reliably raise RuntimeError") + # mock which("bader") to return None so we always fall back to use bader_exe_path + with patch("shutil.which", return_value=None), pytest.raises( + RuntimeError, match="BaderAnalysis requires the executable bader be in the PATH or the full path " + ): + BaderAnalysis(chgcar_filename=f"{PymatgenTest.TEST_FILES_DIR}/CHGCAR.Fe3O4", bader_exe_path="") diff --git a/pymatgen/command_line/tests/test_gulp_caller.py b/pymatgen/command_line/tests/test_gulp_caller.py index b32a30a1fb4..14179653af8 100644 --- a/pymatgen/command_line/tests/test_gulp_caller.py +++ b/pymatgen/command_line/tests/test_gulp_caller.py @@ -56,7 +56,6 @@ def test_run(self): gin += "buck\n" gin += "Mg core O shel 946.627 0.31813 0.00000 0.0 10.0\n" gin += "O shel O shel 22764.000 0.14900 27.87900 0.0 12.0\n" - gin = gin gc = GulpCaller() """Some inherent checks are in the run_gulp function itself. diff --git a/pymatgen/core/lattice.py b/pymatgen/core/lattice.py index 558fcb1921c..02c7aade431 100644 --- a/pymatgen/core/lattice.py +++ b/pymatgen/core/lattice.py @@ -23,6 +23,8 @@ if TYPE_CHECKING: from numpy.typing import ArrayLike + from pymatgen.core.trajectory import Vector3D + __author__ = "Shyue Ping Ong, Michael Kocher" __copyright__ = "Copyright 2011, The Materials Project" __maintainer__ = "Shyue Ping Ong" @@ -67,7 +69,7 @@ def __init__(self, matrix: ArrayLike, pbc: tuple[bool, bool, bool] = (True, True self._pbc = tuple(pbc) @property - def lengths(self) -> tuple[float, float, float]: + def lengths(self) -> Vector3D: """ Lattice lengths. @@ -76,7 +78,7 @@ def lengths(self) -> tuple[float, float, float]: return tuple(np.sqrt(np.sum(self._matrix**2, axis=1)).tolist()) # type: ignore @property - def angles(self) -> tuple[float, float, float]: + def angles(self) -> Vector3D: """ Lattice angles. @@ -414,7 +416,7 @@ def c(self) -> float: return self.lengths[2] @property - def abc(self) -> tuple[float, float, float]: + def abc(self) -> Vector3D: """Lengths of the lattice vectors, i.e. (a, b, c).""" return self.lengths diff --git a/pymatgen/core/sites.py b/pymatgen/core/sites.py index 4b67425b368..cab0ab50160 100644 --- a/pymatgen/core/sites.py +++ b/pymatgen/core/sites.py @@ -71,7 +71,7 @@ def __init__( self._species: Composition = species # type: ignore self.coords: np.ndarray = coords # type: ignore self.properties: dict = properties or {} - self.label = label if label else self.species_string + self._label = label def __getattr__(self, attr): # overriding getattr doesn't play nicely with pickle, so we can't use self._properties @@ -86,7 +86,7 @@ def species(self) -> Composition: return self._species @species.setter - def species(self, species: SpeciesLike | CompositionLike): + def species(self, species: SpeciesLike | CompositionLike) -> None: if not isinstance(species, Composition): try: species = Composition({get_el_sp(species): 1}) # type: ignore @@ -97,13 +97,22 @@ def species(self, species: SpeciesLike | CompositionLike): raise ValueError("Species occupancies sum to more than 1!") self._species = species + @property + def label(self) -> str: + """Site label.""" + return self._label if self._label is not None else self.species_string + + @label.setter + def label(self, label: str) -> None: + self._label = label + @property def x(self) -> float: """Cartesian x coordinate.""" return self.coords[0] @x.setter - def x(self, x: float): + def x(self, x: float) -> None: self.coords[0] = x @property @@ -112,7 +121,7 @@ def y(self) -> float: return self.coords[1] @y.setter - def y(self, y: float): + def y(self, y: float) -> None: self.coords[1] = y @property @@ -121,7 +130,7 @@ def z(self) -> float: return self.coords[2] @z.setter - def z(self, z: float): + def z(self, z: float) -> None: self.coords[2] = z def distance(self, other) -> float: @@ -345,7 +354,7 @@ def __init__( self._species: Composition = species # type: ignore self._coords: np.ndarray | None = None self.properties: dict = properties or {} - self.label = label if label else self.species_string + self._label = label def __hash__(self) -> int: """ @@ -360,7 +369,7 @@ def lattice(self) -> Lattice: return self._lattice @lattice.setter - def lattice(self, lattice: Lattice): + def lattice(self, lattice: Lattice) -> None: """Sets Lattice associated with PeriodicSite.""" self._lattice = lattice self._coords = self._lattice.get_cartesian_coords(self._frac_coords) @@ -373,7 +382,7 @@ def coords(self) -> np.ndarray: return self._coords @coords.setter - def coords(self, coords): + def coords(self, coords) -> None: """Set Cartesian coordinates.""" self._coords = np.array(coords) self._frac_coords = self._lattice.get_fractional_coords(self._coords) @@ -384,7 +393,7 @@ def frac_coords(self) -> np.ndarray: return self._frac_coords @frac_coords.setter - def frac_coords(self, frac_coords): + def frac_coords(self, frac_coords) -> None: """Set fractional coordinates.""" self._frac_coords = np.array(frac_coords) self._coords = self._lattice.get_cartesian_coords(self._frac_coords) @@ -395,7 +404,7 @@ def a(self) -> float: return self._frac_coords[0] @a.setter - def a(self, a: float): + def a(self, a: float) -> None: self._frac_coords[0] = a self._coords = self._lattice.get_cartesian_coords(self._frac_coords) @@ -405,7 +414,7 @@ def b(self) -> float: return self._frac_coords[1] @b.setter - def b(self, b: float): + def b(self, b: float) -> None: self._frac_coords[1] = b self._coords = self._lattice.get_cartesian_coords(self._frac_coords) @@ -415,7 +424,7 @@ def c(self) -> float: return self._frac_coords[2] @c.setter - def c(self, c: float): + def c(self, c: float) -> None: self._frac_coords[2] = c self._coords = self._lattice.get_cartesian_coords(self._frac_coords) @@ -425,7 +434,7 @@ def x(self) -> float: return self.coords[0] @x.setter - def x(self, x: float): + def x(self, x: float) -> None: self.coords[0] = x self._frac_coords = self._lattice.get_fractional_coords(self.coords) @@ -435,7 +444,7 @@ def y(self) -> float: return self.coords[1] @y.setter - def y(self, y: float): + def y(self, y: float) -> None: self.coords[1] = y self._frac_coords = self._lattice.get_fractional_coords(self.coords) @@ -445,7 +454,7 @@ def z(self) -> float: return self.coords[2] @z.setter - def z(self, z: float): + def z(self, z: float) -> None: self.coords[2] = z self._frac_coords = self._lattice.get_fractional_coords(self.coords) @@ -455,7 +464,7 @@ def to_unit_cell(self, in_place=False) -> PeriodicSite | None: if in_place: self.frac_coords = np.array(frac_coords) return None - return PeriodicSite(self.species, frac_coords, self.lattice, properties=self.properties) + return PeriodicSite(self.species, frac_coords, self.lattice, properties=self.properties, label=self.label) def is_periodic_image(self, other: PeriodicSite, tolerance: float = 1e-8, check_lattice: bool = True) -> bool: """ diff --git a/pymatgen/core/structure.py b/pymatgen/core/structure.py index 32b65441f56..c8d09cc1ecf 100644 --- a/pymatgen/core/structure.py +++ b/pymatgen/core/structure.py @@ -84,7 +84,7 @@ def __init__( self.properties = properties or {} self.nn_distance = nn_distance self.index = index - self.label = label if label is not None else self.species_string + self._label = label def __len__(self) -> Literal[3]: """Make neighbor Tuple-like to retain backwards compatibility.""" @@ -157,7 +157,7 @@ def __init__( self.nn_distance = nn_distance self.index = index self.image = image - self.label = label if label is not None else self.species_string + self._label = label @property # type: ignore def coords(self) -> np.ndarray: # type: ignore @@ -858,7 +858,7 @@ def __init__( to_unit_cell: bool = False, coords_are_cartesian: bool = False, site_properties: dict | None = None, - labels: list | None = None, + labels: Sequence[str | None] | None = None, ) -> None: """ Create a periodic structure. @@ -988,6 +988,7 @@ def from_spacegroup( site_properties: dict[str, Sequence] | None = None, coords_are_cartesian: bool = False, tol: float = 1e-5, + labels: Sequence[str | None] | None = None, ) -> IStructure | Structure: """ Generate a structure using a spacegroup. Note that only symmetrically @@ -1027,6 +1028,10 @@ def from_spacegroup( fractional_coords. Defaults to None for no properties. tol (float): A fractional tolerance to deal with numerical precision issues in determining if orbits are the same. + labels (list[str]): Labels associated with the sites as a + list of strings, e.g. ['Li1', 'Li2']. Must have the same + length as the species and fractional coords. Defaults to + None for no labels. """ from pymatgen.symmetry.groups import SpaceGroup @@ -1056,10 +1061,13 @@ def from_spacegroup( all_sp: list[str | Element | Species | DummySpecies | Composition] = [] all_coords: list[list[float]] = [] all_site_properties: dict[str, list] = collections.defaultdict(list) + all_labels: list[str | None] = [] for idx, (sp, c) in enumerate(zip(species, frac_coords)): cc = spg.get_orbit(c, tol=tol) all_sp.extend([sp] * len(cc)) all_coords.extend(cc) # type: ignore + label = labels[idx] if labels else None + all_labels.extend([label] * len(cc)) for k, v in props.items(): all_site_properties[k].extend([v[idx]] * len(cc)) @@ -1075,6 +1083,7 @@ def from_magnetic_spacegroup( site_properties: dict[str, Sequence], coords_are_cartesian: bool = False, tol: float = 1e-5, + labels: Sequence[str | None] | None = None, ) -> IStructure | Structure: """ Generate a structure using a magnetic spacegroup. Note that only @@ -1119,6 +1128,10 @@ def from_magnetic_spacegroup( coordinates in Cartesian coordinates. Defaults to False. tol (float): A fractional tolerance to deal with numerical precision issues in determining if orbits are the same. + labels (list[str]): Labels associated with the sites as a + list of strings, e.g. ['Li1', 'Li2']. Must have the same + length as the species and fractional coords. Defaults to + None for no labels. Returns: Structure | IStructure @@ -1151,18 +1164,21 @@ def from_magnetic_spacegroup( all_coords: list[list[float]] = [] all_magmoms: list[float] = [] all_site_properties: dict[str, list] = collections.defaultdict(list) - for i, (sp, c, m) in enumerate(zip(species, frac_coords, magmoms)): # type: ignore + all_labels: list[str | None] = [] + for idx, (sp, c, m) in enumerate(zip(species, frac_coords, magmoms)): # type: ignore cc, mm = msg.get_orbit(c, m, tol=tol) all_sp.extend([sp] * len(cc)) all_coords.extend(cc) all_magmoms.extend(mm) + label = labels[idx] if labels else None + all_labels.extend([label] * len(cc)) for k, v in site_properties.items(): if k != "magmom": - all_site_properties[k].extend([v[i]] * len(cc)) + all_site_properties[k].extend([v[idx]] * len(cc)) all_site_properties["magmom"] = all_magmoms - return cls(latt, all_sp, all_coords, site_properties=all_site_properties) + return cls(latt, all_sp, all_coords, site_properties=all_site_properties, labels=all_labels) def unset_charge(self): """Reset the charge to None, i.e., computed dynamically based on oxidation states.""" @@ -1319,6 +1335,7 @@ def __mul__(self, scaling_matrix: int | Sequence[int] | Sequence[Sequence[int]]) coords_are_cartesian=True, to_unit_cell=False, skip_checks=True, + label=site.label, ) new_sites.append(s) @@ -1405,6 +1422,7 @@ def get_sites_in_sphere( nn_distance=dist, image=img, # type: ignore index=i, + label=self[i].label, ) neighbors.append(nn_site) return neighbors @@ -1792,6 +1810,7 @@ def get_all_neighbors( nn_distance=d, index=pindex, image=tuple(image), + label=psite.label, ) ) @@ -2184,7 +2203,7 @@ def interpolate( else: lat = self.lattice fcoords = start_coords + x * vec - structs.append(self.__class__(lat, sp, fcoords, site_properties=self.site_properties)) + structs.append(self.__class__(lat, sp, fcoords, site_properties=self.site_properties, labels=self.labels)) return structs def get_miller_index_from_site_indexes(self, site_ids, round_dp=4, verbose=True): @@ -2924,7 +2943,8 @@ def __init__( prop = None if site_properties: prop = {k: v[idx] for k, v in site_properties.items()} - sites.append(Site(species[idx], coords[idx], properties=prop)) + label = labels[idx] if labels else None + sites.append(Site(species[idx], coords[idx], properties=prop, label=label)) self._sites = tuple(sites) if validate_proximity and not self.is_valid(): @@ -3208,7 +3228,7 @@ def get_sites_in_sphere(self, pt: ArrayLike, r: float) -> list[Neighbor]: for i, site in enumerate(self._sites): dist = site.distance_from_point(pt) if dist <= r: - neighbors.append(Neighbor(site.species, site.coords, site.properties, dist, i)) + neighbors.append(Neighbor(site.species, site.coords, site.properties, dist, i, label=site.label)) return neighbors def get_neighbors(self, site: Site, r: float) -> list[Neighbor]: @@ -3297,7 +3317,7 @@ def get_boxed_structure( if a <= x_range or b <= y_range or c <= z_range: raise ValueError("Box is not big enough to contain Molecule.") lattice = Lattice.from_parameters(a * images[0], b * images[1], c * images[2], 90, 90, 90) # type: ignore - nimages = images[0] * images[1] * images[2] # type: ignore + nimages: int = images[0] * images[1] * images[2] # type: ignore all_coords: list[ArrayLike] = [] centered_coords = self.cart_coords - self.center_of_mass + offset @@ -3346,18 +3366,20 @@ def get_boxed_structure( if reorder: return cls( lattice, - self.species * nimages, # type: ignore + self.species * nimages, all_coords, coords_are_cartesian=True, site_properties=sprops, + labels=self.labels * nimages, ).get_sorted_structure() return cls( lattice, - self.species * nimages, # type: ignore + self.species * nimages, coords, coords_are_cartesian=True, site_properties=sprops, + labels=self.labels * nimages, ) def get_centered_molecule(self) -> IMolecule | Molecule: @@ -3376,6 +3398,7 @@ def get_centered_molecule(self) -> IMolecule | Molecule: spin_multiplicity=self._spin_multiplicity, site_properties=self.site_properties, charge_spin_check=self._charge_spin_check, + labels=self.labels, ) def to(self, filename: str = "", fmt: str = "") -> str | None: @@ -3526,7 +3549,7 @@ def __init__( to_unit_cell: bool = False, coords_are_cartesian: bool = False, site_properties: dict | None = None, - labels: list | None = None, + labels: Sequence[str | None] | None = None, ): """ Create a periodic structure. @@ -3703,6 +3726,7 @@ def insert( # type: ignore coords_are_cartesian: bool = False, validate_proximity: bool = False, properties: dict | None = None, + label: str | None = None, ): """ Insert a site to the structure. @@ -3716,15 +3740,16 @@ def insert( # type: ignore validate_proximity (bool): Whether to check if inserted site is too close to an existing site. Defaults to False. properties (dict): Properties associated with the site. + label (str): Label associated with the site. Returns: New structure with inserted site. """ if not coords_are_cartesian: - new_site = PeriodicSite(species, coords, self._lattice, properties=properties) + new_site = PeriodicSite(species, coords, self._lattice, properties=properties, label=label) else: frac_coords = self._lattice.get_fractional_coords(coords) - new_site = PeriodicSite(species, frac_coords, self._lattice, properties=properties) + new_site = PeriodicSite(species, frac_coords, self._lattice, properties=properties, label=label) if validate_proximity: for site in self: @@ -3740,6 +3765,7 @@ def replace( coords: ArrayLike | None = None, coords_are_cartesian: bool = False, properties: dict | None = None, + label: str | None = None, ): """ Replace a single site. Takes either a species or a dict of species and @@ -3753,6 +3779,7 @@ def replace( coords_are_cartesian (bool): Whether coordinates are cartesian. Defaults to False. properties (dict): Properties associated with the site. + label (str): Label associated with the site. """ if coords is None: frac_coords = self[i].frac_coords @@ -3761,7 +3788,7 @@ def replace( else: frac_coords = coords # type: ignore - new_site = PeriodicSite(species, frac_coords, self._lattice, properties=properties) + new_site = PeriodicSite(species, frac_coords, self._lattice, properties=properties, label=label) self._sites[i] = new_site def substitute(self, index: int, func_group: IMolecule | Molecule | str, bond_order: int = 1) -> None: @@ -3856,7 +3883,7 @@ def substitute(self, index: int, func_group: IMolecule | Molecule | str, bond_or # group. del self[index] for site in fgroup[1:]: - s_new = PeriodicSite(site.species, site.coords, self.lattice, coords_are_cartesian=True) + s_new = PeriodicSite(site.species, site.coords, self.lattice, coords_are_cartesian=True, label=site.label) self._sites.append(s_new) def remove_species(self, species: Sequence[SpeciesLike]) -> None: @@ -3878,6 +3905,7 @@ def remove_species(self, species: Sequence[SpeciesLike]) -> None: site.frac_coords, self._lattice, properties=site.properties, + label=site.label, ) ) self._sites = new_sites @@ -3918,6 +3946,7 @@ def operate_site(site): self._lattice, properties=site.properties, skip_checks=True, + label=site.label, ) else: @@ -3931,6 +3960,7 @@ def operate_site(site): self._lattice, properties=site.properties, skip_checks=True, + label=site.label, ) self._sites = [operate_site(s) for s in self._sites] @@ -4048,6 +4078,7 @@ def rotate_sites( coords_are_cartesian=True, properties=site.properties, skip_checks=True, + label=site.label, ) self._sites[idx] = new_site @@ -4447,6 +4478,7 @@ def insert( # type: ignore coords: ArrayLike, validate_proximity: bool = False, properties: dict | None = None, + label: str | None = None, ) -> Molecule: """ Insert a site to the molecule. @@ -4458,11 +4490,12 @@ def insert( # type: ignore validate_proximity (bool): Whether to check if inserted site is too close to an existing site. Defaults to True. properties (dict): Dict of properties for the Site. + label (str): Label of inserted site Returns: New molecule with inserted site. """ - new_site = Site(species, coords, properties=properties) + new_site = Site(species, coords, properties=properties, label=label) if validate_proximity: for site in self: if site.distance(new_site) < self.DISTANCE_TOLERANCE: @@ -4483,7 +4516,7 @@ def remove_species(self, species: Sequence[SpeciesLike]): for site in self._sites: new_sp_occu = {sp: amt for sp, amt in site.species.items() if sp not in species} if len(new_sp_occu) > 0: - new_sites.append(Site(new_sp_occu, site.coords, properties=site.properties)) + new_sites.append(Site(new_sp_occu, site.coords, properties=site.properties, label=site.label)) self._sites = new_sites def remove_sites(self, indices: Sequence[int]): @@ -4511,7 +4544,7 @@ def translate_sites(self, indices: Sequence[int] | None = None, vector: ArrayLik vector = [0, 0, 0] for i in indices: site = self._sites[i] - new_site = Site(site.species, site.coords + vector, properties=site.properties) + new_site = Site(site.species, site.coords + vector, properties=site.properties, label=site.label) self._sites[i] = new_site def rotate_sites( @@ -4554,7 +4587,7 @@ def rotate_sites( for idx in indices: site = self._sites[idx] s = ((np.dot(rm, (site.coords - anchor).T)).T + anchor).ravel() - new_site = Site(site.species, s, properties=site.properties) + new_site = Site(site.species, s, properties=site.properties, label=site.label) self._sites[idx] = new_site def perturb(self, distance: float): @@ -4586,7 +4619,7 @@ def apply_operation(self, symmop: SymmOp): def operate_site(site): new_cart = symmop.operate(site.coords) - return Site(site.species, new_cart, properties=site.properties) + return Site(site.species, new_cart, properties=site.properties, label=site.label) self._sites = [operate_site(site) for site in self._sites] diff --git a/pymatgen/core/tests/test_structure.py b/pymatgen/core/tests/test_structure.py index 9a7fe68f6b0..6cba4157248 100644 --- a/pymatgen/core/tests/test_structure.py +++ b/pymatgen/core/tests/test_structure.py @@ -6,7 +6,6 @@ import warnings from pathlib import Path from shutil import which -from tempfile import TemporaryDirectory from unittest import skipIf import numpy as np @@ -57,10 +56,10 @@ def test_neighbor_labels(self): comp = Composition("C") for label in (None, "", "str label", ("tuple", "label")): neighbor = Neighbor(comp, (0, 0, 0), label=label) - assert neighbor.label == label if label is not None else str(comp) + assert neighbor.label == label if label is not None else "C" p_neighbor = PeriodicNeighbor(comp, (0, 0, 0), (10, 10, 10), label=label) - assert p_neighbor.label == label if label is not None else str(comp) + assert p_neighbor.label == label if label is not None else "C" class IStructureTest(PymatgenTest): @@ -777,6 +776,7 @@ def setUp(self): self.structure = Structure(lattice, ["Si", "Si"], coords) self.cu_structure = Structure(lattice, ["Cu", "Cu"], coords) self.disordered = Structure.from_spacegroup("Im-3m", Lattice.cubic(3), [Composition("Fe0.5Mn0.5")], [[0, 0, 0]]) + self.labeled_structure = Structure(lattice, ["Si", "Si"], coords, labels=["Si1", "Si2"]) def test_mutable_sequence_methods(self): struct = self.structure @@ -1064,6 +1064,11 @@ def test_make_supercell(self): assert self.structure.formula == "Si32" self.assert_all_close(self.structure.lattice.abc, [15.360792, 35.195996, 7.680396], 5) + def test_make_supercell_labeled(self): + struct = self.labeled_structure.copy() + struct.make_supercell([1, 1, 2]) + assert set(struct.labels) == {"Si1", "Si2"} + def test_disordered_supercell_primitive_cell(self): latt = Lattice.cubic(2) coords = [[0.5, 0.5, 0.5]] @@ -1467,8 +1472,7 @@ def test_relax_ase_return_traj(self): def test_relax_ase_opt_kwargs(self): pytest.importorskip("ase") structure = self.cu_structure - tmp_dir = TemporaryDirectory() - traj_file = f"{tmp_dir.name}/testing.traj" + traj_file = f"{self.tmp_path}/testing.traj" relaxed, traj = structure.relax( calculator=EMT(), fmax=0.01, steps=2, return_trajectory=True, opt_kwargs={"trajectory": traj_file} ) @@ -2005,8 +2009,7 @@ def test_relax_ase_mol(self): def test_relax_ase_mol_return_traj(self): pytest.importorskip("ase") - tmp_dir = TemporaryDirectory() - traj_file = f"{tmp_dir.name}/testing.traj" + traj_file = f"{self.tmp_path}/testing.traj" relaxed, traj = self.mol.relax( calculator=EMT(), fmax=0.01, steps=2, return_trajectory=True, opt_kwargs={"trajectory": traj_file} ) diff --git a/pymatgen/io/abinit/inputs.py b/pymatgen/io/abinit/inputs.py index 1d6f07d67f9..16adc463ec9 100644 --- a/pymatgen/io/abinit/inputs.py +++ b/pymatgen/io/abinit/inputs.py @@ -1087,9 +1087,6 @@ def __init__(self, structure: Structure, pseudos, pseudo_dir="", ndtset=1): if isinstance(pseudos, Pseudo): pseudos = [pseudos] - elif isinstance(pseudos, PseudoTable): - pseudos = pseudos - elif all(isinstance(p, Pseudo) for p in pseudos): pseudos = PseudoTable(pseudos) diff --git a/pymatgen/io/cif.py b/pymatgen/io/cif.py index 3c72203627a..40344fb1f04 100644 --- a/pymatgen/io/cif.py +++ b/pymatgen/io/cif.py @@ -13,7 +13,7 @@ from io import StringIO from itertools import groupby from pathlib import Path -from typing import Literal +from typing import TYPE_CHECKING, Literal import numpy as np from monty.io import zopen @@ -32,6 +32,9 @@ from pymatgen.symmetry.structure import SymmetrizedStructure from pymatgen.util.coord import find_in_coord_list_pbc, in_coord_list_pbc +if TYPE_CHECKING: + from pymatgen.core.trajectory import Vector3D + __author__ = "Shyue Ping Ong, Will Richards, Matthew Horton" sub_spgrp = partial(re.sub, r"[\s_]", "") @@ -552,17 +555,26 @@ def _sanitize_data(self, data): return data - def _unique_coords(self, coords_in, magmoms_in=None, lattice=None): + def _unique_coords( + self, + coords: list[Vector3D], + magmoms: list[Magmom] | None = None, + lattice: Lattice | None = None, + labels: dict[Vector3D, str] | None = None, + ): """ Generate unique coordinates using coord and symmetry positions and also their corresponding magnetic moments, if supplied. """ - coords = [] - if magmoms_in: - magmoms = [] - if len(magmoms_in) != len(coords_in): + coords_out: list[np.ndarray] = [] + labels_out = [] + labels = labels or {} + + if magmoms: + magmoms_out = [] + if len(magmoms) != len(coords): raise ValueError - for tmp_coord, tmp_magmom in zip(coords_in, magmoms_in): + for tmp_coord, tmp_magmom in zip(coords, magmoms): for op in self.symmetry_operations: coord = op.operate(tmp_coord) coord = np.array([i - math.floor(i) for i in coord]) @@ -575,18 +587,22 @@ def _unique_coords(self, coords_in, magmoms_in=None, lattice=None): ) else: magmom = Magmom(tmp_magmom) - if not in_coord_list_pbc(coords, coord, atol=self._site_tolerance): - coords.append(coord) - magmoms.append(magmom) - return coords, magmoms + if not in_coord_list_pbc(coords_out, coord, atol=self._site_tolerance): + coords_out.append(coord) + magmoms_out.append(magmom) + labels_out.append(labels.get(tmp_coord)) + return coords_out, magmoms_out, labels_out - for tmp_coord in coords_in: + for tmp_coord in coords: for op in self.symmetry_operations: coord = op.operate(tmp_coord) coord = np.array([i - math.floor(i) for i in coord]) - if not in_coord_list_pbc(coords, coord, atol=self._site_tolerance): - coords.append(coord) - return coords, [Magmom(0)] * len(coords) # return dummy magmoms + if not in_coord_list_pbc(coords_out, coord, atol=self._site_tolerance): + coords_out.append(coord) + labels_out.append(labels.get(tmp_coord)) + + dummy_magmoms = [Magmom(0)] * len(coords_out) + return coords_out, dummy_magmoms, labels_out def get_lattice( self, @@ -1022,9 +1038,11 @@ def get_matching_coord(coord): tmp_magmom = [coord_to_magmoms[tmp_coord] for tmp_coord in tmp_coords] if self.feature_flags["magcif"]: - coords, magmoms = self._unique_coords(tmp_coords, magmoms_in=tmp_magmom, lattice=lattice) + coords, magmoms, new_labels = self._unique_coords( + tmp_coords, magmoms=tmp_magmom, labels=labels, lattice=lattice + ) else: - coords, magmoms = self._unique_coords(tmp_coords) + coords, magmoms, new_labels = self._unique_coords(tmp_coords, labels=labels) if set(comp.elements) == {Element("O"), Element("H")}: # O with implicit hydrogens @@ -1049,7 +1067,7 @@ def get_matching_coord(coord): all_coords.extend(coords) all_species.extend(len(coords) * [species]) all_magmoms.extend(magmoms) - all_labels.extend(len(coords) * [labels[tmp_coords[0]]]) + all_labels.extend(new_labels) # rescale occupancies if necessary for idx, species in enumerate(all_species): @@ -1342,16 +1360,20 @@ def __init__( atom_site_fract_x.append(format_str.format(site.a)) atom_site_fract_y.append(format_str.format(site.b)) atom_site_fract_z.append(format_str.format(site.c)) - atom_site_label.append(f"{sp.symbol}{count}") atom_site_occupancy.append(str(occu)) + site_label = f"{sp.symbol}{count}" if "magmom" in site.properties: mag = site.properties["magmom"] elif getattr(sp, "spin", None) is not None: mag = sp.spin else: + # Use site label if available for regular sites + site_label = site.label if site.label != site.species_string else site_label mag = 0 + atom_site_label.append(site_label) + magmom = Magmom(mag) if write_magmoms and abs(magmom) > 0: moment = Magmom.get_moment_relative_to_crystal_axes(magmom, latt) @@ -1386,7 +1408,8 @@ def __init__( atom_site_fract_x.append(format_str.format(site.a)) atom_site_fract_y.append(format_str.format(site.b)) atom_site_fract_z.append(format_str.format(site.c)) - atom_site_label.append(f"{sp.symbol}{count}") + site_label = site.label if site.label != site.species_string else f"{sp.symbol}{count}" + atom_site_label.append(site_label) atom_site_occupancy.append(str(occu)) count += 1 diff --git a/pymatgen/io/lammps/tests/test_inputs.py b/pymatgen/io/lammps/tests/test_inputs.py index cc44970ca8c..5ec0819bf0e 100644 --- a/pymatgen/io/lammps/tests/test_inputs.py +++ b/pymatgen/io/lammps/tests/test_inputs.py @@ -4,9 +4,7 @@ import os import re import shutil -import tempfile import unittest -from pathlib import Path import pandas as pd import pytest @@ -671,45 +669,43 @@ def tearDownClass(cls): class LammpsTemplateGenTest(PymatgenTest): def test_write_inputs(self): - with tempfile.TemporaryDirectory() as tmp_dir: - # simple script without data file - lis = LammpsTemplateGen().get_input_set( - script_template=f"{PymatgenTest.TEST_FILES_DIR}/lammps/kappa.txt", - settings={"method": "heat"}, - data=None, - data_filename="data.peptide", - ) - tmp_dir = Path(tmp_dir) - assert len(lis) == 1 - lis.write_input(tmp_dir / "heat") - - with open(tmp_dir / "heat" / "in.lammps") as f: - kappa_script = f.read() - fix_hot = re.search(r"fix\s+hot\s+all\s+([^\s]+)\s+", kappa_script) - # placeholders supposed to be filled - assert fix_hot.group(1) == "heat" - fix_cold = re.search(r"fix\s+cold\s+all\s+([^\s]+)\s+", kappa_script) - assert fix_cold.group(1) == "heat" - lattice = re.search(r"lattice\s+fcc\s+(.*)\n", kappa_script) - # parentheses not supposed to be filled - assert lattice.group(1) == "${rho}" - pair_style = re.search(r"pair_style\slj/cut\s+(.*)\n", kappa_script) - assert pair_style.group(1) == "${rc}" - - # script with data file - obj = LammpsData.from_file( - os.path.join(PymatgenTest.TEST_FILES_DIR, "lammps", "data.quartz"), atom_style="atomic" - ) - lis = LammpsTemplateGen().get_input_set( - script_template=f"{PymatgenTest.TEST_FILES_DIR}/lammps/in.peptide", - settings=None, - data=obj, - data_filename="data.peptide", - ) - assert len(lis) == 2 - assert isinstance(lis["data.peptide"], LammpsData) - lis.write_input(tmp_dir / "obj") - - obj_read = LammpsData.from_file(str(tmp_dir / "obj" / "data.peptide"), atom_style="atomic") - pd.testing.assert_frame_equal(obj_read.masses, obj.masses) - pd.testing.assert_frame_equal(obj_read.atoms, obj.atoms) + # simple script without data file + lis = LammpsTemplateGen().get_input_set( + script_template=f"{PymatgenTest.TEST_FILES_DIR}/lammps/kappa.txt", + settings={"method": "heat"}, + data=None, + data_filename="data.peptide", + ) + assert len(lis) == 1 + lis.write_input(self.tmp_path / "heat") + + with open(self.tmp_path / "heat" / "in.lammps") as f: + kappa_script = f.read() + fix_hot = re.search(r"fix\s+hot\s+all\s+([^\s]+)\s+", kappa_script) + # placeholders supposed to be filled + assert fix_hot.group(1) == "heat" + fix_cold = re.search(r"fix\s+cold\s+all\s+([^\s]+)\s+", kappa_script) + assert fix_cold.group(1) == "heat" + lattice = re.search(r"lattice\s+fcc\s+(.*)\n", kappa_script) + # parentheses not supposed to be filled + assert lattice.group(1) == "${rho}" + pair_style = re.search(r"pair_style\slj/cut\s+(.*)\n", kappa_script) + assert pair_style.group(1) == "${rc}" + + # script with data file + obj = LammpsData.from_file( + os.path.join(PymatgenTest.TEST_FILES_DIR, "lammps", "data.quartz"), atom_style="atomic" + ) + lis = LammpsTemplateGen().get_input_set( + script_template=f"{PymatgenTest.TEST_FILES_DIR}/lammps/in.peptide", + settings=None, + data=obj, + data_filename="data.peptide", + ) + assert len(lis) == 2 + assert isinstance(lis["data.peptide"], LammpsData) + lis.write_input(self.tmp_path / "obj") + + obj_read = LammpsData.from_file(str(self.tmp_path / "obj" / "data.peptide"), atom_style="atomic") + pd.testing.assert_frame_equal(obj_read.masses, obj.masses) + pd.testing.assert_frame_equal(obj_read.atoms, obj.atoms) diff --git a/pymatgen/io/lobster/lobsterenv.py b/pymatgen/io/lobster/lobsterenv.py index 0566bbe1c44..30abf061878 100644 --- a/pymatgen/io/lobster/lobsterenv.py +++ b/pymatgen/io/lobster/lobsterenv.py @@ -1279,9 +1279,7 @@ def from_Lobster( Returns: LobsterLightStructureEnvironments """ strategy = None - valences = valences valences_origin = "user-defined" - structure = structure coordination_environments = [] diff --git a/pymatgen/io/lobster/outputs.py b/pymatgen/io/lobster/outputs.py index 1c356b33469..661f7559809 100644 --- a/pymatgen/io/lobster/outputs.py +++ b/pymatgen/io/lobster/outputs.py @@ -116,12 +116,8 @@ def __init__(self, are_coops: bool = False, are_cobis: bool = False, filename: s # Subtract 1 to skip the average num_bonds = int(parameters[0]) - 1 self.efermi = float(parameters[-1]) - if int(parameters[1]) == 2: - spins = [Spin.up, Spin.down] - self.is_spin_polarized = True - else: - spins = [Spin.up] - self.is_spin_polarized = False + self.is_spin_polarized = int(parameters[1]) == 2 + spins = [Spin.up, Spin.down] if int(parameters[1]) == 2 else [Spin.up] # The COHP data start in row num_bonds + 3 data = np.array([np.array(row.split(), dtype=float) for row in contents[num_bonds + 3 :]]).transpose() @@ -135,22 +131,22 @@ def __init__(self, are_coops: bool = False, are_cobis: bool = False, filename: s orb_cohp: dict[str, Any] = {} # present for Lobster versions older than Lobster 2.2.0 - veryold = False + very_old = False # the labeling had to be changed: there are more than one COHP for each atom combination # this is done to make the labeling consistent with ICOHPLIST.lobster - bondnumber = 0 + bond_num = 0 for bond in range(num_bonds): bond_data = self._get_bond_data(contents[3 + bond]) - label = str(bondnumber) + label = str(bond_num) orbs = bond_data["orbitals"] cohp = {spin: data[2 * (bond + s * (num_bonds + 1)) + 3] for s, spin in enumerate(spins)} icohp = {spin: data[2 * (bond + s * (num_bonds + 1)) + 4] for s, spin in enumerate(spins)} if orbs is None: - bondnumber = bondnumber + 1 - label = str(bondnumber) + bond_num = bond_num + 1 + label = str(bond_num) cohp_data[label] = { "COHP": cohp, "ICOHP": icohp, @@ -172,11 +168,11 @@ def __init__(self, are_coops: bool = False, are_cobis: bool = False, filename: s ) else: # present for Lobster versions older than Lobster 2.2.0 - if bondnumber == 0: - veryold = True - if veryold: - bondnumber += 1 - label = str(bondnumber) + if bond_num == 0: + very_old = True + if very_old: + bond_num += 1 + label = str(bond_num) orb_cohp[label] = { bond_data["orb_label"]: { @@ -189,7 +185,7 @@ def __init__(self, are_coops: bool = False, are_cobis: bool = False, filename: s } # present for lobster older than 2.2.0 - if veryold: + if very_old: for bond_str in orb_cohp: cohp_data[bond_str] = { "COHP": None, @@ -302,18 +298,12 @@ def __init__(self, are_coops: bool = False, are_cobis: bool = False, filename: s # If the calculation is spin polarized, the line in the middle # of the file will be another header line. - if "distance" in data[len(data) // 2]: - # TODO: adapt this for orbitalwise stuff - self.is_spin_polarized = True - else: - self.is_spin_polarized = False + # TODO: adapt this for orbitalwise stuff + self.is_spin_polarized = "distance" in data[len(data) // 2] # check if orbitalwise ICOHPLIST # include case when there is only one ICOHP!!! - if len(data) > 2 and "_" in data[1].split()[1]: - self.orbitalwise = True - else: - self.orbitalwise = False + self.orbitalwise = len(data) > 2 and "_" in data[1].split()[1] if self.orbitalwise: data_without_orbitals = [] @@ -364,7 +354,7 @@ def __init__(self, are_coops: bool = False, are_cobis: bool = False, filename: s length = float(line[3]) translation = [int(line[4]), int(line[5]), int(line[6])] icohp[Spin.up] = float(line[7]) - num = int(1) + num = 1 if self.is_spin_polarized: icohp[Spin.down] = float(data_without_orbitals[bond + num_bonds + 1].split()[7]) @@ -1141,10 +1131,7 @@ def __init__(self, filenames=".", vasprun="vasprun.xml", Kpointsfile="KPOINTS"): linenumbers.append(iline) if ifilename == 0: - if len(linenumbers) == 2: - self.is_spinpolarized = True - else: - self.is_spinpolarized = False + self.is_spinpolarized = len(linenumbers) == 2 if ifilename == 0: eigenvals = {} diff --git a/pymatgen/io/nwchem.py b/pymatgen/io/nwchem.py index 833c821792e..a170f22fec8 100644 --- a/pymatgen/io/nwchem.py +++ b/pymatgen/io/nwchem.py @@ -261,25 +261,23 @@ def from_molecule( example, to perform cosmo calculations with DFT, you'd supply {'cosmo': "cosmo"}. """ - title = title if title is not None else "{} {} {}".format(re.sub(r"\s", "", mol.formula), theory, operation) + formula = re.sub(r"\s", "", mol.formula) + title = title if title is not None else f"{formula} {theory} {operation}" charge = charge if charge is not None else mol.charge - nelectrons = -charge + mol.charge + mol.nelectrons # pylint: disable=E1130 + n_electrons = -charge + mol.charge + mol.nelectrons # pylint: disable=E1130 if spin_multiplicity is not None: - spin_multiplicity = spin_multiplicity - if (nelectrons + spin_multiplicity) % 2 != 1: + if (n_electrons + spin_multiplicity) % 2 != 1: raise ValueError(f"{charge=} and {spin_multiplicity=} is not possible for this molecule") elif charge == mol.charge: spin_multiplicity = mol.spin_multiplicity else: - spin_multiplicity = 1 if nelectrons % 2 == 0 else 2 + spin_multiplicity = 1 if n_electrons % 2 == 0 else 2 elements = set(mol.composition.get_el_amt_dict()) if isinstance(basis_set, str): basis_set = {el: basis_set for el in elements} - basis_set_option = basis_set_option - return NwTask( charge, spin_multiplicity, diff --git a/pymatgen/io/res.py b/pymatgen/io/res.py index 1265f3c0481..2d19437776e 100644 --- a/pymatgen/io/res.py +++ b/pymatgen/io/res.py @@ -30,6 +30,8 @@ from collections.abc import Iterator from datetime import date + from pymatgen.core.trajectory import Vector3D + __all__ = ["ResProvider", "AirssProvider", "ResIO", "ResWriter", "ResParseError", "ResError"] @@ -73,7 +75,7 @@ def __str__(self) -> str: class Ion: specie: str specie_num: int - pos: tuple[float, float, float] + pos: Vector3D occupancy: float spin: float | None @@ -488,9 +490,7 @@ def get_cut_grid_gmax_fsbc(self) -> tuple[float, float, float, str] | None: self._raise_or_none(ResParseError("Could not find line with cut-off energy.")) return None - def get_mpgrid_offset_nkpts_spacing( - self, - ) -> tuple[tuple[int, int, int], tuple[float, float, float], int, float] | None: + def get_mpgrid_offset_nkpts_spacing(self) -> tuple[tuple[int, int, int], Vector3D, int, float] | None: """ Retrieves the MP grid, the grid offsets, number of kpoints, and maximum kpoint spacing. diff --git a/pymatgen/io/tests/test_cif.py b/pymatgen/io/tests/test_cif.py index 42fffbcbce8..c3d60c73c9c 100644 --- a/pymatgen/io/tests/test_cif.py +++ b/pymatgen/io/tests/test_cif.py @@ -333,6 +333,25 @@ def test_site_labels(self): # Ensure the site label starts with the site species name assert site.label.startswith(site.specie.name) + # ensure multiple species with different names have correct labels + parser2 = CifParser(f"{self.TEST_FILES_DIR}/Fe3O4.cif") + struct2 = parser2.get_structures(primitive=False)[0] + + expected_site_names2 = {*"O1 O2 O3 O4 O5 O6 O7 O8 Fe9 Fe10 Fe11 Fe12 Fe13 Fe14".split()} + assert set(struct2.labels) == expected_site_names2 + + def test_cif_writer_labeled(self): + parser = CifParser(f"{self.TEST_FILES_DIR}/garnet.cif") + struct = parser.get_structures()[0] + for idx, site in enumerate(struct): + site.label = f"my_{site.specie.name}{idx}" + writer = CifWriter(struct) + + parser2 = CifParser.from_str(str(writer)) + struct2 = parser2.get_structures()[0] + + assert set(struct.labels) == set(struct2.labels) + def test_cif_parser_springer_pauling(self): # Below are 10 tests for CIFs from the Springer Materials/Pauling file DBs. diff --git a/pymatgen/io/tests/test_packmol.py b/pymatgen/io/tests/test_packmol.py index 6b8b50afa14..c92f37a629c 100644 --- a/pymatgen/io/tests/test_packmol.py +++ b/pymatgen/io/tests/test_packmol.py @@ -2,8 +2,8 @@ from __future__ import annotations import os -import tempfile from pathlib import Path +from shutil import which from subprocess import TimeoutExpired import pytest @@ -15,259 +15,230 @@ test_dir = os.path.join(PymatgenTest.TEST_FILES_DIR, "packmol") -# Just skip this whole test for now since packmol is problematic. -if True: # if which("packmol") is None: +if which("packmol") is None: pytest.skip("packmol executable not present", allow_module_level=True) -@pytest.fixture() -def ethanol(): - """ - Returns a Molecule of ethanol - """ - ethanol_coords = [ - [0.00720, -0.56870, 0.00000], - [-1.28540, 0.24990, 0.00000], - [1.13040, 0.31470, 0.00000], - [0.03920, -1.19720, 0.89000], - [0.03920, -1.19720, -0.89000], - [-1.31750, 0.87840, 0.89000], - [-1.31750, 0.87840, -0.89000], - [-2.14220, -0.42390, -0.00000], - [1.98570, -0.13650, -0.00000], - ] - ethanol_atoms = ["C", "C", "O", "H", "H", "H", "H", "H", "H"] +ethanol_coords = [ + [0.00720, -0.56870, 0.00000], + [-1.28540, 0.24990, 0.00000], + [1.13040, 0.31470, 0.00000], + [0.03920, -1.19720, 0.89000], + [0.03920, -1.19720, -0.89000], + [-1.31750, 0.87840, 0.89000], + [-1.31750, 0.87840, -0.89000], + [-2.14220, -0.42390, -0.00000], + [1.98570, -0.13650, -0.00000], +] +ethanol_atoms = ["C", "C", "O", "H", "H", "H", "H", "H", "H"] - return Molecule(ethanol_atoms, ethanol_coords) +ethanol = Molecule(ethanol_atoms, ethanol_coords) -@pytest.fixture() -def water(): - """ - Returns a Molecule of water - """ - water_coords = [ - [9.626, 6.787, 12.673], - [9.626, 8.420, 12.673], - [10.203, 7.604, 12.673], - ] - water_atoms = ["H", "H", "O"] +water_coords = [ + [9.626, 6.787, 12.673], + [9.626, 8.420, 12.673], + [10.203, 7.604, 12.673], +] +water_atoms = ["H", "H", "O"] - return Molecule(water_atoms, water_coords) +water = Molecule(water_atoms, water_coords) -class TestPackmolSet: - def test_packmol_with_molecule(self, water, ethanol): +class TestPackmolSet(PymatgenTest): + def test_packmol_with_molecule(self): """ Test coords input as Molecule """ - with tempfile.TemporaryDirectory() as scratch_dir: - pw = PackmolBoxGen().get_input_set( - molecules=[ - {"name": "water", "number": 10, "coords": water}, - {"name": "ethanol", "number": 20, "coords": ethanol}, - ], - ) - pw.write_input(scratch_dir) - pw.run(scratch_dir) - assert os.path.exists(os.path.join(scratch_dir, "packmol_out.xyz")) - out = Molecule.from_file(os.path.join(scratch_dir, "packmol_out.xyz")) - assert out.composition.num_atoms == 10 * 3 + 20 * 9 + pw = PackmolBoxGen().get_input_set( + molecules=[ + {"name": "water", "number": 10, "coords": water}, + {"name": "ethanol", "number": 20, "coords": ethanol}, + ], + ) + pw.write_input(self.tmp_path) + pw.run(self.tmp_path) + assert os.path.exists(os.path.join(self.tmp_path, "packmol_out.xyz")) + out = Molecule.from_file(os.path.join(self.tmp_path, "packmol_out.xyz")) + assert out.composition.num_atoms == 10 * 3 + 20 * 9 def test_packmol_with_str(self): """ Test coords input as strings """ - with tempfile.TemporaryDirectory() as scratch_dir: - pw = PackmolBoxGen().get_input_set( - molecules=[ - {"name": "EMC", "number": 10, "coords": os.path.join(test_dir, "subdir with spaces", "EMC.xyz.gz")}, - {"name": "LiTFSi", "number": 20, "coords": os.path.join(test_dir, "LiTFSi.xyz.gz")}, - ], - ) - pw.write_input(scratch_dir) - pw.run(scratch_dir) - assert os.path.exists(os.path.join(scratch_dir, "packmol_out.xyz")) - out = Molecule.from_file(os.path.join(scratch_dir, "packmol_out.xyz")) - assert out.composition.num_atoms == 10 * 15 + 20 * 16 + pw = PackmolBoxGen().get_input_set( + molecules=[ + {"name": "EMC", "number": 10, "coords": os.path.join(test_dir, "subdir with spaces", "EMC.xyz")}, + {"name": "LiTFSi", "number": 20, "coords": os.path.join(test_dir, "LiTFSi.xyz")}, + ], + ) + pw.write_input(self.tmp_path) + pw.run(self.tmp_path) + assert os.path.exists(os.path.join(self.tmp_path, "packmol_out.xyz")) + out = Molecule.from_file(os.path.join(self.tmp_path, "packmol_out.xyz")) + assert out.composition.num_atoms == 10 * 15 + 20 * 16 def test_packmol_with_path(self): """ Test coords input as Path. Use a subdirectory with spaces. """ - p1 = Path(os.path.join(test_dir, "subdir with spaces", "EMC.xyz.gz")) - p2 = Path(os.path.join(test_dir, "LiTFSi.xyz.gz")) - with tempfile.TemporaryDirectory() as scratch_dir: - pw = PackmolBoxGen().get_input_set( - molecules=[ - {"name": "EMC", "number": 10, "coords": p1}, - {"name": "LiTFSi", "number": 20, "coords": p2}, - ], - ) - pw.write_input(scratch_dir) - pw.run(scratch_dir) - assert os.path.exists(os.path.join(scratch_dir, "packmol_out.xyz")) - out = Molecule.from_file(os.path.join(scratch_dir, "packmol_out.xyz")) - assert out.composition.num_atoms == 10 * 15 + 20 * 16 - - def test_control_params(self, water, ethanol): + p1 = Path(os.path.join(test_dir, "subdir with spaces", "EMC.xyz")) + p2 = Path(os.path.join(test_dir, "LiTFSi.xyz")) + pw = PackmolBoxGen().get_input_set( + molecules=[ + {"name": "EMC", "number": 10, "coords": p1}, + {"name": "LiTFSi", "number": 20, "coords": p2}, + ], + ) + pw.write_input(self.tmp_path) + pw.run(self.tmp_path) + assert os.path.exists(os.path.join(self.tmp_path, "packmol_out.xyz")) + out = Molecule.from_file(os.path.join(self.tmp_path, "packmol_out.xyz")) + assert out.composition.num_atoms == 10 * 15 + 20 * 16 + + def test_control_params(self): """ Check that custom control_params work and that ValueError is raised when 'ERROR' appears in stdout (even if return code is 0) """ - with tempfile.TemporaryDirectory() as scratch_dir: - input_set = PackmolBoxGen( - control_params={"maxit": 0, "nloop": 0}, - ).get_input_set( - molecules=[ - {"name": "water", "number": 1000, "coords": water}, - {"name": "ethanol", "number": 2000, "coords": ethanol}, - ], - ) - input_set.write_input(scratch_dir) - with open(os.path.join(scratch_dir, "packmol.inp")) as f: - input_string = f.read() - assert "maxit 0" in input_string - assert "nloop 0" in input_string - with pytest.raises(ValueError): - input_set.run(scratch_dir) - - def test_timeout(self, water, ethanol): + input_set = PackmolBoxGen( + control_params={"maxit": 0, "nloop": 0}, + ).get_input_set( + molecules=[ + {"name": "water", "number": 1000, "coords": water}, + {"name": "ethanol", "number": 2000, "coords": ethanol}, + ], + ) + input_set.write_input(self.tmp_path) + with open(os.path.join(self.tmp_path, "packmol.inp")) as f: + input_string = f.read() + assert "maxit 0" in input_string + assert "nloop 0" in input_string + with pytest.raises(ValueError): + input_set.run(self.tmp_path) + + def test_timeout(self): """ Check that the timeout works """ - with tempfile.TemporaryDirectory() as scratch_dir: - pw = PackmolBoxGen().get_input_set( - molecules=[ - {"name": "water", "number": 1000, "coords": water}, - {"name": "ethanol", "number": 2000, "coords": ethanol}, - ], - ) - pw.write_input(scratch_dir) - with pytest.raises(TimeoutExpired): - pw.run(scratch_dir, 1) + pw = PackmolBoxGen().get_input_set( + molecules=[ + {"name": "water", "number": 1000, "coords": water}, + {"name": "ethanol", "number": 2000, "coords": ethanol}, + ], + ) + pw.write_input(self.tmp_path) + with pytest.raises(TimeoutExpired): + pw.run(self.tmp_path, 1) - def test_no_return_and_box(self, water, ethanol): + def test_no_return_and_box(self): """ Make sure the code raises an error if packmol doesn't exit cleanly. Also verify the box arg works properly. """ - with tempfile.TemporaryDirectory() as scratch_dir: - pw = PackmolBoxGen().get_input_set( - molecules=[ - {"name": "water", "number": 1000, "coords": water}, - {"name": "ethanol", "number": 2000, "coords": ethanol}, - ], - box=[0, 0, 0, 2, 2, 2], - ) - pw.write_input(scratch_dir) - with open(os.path.join(scratch_dir, "packmol.inp")) as f: - input_string = f.read() - assert "inside box 0 0 0 2 2 2" in input_string - with pytest.raises(ValueError): - pw.run(scratch_dir) - - def test_chdir_behavior(self, water, ethanol): + pw = PackmolBoxGen().get_input_set( + molecules=[ + {"name": "water", "number": 1000, "coords": water}, + {"name": "ethanol", "number": 2000, "coords": ethanol}, + ], + box=[0, 0, 0, 2, 2, 2], + ) + pw.write_input(self.tmp_path) + with open(os.path.join(self.tmp_path, "packmol.inp")) as f: + input_string = f.read() + assert "inside box 0 0 0 2 2 2" in input_string + with pytest.raises(ValueError): + pw.run(self.tmp_path) + + def test_chdir_behavior(self): """ Make sure the code returns to the starting directory whether or not packmol exits cleanly. """ startdir = str(Path.cwd()) - with tempfile.TemporaryDirectory() as scratch_dir: - # this one will not exit cleanly b/c the box is too small - pw = PackmolBoxGen().get_input_set( - molecules=[ - {"name": "water", "number": 1000, "coords": water}, - {"name": "ethanol", "number": 2000, "coords": ethanol}, - ], - box=[0, 0, 0, 2, 2, 2], - ) - pw.write_input(scratch_dir) - with pytest.raises(ValueError): - pw.run(scratch_dir) - assert str(Path.cwd()) == startdir - - with tempfile.TemporaryDirectory() as scratch_dir: - # this one will exit cleanly - pw = PackmolBoxGen().get_input_set( - molecules=[ - {"name": "water", "number": 1000, "coords": water}, - {"name": "ethanol", "number": 2000, "coords": ethanol}, - ], - ) - pw.write_input(scratch_dir) - pw.run(scratch_dir) - assert str(Path.cwd()) == startdir - - def test_random_seed(self, water, ethanol): + # this one will not exit cleanly b/c the box is too small + pw = PackmolBoxGen().get_input_set( + molecules=[ + {"name": "water", "number": 1000, "coords": water}, + {"name": "ethanol", "number": 2000, "coords": ethanol}, + ], + box=[0, 0, 0, 2, 2, 2], + ) + pw.write_input(self.tmp_path) + with pytest.raises(ValueError): + pw.run(self.tmp_path) + assert str(Path.cwd()) == startdir + + # this one will exit cleanly + pw = PackmolBoxGen().get_input_set( + molecules=[ + {"name": "water", "number": 1000, "coords": water}, + {"name": "ethanol", "number": 2000, "coords": ethanol}, + ], + ) + pw.write_input(self.tmp_path) + pw.run(self.tmp_path) + assert str(Path.cwd()) == startdir + + def test_random_seed(self): """ Confirm that seed = -1 generates random structures while seed = 1 is deterministic """ + pytest.importorskip("openbabel") mm = MoleculeMatcher() # deterministic output - with tempfile.TemporaryDirectory() as scratch_dir: - pw = PackmolBoxGen( - seed=1, - inputfile="input.in", - outputfile="output.xyz", - ).get_input_set( - # scratch_dir, - molecules=[ - {"name": "water", "number": 10, "coords": water}, - {"name": "ethanol", "number": 20, "coords": ethanol}, - ], - ) - pw.write_input(scratch_dir) - pw.run(scratch_dir) - out1 = Molecule.from_file(os.path.join(scratch_dir, "output.xyz")) - pw.run(scratch_dir) - out2 = Molecule.from_file(os.path.join(scratch_dir, "output.xyz")) - assert mm.fit(out1, out2) + pw = PackmolBoxGen(seed=1, inputfile="input.in", outputfile="output.xyz").get_input_set( + # self.tmp_path, + molecules=[ + {"name": "water", "number": 10, "coords": water}, + {"name": "ethanol", "number": 20, "coords": ethanol}, + ], + ) + pw.write_input(self.tmp_path) + pw.run(self.tmp_path) + out1 = Molecule.from_file(os.path.join(self.tmp_path, "output.xyz")) + pw.run(self.tmp_path) + out2 = Molecule.from_file(os.path.join(self.tmp_path, "output.xyz")) + assert mm.fit(out1, out2) # randomly generated structures - with tempfile.TemporaryDirectory() as scratch_dir: - pw = PackmolBoxGen( - seed=-1, - inputfile="input.in", - outputfile="output.xyz", - ).get_input_set( - molecules=[ - {"name": "water", "number": 10, "coords": water}, - {"name": "ethanol", "number": 20, "coords": ethanol}, - ], - ) - pw.write_input(scratch_dir) - pw.run(scratch_dir) - out1 = Molecule.from_file(os.path.join(scratch_dir, "output.xyz")) - pw.run(scratch_dir) - out2 = Molecule.from_file(os.path.join(scratch_dir, "output.xyz")) - assert not mm.fit(out1, out2) - - def test_arbitrary_filenames(self, water, ethanol): + pw = PackmolBoxGen(seed=-1, inputfile="input.in", outputfile="output.xyz").get_input_set( + molecules=[ + {"name": "water", "number": 10, "coords": water}, + {"name": "ethanol", "number": 20, "coords": ethanol}, + ], + ) + pw.write_input(self.tmp_path) + pw.run(self.tmp_path) + out1 = Molecule.from_file(os.path.join(self.tmp_path, "output.xyz")) + pw.run(self.tmp_path) + out2 = Molecule.from_file(os.path.join(self.tmp_path, "output.xyz")) + assert not mm.fit(out1, out2) + + def test_arbitrary_filenames(self): """ Make sure custom input and output filenames work. Use a subdirectory with spaces. """ - with tempfile.TemporaryDirectory() as scratch_dir: - os.mkdir(os.path.join(scratch_dir, "subdirectory with spaces")) - pw = PackmolBoxGen( - inputfile="input.in", outputfile=Path("output.xyz"), stdoutfile=Path("stdout.txt") - ).get_input_set( - molecules=[ - {"name": "water", "number": 10, "coords": water}, - {"name": "ethanol", "number": 20, "coords": ethanol}, - ], - ) - pw.write_input( - os.path.join(scratch_dir, "subdirectory with spaces"), - ) - assert os.path.exists(os.path.join(scratch_dir, "subdirectory with spaces", "input.in")) - pw.run( - os.path.join(scratch_dir, "subdirectory with spaces"), - ) - assert os.path.exists(os.path.join(scratch_dir, "subdirectory with spaces", "output.xyz")) - assert os.path.exists(os.path.join(scratch_dir, "subdirectory with spaces", "stdout.txt")) - out = Molecule.from_file(os.path.join(scratch_dir, "subdirectory with spaces", "output.xyz")) - assert out.composition.num_atoms == 10 * 3 + 20 * 9 + os.mkdir(os.path.join(self.tmp_path, "subdirectory with spaces")) + pw = PackmolBoxGen( + inputfile="input.in", outputfile=Path("output.xyz"), stdoutfile=Path("stdout.txt") + ).get_input_set( + molecules=[ + {"name": "water", "number": 10, "coords": water}, + {"name": "ethanol", "number": 20, "coords": ethanol}, + ], + ) + pw.write_input( + os.path.join(self.tmp_path, "subdirectory with spaces"), + ) + assert os.path.exists(os.path.join(self.tmp_path, "subdirectory with spaces", "input.in")) + pw.run( + os.path.join(self.tmp_path, "subdirectory with spaces"), + ) + assert os.path.exists(os.path.join(self.tmp_path, "subdirectory with spaces", "output.xyz")) + assert os.path.exists(os.path.join(self.tmp_path, "subdirectory with spaces", "stdout.txt")) + out = Molecule.from_file(os.path.join(self.tmp_path, "subdirectory with spaces", "output.xyz")) + assert out.composition.num_atoms == 10 * 3 + 20 * 9 diff --git a/pymatgen/io/tests/test_template_input.py b/pymatgen/io/tests/test_template_input.py index a62da997ee7..f7b8f37b53c 100644 --- a/pymatgen/io/tests/test_template_input.py +++ b/pymatgen/io/tests/test_template_input.py @@ -1,7 +1,6 @@ from __future__ import annotations import os -import tempfile import pytest @@ -11,42 +10,41 @@ test_dir = os.path.join(PymatgenTest.TEST_FILES_DIR) -class TestTemplateInputGen: +class TestTemplateInputGen(PymatgenTest): def test_write_inputs(self): - with tempfile.TemporaryDirectory() as scratch_dir: - tis = TemplateInputGen().get_input_set( - template=os.path.join(test_dir, "template_input_file.txt"), - variables={"TEMPERATURE": 298}, - filename="hello_world.in", - ) - tis.write_input(scratch_dir) - with open(os.path.join(scratch_dir, "hello_world.in")) as f: - assert "298" in f.read() + tis = TemplateInputGen().get_input_set( + template=os.path.join(test_dir, "template_input_file.txt"), + variables={"TEMPERATURE": 298}, + filename="hello_world.in", + ) + tis.write_input(self.tmp_path) + with open(os.path.join(self.tmp_path, "hello_world.in")) as f: + assert "298" in f.read() - with pytest.raises(FileNotFoundError, match="No such file or directory:"): - tis.write_input(os.path.join(scratch_dir, "temp"), make_dir=False) + with pytest.raises(FileNotFoundError, match="No such file or directory:"): + tis.write_input(os.path.join(self.tmp_path, "temp"), make_dir=False) - tis.write_input(os.path.join(scratch_dir, "temp"), make_dir=True) + tis.write_input(os.path.join(self.tmp_path, "temp"), make_dir=True) - tis = TemplateInputGen().get_input_set( - template=os.path.join(test_dir, "template_input_file.txt"), - variables={"TEMPERATURE": 400}, - filename="hello_world.in", - ) + tis = TemplateInputGen().get_input_set( + template=os.path.join(test_dir, "template_input_file.txt"), + variables={"TEMPERATURE": 400}, + filename="hello_world.in", + ) - # test len, iter, getitem - assert len(tis.inputs) == 1 - assert len(list(tis.inputs)) == 1 - assert isinstance(tis.inputs["hello_world.in"], str) + # test len, iter, getitem + assert len(tis.inputs) == 1 + assert len(list(tis.inputs)) == 1 + assert isinstance(tis.inputs["hello_world.in"], str) - with pytest.raises(FileExistsError, match="hello_world.in"): - tis.write_input(scratch_dir, overwrite=False) + with pytest.raises(FileExistsError, match="hello_world.in"): + tis.write_input(self.tmp_path, overwrite=False) - tis.write_input(scratch_dir, overwrite=True) + tis.write_input(self.tmp_path, overwrite=True) - with open(os.path.join(scratch_dir, "hello_world.in")) as f: - assert "400" in f.read() + with open(os.path.join(self.tmp_path, "hello_world.in")) as f: + assert "400" in f.read() - tis.write_input(scratch_dir, zip_inputs=True) + tis.write_input(self.tmp_path, zip_inputs=True) - assert "InputSet.zip" in list(os.listdir(scratch_dir)) + assert "InputSet.zip" in list(os.listdir(self.tmp_path)) diff --git a/pymatgen/io/vasp/inputs.py b/pymatgen/io/vasp/inputs.py index 8e4b71f832d..149b5383cdf 100644 --- a/pymatgen/io/vasp/inputs.py +++ b/pymatgen/io/vasp/inputs.py @@ -40,6 +40,7 @@ if TYPE_CHECKING: from numpy.typing import ArrayLike + from pymatgen.core.trajectory import Vector3D from pymatgen.util.typing import PathLike @@ -1034,7 +1035,7 @@ def __init__( num_kpts: int = 0, style: KpointsSupportedModes = supported_modes.Gamma, kpts: Sequence[float | int | Sequence] = ((1, 1, 1),), - kpts_shift: tuple[float, float, float] = (0, 0, 0), + kpts_shift: Vector3D = (0, 0, 0), kpts_weights=None, coord_type=None, labels=None, @@ -1155,7 +1156,7 @@ def automatic(subdivisions): ) @staticmethod - def gamma_automatic(kpts: tuple[int, int, int] = (1, 1, 1), shift: tuple[float, float, float] = (0, 0, 0)): + def gamma_automatic(kpts: tuple[int, int, int] = (1, 1, 1), shift: Vector3D = (0, 0, 0)): """ Convenient static constructor for an automatic Gamma centered Kpoint grid. @@ -1177,7 +1178,7 @@ def gamma_automatic(kpts: tuple[int, int, int] = (1, 1, 1), shift: tuple[float, ) @staticmethod - def monkhorst_automatic(kpts: tuple[int, int, int] = (2, 2, 2), shift: tuple[float, float, float] = (0, 0, 0)): + def monkhorst_automatic(kpts: tuple[int, int, int] = (2, 2, 2), shift: Vector3D = (0, 0, 0)): """ Convenient static constructor for an automatic Monkhorst pack Kpoint grid. diff --git a/pymatgen/io/vasp/outputs.py b/pymatgen/io/vasp/outputs.py index 17d7865c7fd..d6ae6f28bae 100644 --- a/pymatgen/io/vasp/outputs.py +++ b/pymatgen/io/vasp/outputs.py @@ -3641,14 +3641,14 @@ def __init__(self, poscar, data, data_aug=None): self._distance_matrix = {} @staticmethod - def from_file(filename): + def from_file(filename: str): """ - Reads a CHGCAR file. + Read a CHGCAR file. :param filename: Filename :return: Chgcar """ - (poscar, data, data_aug) = VolumetricData.parse_file(filename) + poscar, data, data_aug = VolumetricData.parse_file(filename) return Chgcar(poscar, data, data_aug=data_aug) @property diff --git a/pymatgen/io/xtb/inputs.py b/pymatgen/io/xtb/inputs.py index 7d4a272d70c..a6edd2f57b6 100644 --- a/pymatgen/io/xtb/inputs.py +++ b/pymatgen/io/xtb/inputs.py @@ -81,7 +81,6 @@ def constrains_template(molecule, reference_fnm, constraints) -> str: """ atoms_to_constrain = constraints["atoms"] force_constant = constraints["force_constant"] - reference_fnm = reference_fnm mol = molecule atoms_for_mtd = [idx for idx in range(1, len(mol) + 1) if idx not in atoms_to_constrain] # Write as 1-3,5 instead of 1,2,3,5 @@ -91,7 +90,6 @@ def constrains_template(molecule, reference_fnm, constraints) -> str: interval_list.append(v) if i != len(atoms_for_mtd) - 1: interval_list.append(atoms_for_mtd[i + 1]) - force_constant = force_constant allowed_mtd_string = ",".join( [f"{interval_list[i]}-{interval_list[i + 1]}" for i in range(len(interval_list)) if i % 2 == 0] ) diff --git a/pymatgen/phonon/tests/test_thermal_displacements.py b/pymatgen/phonon/tests/test_thermal_displacements.py index a88f324d4a8..3a10abe38b1 100644 --- a/pymatgen/phonon/tests/test_thermal_displacements.py +++ b/pymatgen/phonon/tests/test_thermal_displacements.py @@ -1,7 +1,6 @@ from __future__ import annotations import os -import tempfile import numpy as np from pytest import approx @@ -167,13 +166,12 @@ def test_beta(self): def test_write_file(self): printed = False - with tempfile.TemporaryDirectory() as tmpdirname: - self.thermal.write_cif(os.path.join(tmpdirname, "U.cif")) - with open(os.path.join(tmpdirname, "U.cif")) as file: - file.seek(0) # set position to start of file - lines = file.read().splitlines() # now we won't have those newlines - if "_atom_site_aniso_U_12" in lines: - printed = True + self.thermal.write_cif(f"{self.tmp_path}/U.cif") + with open(f"{self.tmp_path}/U.cif") as file: + file.seek(0) # set position to start of file + lines = file.read().splitlines() # now we won't have those newlines + if "_atom_site_aniso_U_12" in lines: + printed = True assert printed def test_from_ucif(self): @@ -304,21 +302,19 @@ def test_to_structure_with_site_properties(self): def test_visualization_directionality_criterion(self): # test file creation for VESTA printed = False - with tempfile.TemporaryDirectory() as tmp_dir: - self.thermal.visualize_directionality_quality_criterion( - filename=os.path.join(tmp_dir, "U.vesta"), other=self.thermal, which_structure=0 - ) - with open(os.path.join(tmp_dir, "U.vesta")) as file: - file.seek(0) # set position to start of file - lines = file.read().splitlines() # now we won't have those newlines - if "VECTR" in lines: - printed = True + self.thermal.visualize_directionality_quality_criterion( + filename=f"{self.tmp_path}/U.vesta", other=self.thermal, which_structure=0 + ) + with open(f"{self.tmp_path}/U.vesta") as file: + file.seek(0) # set position to start of file + lines = file.read().splitlines() # now we won't have those newlines + if "VECTR" in lines: + printed = True assert printed def test_from_cif_P1(self): - with tempfile.TemporaryDirectory() as tmp_dir: - self.thermal.write_cif(os.path.join(tmp_dir, "U.cif")) - new_thermals = ThermalDisplacementMatrices.from_cif_P1(os.path.join(tmp_dir, "U.cif")) - self.assert_all_close(new_thermals[0].thermal_displacement_matrix_cif_matrixform, self.thermal.Ucif) - self.assert_all_close(new_thermals[0].structure.frac_coords, self.thermal.structure.frac_coords) - self.assert_all_close(new_thermals[0].structure.volume, self.thermal.structure.volume) + self.thermal.write_cif(f"{self.tmp_path}/U.cif") + new_thermals = ThermalDisplacementMatrices.from_cif_P1(f"{self.tmp_path}/U.cif") + self.assert_all_close(new_thermals[0].thermal_displacement_matrix_cif_matrixform, self.thermal.Ucif) + self.assert_all_close(new_thermals[0].structure.frac_coords, self.thermal.structure.frac_coords) + self.assert_all_close(new_thermals[0].structure.volume, self.thermal.structure.volume)