diff --git a/pymatgen/core/structure.py b/pymatgen/core/structure.py index 67315cb12a6..848b8e50103 100644 --- a/pymatgen/core/structure.py +++ b/pymatgen/core/structure.py @@ -273,9 +273,9 @@ def types_of_specie(self) -> tuple[Element | Species | DummySpecies]: def group_by_types(self) -> Iterator[Site | PeriodicSite]: """Iterate over species grouped by type""" - for t in self.types_of_species: + for sp_typ in self.types_of_species: for site in self: - if site.specie == t: + if site.specie == sp_typ: yield site def indices_from_symbol(self, symbol: str) -> tuple[int, ...]: diff --git a/pymatgen/io/vasp/sets.py b/pymatgen/io/vasp/sets.py index caf48aa98fd..fc509d03972 100644 --- a/pymatgen/io/vasp/sets.py +++ b/pymatgen/io/vasp/sets.py @@ -47,6 +47,7 @@ from glob import glob from itertools import chain from pathlib import Path +from typing import Any, Literal, Sequence from zipfile import ZipFile import numpy as np @@ -64,6 +65,10 @@ from pymatgen.symmetry.bandstructure import HighSymmKpath MODULE_DIR = Path(__file__).resolve().parent +# TODO (janosh): replace with following line once PMG is py3.9+ only +# which guarantees absolute path to __file__ +# reason: faster and shorter than pathlib (slow import and less OOP overhead) +# MODULE_DIR = os.path.dirname(__file__) class VaspInputSet(MSONable, metaclass=abc.ABCMeta): @@ -73,6 +78,8 @@ class VaspInputSet(MSONable, metaclass=abc.ABCMeta): class. Start from DictSet or MPRelaxSet or MITRelaxSet. """ + _valid_potcars: Sequence[str] | None = None + @property @abc.abstractmethod def incar(self): @@ -174,12 +181,12 @@ def write_input( with zopen(os.path.join(output_dir, key), "wt") as file: file.write(str(val)) else: - vinput = self.get_vasp_input() - vinput.write_input(output_dir, make_dir_if_not_present=make_dir_if_not_present) + vasp_input = self.get_vasp_input() + vasp_input.write_input(output_dir, make_dir_if_not_present=make_dir_if_not_present) cif_name = "" if include_cif: - s = vinput["POSCAR"].structure + s = vasp_input["POSCAR"].structure cif_name = f"{output_dir}/{s.formula.replace(' ', '')}.cif" s.to(filename=cif_name) @@ -205,10 +212,10 @@ def as_dict(self, verbosity=2): Returns: MSONable dict """ - d = MSONable.as_dict(self) + dct = MSONable.as_dict(self) if verbosity == 1: - d.pop("structure", None) - return d + dct.pop("structure", None) + return dct def _load_yaml_config(fname): @@ -250,24 +257,25 @@ class DictSet(VaspInputSet): def __init__( self, - structure, - config_dict, + structure: Structure, + config_dict: dict[str, Any], files_to_transfer=None, user_incar_settings=None, user_kpoints_settings=None, user_potcar_settings=None, - constrain_total_magmom=False, - sort_structure=True, - potcar_functional=None, - user_potcar_functional=None, - force_gamma=False, + constrain_total_magmom: bool = False, + sort_structure: bool = True, + user_potcar_functional: Literal[ + "PBE", "PBE_52", "PBE_54", "LDA", "LDA_52", "LDA_54", "PW91", "LDA_US", "PW91_US" + ] = None, + force_gamma: bool = False, reduce_structure=None, vdw=None, - use_structure_charge=False, - standardize=False, + use_structure_charge: bool = False, + standardize: bool = False, sym_prec=0.1, - international_monoclinic=True, - validate_magmom=True, + international_monoclinic: bool = True, + validate_magmom: bool = True, ): """ Args: @@ -335,6 +343,9 @@ def __init__( validate_magmom (bool): Ensure that the missing magmom values are filled in with the VASP default value of 1.0 """ + if (valid_potcars := self._valid_potcars) and user_potcar_functional not in valid_potcars: + raise ValueError(f"Invalid {user_potcar_functional=}, must be one of {valid_potcars}") + struct_has_Yb = any(specie.symbol == "Yb" for site in structure for specie in site.species) uses_Yb_2_psp = self.CONFIG["POTCAR"]["Yb"] == "Yb_2" if struct_has_Yb and uses_Yb_2_psp: @@ -349,7 +360,11 @@ def __init__( if sort_structure: structure = structure.get_sorted_structure() if validate_magmom: - get_valid_magmom_struct(structure, spin_mode="auto", inplace=True) # noqa: PD002 + get_valid_magmom_struct(structure, spin_mode="auto") + + if user_potcar_functional == "PBE_54" and "W" in structure.symbol_set: + # when using 5.4 POTCARs, default Tungsten POTCAR to W_Sv but still allow user to override + user_potcar_settings = {"W": "W_sv", **(user_potcar_settings or {})} self._structure = structure self._config_dict = deepcopy(config_dict) @@ -385,22 +400,8 @@ def __init__( raise KeyError( f"Invalid or unsupported van-der-Waals functional. Supported functionals are {', '.join(vdw_par)}." ) - # read the POTCAR_FUNCTIONAL from the .yaml - self.potcar_functional = self._config_dict.get("POTCAR_FUNCTIONAL", "PBE") - - if potcar_functional is not None and user_potcar_functional is not None: - raise ValueError( - "Received both 'potcar_functional' and 'user_potcar_functional arguments. " - "'potcar_functional is deprecated." - ) - if potcar_functional: - warnings.warn( - "'potcar_functional' argument is deprecated. Use 'user_potcar_functional' instead.", - FutureWarning, - ) - self.potcar_functional = potcar_functional - elif user_potcar_functional: - self.potcar_functional = user_potcar_functional + # 'or' case reads the POTCAR_FUNCTIONAL from the .yaml + self.potcar_functional = user_potcar_functional or self._config_dict.get("POTCAR_FUNCTIONAL", "PBE") # warn if a user is overriding POTCAR_FUNCTIONAL if self.potcar_functional != self._config_dict.get("POTCAR_FUNCTIONAL"): @@ -608,15 +609,15 @@ def nelect(self) -> float: """ Gets the default number of electrons for a given structure. """ - nelectrons_by_element = {p.element: p.nelectrons for p in self.potcar} - nelect = sum( - num_atoms * nelectrons_by_element[str(el)] + n_electrons_by_element = {p.element: p.nelectrons for p in self.potcar} + n_elect = sum( + num_atoms * n_electrons_by_element[str(el)] for el, num_atoms in self.structure.composition.element_composition.items() ) if self.use_structure_charge: - return nelect - self.structure.charge - return nelect + return n_elect - self.structure.charge + return n_elect @property def kpoints(self) -> Kpoints | None: @@ -638,7 +639,7 @@ def kpoints(self) -> Kpoints | None: ) and self.user_kpoints_settings == {}: return None - settings = self.user_kpoints_settings or self._config_dict.get("KPOINTS") + settings = self.user_kpoints_settings or self._config_dict.get("KPOINTS") or {} if isinstance(settings, Kpoints): return settings @@ -750,7 +751,6 @@ def calculate_ng(self, max_prime_factor: int = 7, must_inc_2: bool = True) -> tu _RYTOEV = 13.605826 _AUTOA = 0.529177249 - _PI = 3.141592653589793238 # TODO Only do this for VASP 6 for now. Older version require more advanced logic @@ -761,7 +761,7 @@ def calculate_ng(self, max_prime_factor: int = 7, must_inc_2: bool = True) -> tu encut = max(i_species.enmax for i_species in self.get_vasp_input()["POTCAR"]) _CUTOF = [ - np.sqrt(encut / _RYTOEV) / (2 * _PI / (anorm / _AUTOA)) for anorm in self.poscar.structure.lattice.abc + np.sqrt(encut / _RYTOEV) / (2 * np.pi / (anorm / _AUTOA)) for anorm in self.poscar.structure.lattice.abc ] _PREC = "Normal" # VASP default @@ -906,6 +906,7 @@ class MPScanRelaxSet(DictSet): """ CONFIG = _load_yaml_config("MPSCANRelaxSet") + _valid_potcars = ("PBE_52", "PBE_54") def __init__(self, structure: Structure, bandgap=0, **kwargs): """ @@ -936,13 +937,13 @@ def __init__(self, structure: Structure, bandgap=0, **kwargs): generalized Monkhorst-Pack grids through the use of informatics, Phys. Rev. B. 93 (2016) 1-10. doi:10.1103/PhysRevB.93.155109. """ + + kwargs.setdefault("user_potcar_functional", "PBE_54") + super().__init__(structure, MPScanRelaxSet.CONFIG, **kwargs) self.bandgap = bandgap self.kwargs = kwargs - if self.potcar_functional not in ["PBE_52", "PBE_54"]: - raise ValueError("SCAN calculations require PBE_52 or PBE_54!") - # self.kwargs.get("user_incar_settings", { updates = {} # select the KSPACING and smearing parameters based on the bandgap @@ -2307,15 +2308,7 @@ def incar(self): # The default incar setting is used for metallic system, for # insulator or semiconductor, ISMEAR need to be changed. incar.update( - { - "LCHARG": False, - "NELM": 60, - "PREC": "Normal", - "EDIFFG": -0.02, - "ICHARG": 0, - "NSW": 200, - "EDIFF": 0.0001, - } + {"LCHARG": False, "NELM": 60, "PREC": "Normal", "EDIFFG": -0.02, "ICHARG": 0, "NSW": 200, "EDIFF": 0.0001} ) if self.is_metal: @@ -2349,20 +2342,18 @@ class MVLRelax52Set(DictSet): """ CONFIG = _load_yaml_config("MVLRelax52Set") + _valid_potcars = ("PBE_52", "PBE_54") - def __init__(self, structure: Structure, **kwargs): + def __init__(self, structure: Structure, **kwargs) -> None: """ Args: structure (Structure): input structure. - potcar_functional (str): choose from "PBE_52" and "PBE_54". + user_potcar_functional (str): choose from "PBE_52" and "PBE_54". **kwargs: Other kwargs supported by :class:`DictSet`. """ - if kwargs.get("potcar_functional") or kwargs.get("user_potcar_functional"): - super().__init__(structure, MVLRelax52Set.CONFIG, **kwargs) - else: - super().__init__(structure, MVLRelax52Set.CONFIG, user_potcar_functional="PBE_52", **kwargs) - if self.potcar_functional not in ["PBE_52", "PBE_54"]: - raise ValueError("Please select from PBE_52 and PBE_54!") + kwargs.setdefault("user_potcar_functional", "PBE_52") + + super().__init__(structure, MVLRelax52Set.CONFIG, **kwargs) self.kwargs = kwargs @@ -2394,13 +2385,7 @@ def __init__(self, structures, unset_encut=False, **kwargs): self._config_dict["INCAR"]["EDIFF"] = self._config_dict["INCAR"].pop("EDIFF_PER_ATOM") # NEB specific defaults - defaults = { - "IMAGES": len(structures) - 2, - "IBRION": 1, - "ISYM": 0, - "LCHARG": False, - "LDAU": False, - } + defaults = {"IMAGES": len(structures) - 2, "IBRION": 1, "ISYM": 0, "LCHARG": False, "LDAU": False} self._config_dict["INCAR"].update(defaults) @property @@ -2707,6 +2692,8 @@ class MVLScanRelaxSet(MPRelaxSet): kinetic energy density (partial) """ + _valid_potcars = ("PBE_52", "PBE_54") + def __init__(self, structure: Structure, **kwargs): """ Args: @@ -2717,12 +2704,11 @@ def __init__(self, structure: Structure, **kwargs): **kwargs: Other kwargs supported by :class:`DictSet`. """ # choose PBE_52 unless the user specifies something else - if kwargs.get("potcar_functional") or kwargs.get("user_potcar_functional"): - super().__init__(structure, **kwargs) - else: - super().__init__(structure, user_potcar_functional="PBE_52", **kwargs) + kwargs.setdefault("user_potcar_functional", "PBE_52") + + super().__init__(structure, **kwargs) - if self.potcar_functional not in ["PBE_52", "PBE_54"]: + if self.potcar_functional not in ("PBE_52", "PBE_54"): raise ValueError("SCAN calculations required PBE_52 or PBE_54!") updates = { @@ -2747,6 +2733,7 @@ class LobsterSet(MPRelaxSet): """ CONFIG = _load_yaml_config("MPRelaxSet") + _valid_potcars = ("PBE_52", "PBE_54") def __init__( self, @@ -2756,7 +2743,6 @@ def __init__( reciprocal_density: int | None = None, address_basis_file: str | None = None, user_supplied_basis: dict | None = None, - user_potcar_settings: dict | None = None, **kwargs, ): """ @@ -2769,6 +2755,8 @@ def __init__( e.g. {"Fe": "3d 3p 4s", "O": "2s 2p"}; if not supplied, a standard basis is used address_basis_file (str): address to a file similar to "BASIS_PBE_54_standaard.yaml" in pymatgen.io.lobster.lobster_basis + user_potcar_settings (dict): dict including potcar settings for all elements in structure, + e.g. {"Fe": "Fe_pv", "O": "O"}; if not supplied, a standard basis is used. **kwargs: Other kwargs supported by :class:`DictSet`. """ from pymatgen.io.lobster import Lobsterin @@ -2782,10 +2770,9 @@ def __init__( # newest potcars are preferred # Choose PBE_54 unless the user specifies a different potcar_functional - if kwargs.get("potcar_functional") or kwargs.get("user_potcar_functional"): - super().__init__(structure, **kwargs) - else: - super().__init__(structure, user_potcar_functional="PBE_54", user_potcar_settings={"W": "W_sv"}, **kwargs) + kwargs.setdefault("user_potcar_functional", "PBE_54") + + super().__init__(structure, **kwargs) # reciprocal density if self.user_kpoints_settings is not None: diff --git a/pymatgen/io/vasp/tests/test_inputs.py b/pymatgen/io/vasp/tests/test_inputs.py index b17f2981918..2ec89c58e83 100644 --- a/pymatgen/io/vasp/tests/test_inputs.py +++ b/pymatgen/io/vasp/tests/test_inputs.py @@ -1115,17 +1115,17 @@ def setUp(self): potcar = Potcar.from_file(filepath) filepath = PymatgenTest.TEST_FILES_DIR / "KPOINTS.auto" kpoints = Kpoints.from_file(filepath) - self.vinput = VaspInput(incar, kpoints, poscar, potcar) + self.vasp_input = VaspInput(incar, kpoints, poscar, potcar) def test_to_from_dict(self): - d = self.vinput.as_dict() - vinput = VaspInput.from_dict(d) - comp = vinput["POSCAR"].structure.composition + d = self.vasp_input.as_dict() + vasp_input = VaspInput.from_dict(d) + comp = vasp_input["POSCAR"].structure.composition assert comp == Composition("Fe4P4O16") def test_write(self): tmp_dir = Path("VaspInput.testing") - self.vinput.write_input(tmp_dir) + self.vasp_input.write_input(tmp_dir) filepath = tmp_dir / "INCAR" incar = Incar.from_file(filepath) @@ -1139,7 +1139,7 @@ def test_write(self): def test_run_vasp(self): # To add some test. with ScratchDir(".") as d: - self.vinput.run_vasp(d, vasp_cmd=["cat", "INCAR"]) + self.vasp_input.run_vasp(d, vasp_cmd=["cat", "INCAR"]) with open(os.path.join(d, "vasp.out")) as f: output = f.read() assert output.split("\n")[0] == "ALGO = Damped" @@ -1149,8 +1149,8 @@ def test_from_directory(self): assert vi["INCAR"]["ALGO"] == "Damped" assert "CONTCAR.Li2O" in vi d = vi.as_dict() - vinput = VaspInput.from_dict(d) - assert "CONTCAR.Li2O" in vinput + vasp_input = VaspInput.from_dict(d) + assert "CONTCAR.Li2O" in vasp_input if __name__ == "__main__": diff --git a/pymatgen/io/vasp/tests/test_sets.py b/pymatgen/io/vasp/tests/test_sets.py index 8c6d24f66d7..cb06001c70a 100644 --- a/pymatgen/io/vasp/tests/test_sets.py +++ b/pymatgen/io/vasp/tests/test_sets.py @@ -69,6 +69,8 @@ [MPRelaxSet, MPHSERelaxSet, MVLRelax52Set, MPAbsorptionSet], ) def test_Yb_2_warning(input_set: VaspInputSet) -> None: + # https://github.com/materialsproject/pymatgen/pull/2972 + structure = Structure( lattice=Lattice.cubic(5), species=("Yb", "O"), @@ -176,11 +178,11 @@ def test_potcar_symbols(self): ] ) structure = Structure(lattice, ["P", "Fe", "O"], coords) - mitparamset = MITRelaxSet(structure) - syms = mitparamset.potcar_symbols + mit_param_set = MITRelaxSet(structure) + syms = mit_param_set.potcar_symbols assert syms == ["Fe", "P", "O"] - paramset = MPRelaxSet(structure, sort_structure=False) - syms = paramset.potcar_symbols + param_set = MPRelaxSet(structure, sort_structure=False) + syms = param_set.potcar_symbols assert syms == ["P", "Fe_pv", "O"] def test_potcar_validation(self): @@ -192,6 +194,20 @@ def test_potcar_validation(self): with pytest.warns(BadInputSetWarning, match="not known by pymatgen"): MITRelaxSet(structure).potcar + def test_potcar_special_defaults(self): + # https://github.com/materialsproject/pymatgen/pull/3022 + for user_potcar_settings in [{"Fe": "Fe_pv"}, {"W": "W_pv"}, None]: + for species in [("W", "W"), ("Fe", "W"), ("Fe", "Fe")]: + struct = Structure(lattice=Lattice.cubic(3), species=species, coords=[[0, 0, 0], [0.5, 0.5, 0.5]]) + relax_set = MPRelaxSet( + structure=struct, user_potcar_functional="PBE_54", user_potcar_settings=user_potcar_settings + ) + expected = { # noqa: SIM222 + **({"W": "W_sv"} if "W" in struct.symbol_set else {}), + **(user_potcar_settings or {}), + } or None + assert relax_set.user_potcar_settings == expected + @skip_if_no_psp_dir def test_lda_potcar(self): structure = Structure(self.lattice, ["P", "Fe"], self.coords) @@ -202,28 +218,28 @@ def test_lda_potcar(self): def test_nelect(self): coords = [[0] * 3, [0.5] * 3, [0.75] * 3] lattice = Lattice.cubic(4) - s = Structure(lattice, ["Si", "Si", "Fe"], coords) - assert MITRelaxSet(s).nelect == approx(16) + struct = Structure(lattice, ["Si", "Si", "Fe"], coords) + assert MITRelaxSet(struct).nelect == approx(16) # Test estimate of number of bands (function of nelect) with nmag>0 - assert MITRelaxSet(s).estimate_nbands() == approx(13) - assert MPRelaxSet(s).estimate_nbands() == approx(17) + assert MITRelaxSet(struct).estimate_nbands() == approx(13) + assert MPRelaxSet(struct).estimate_nbands() == approx(17) # Test estimate of number of bands (function of nelect) with nmag==0 - s = Structure(lattice, ["Si", "Si", "Si"], coords) - assert MITRelaxSet(s).estimate_nbands() == approx(11) - assert MPRelaxSet(s).estimate_nbands() == approx(11) + struct = Structure(lattice, ["Si", "Si", "Si"], coords) + assert MITRelaxSet(struct).estimate_nbands() == approx(11) + assert MPRelaxSet(struct).estimate_nbands() == approx(11) # Check that it works even when oxidation states are present. Was a bug # previously. - s = Structure(lattice, ["Si4+", "Si4+", "Fe2+"], coords) - assert MITRelaxSet(s).nelect == approx(16) - assert MPRelaxSet(s).nelect == approx(22) + struct = Structure(lattice, ["Si4+", "Si4+", "Fe2+"], coords) + assert MITRelaxSet(struct).nelect == approx(16) + assert MPRelaxSet(struct).nelect == approx(22) # Check that it works for disordered structure. Was a bug previously - s = Structure(lattice, ["Si4+", "Fe2+", "Si4+"], coords) - assert MITRelaxSet(s).nelect == approx(16) - assert MPRelaxSet(s).nelect == approx(22) + struct = Structure(lattice, ["Si4+", "Fe2+", "Si4+"], coords) + assert MITRelaxSet(struct).nelect == approx(16) + assert MPRelaxSet(struct).nelect == approx(22) @skip_if_no_psp_dir def test_get_incar(self): @@ -662,11 +678,6 @@ def test_write_input_zipped(self): os.remove("MPStaticSet.zip") - def test_conflicting_arguments(self): - si = self.get_structure("Si") - with pytest.raises(ValueError, match="deprecated"): - MPStaticSet(si, potcar_functional="PBE", user_potcar_functional="PBE") - def test_grid_size_from_struct(self): # TODO grab a bunch_of_calculations store as a list of tuples # (structure, ngx, ..., ngxf, ...) where all the grid size values are generated by vasp @@ -1327,6 +1338,21 @@ def test_potcar(self): with pytest.raises(ValueError): MVLScanRelaxSet(self.struct, user_potcar_functional="PBE") + # https://github.com/materialsproject/pymatgen/pull/3022 + # same test also in MITMPRelaxSetTest above (for redundancy, + # should apply to all classes inheriting from DictSet) + for user_potcar_settings in [{"Fe": "Fe_pv"}, {"W": "W_pv"}, None]: + for species in [("W", "W"), ("Fe", "W"), ("Fe", "Fe")]: + struct = Structure(lattice=Lattice.cubic(3), species=species, coords=[[0, 0, 0], [0.5, 0.5, 0.5]]) + relax_set = MPRelaxSet( + structure=struct, user_potcar_functional="PBE_54", user_potcar_settings=user_potcar_settings + ) + expected = { # noqa: SIM222 + **({"W": "W_sv"} if "W" in struct.symbol_set else {}), + **(user_potcar_settings or {}), + } or None + assert relax_set.user_potcar_settings == expected + def test_as_from_dict(self): d = self.mvl_scan_set.as_dict() v = dec.process_decoded(d) @@ -1544,11 +1570,6 @@ def test_override_from_prev_calc(self): assert lepsilon_vis.incar.get("NSW") is None assert lepsilon_vis.incar.get("NPAR") is None - def test_conflicting_arguments(self): - si = self.get_structure("Si") - with pytest.raises(ValueError, match="deprecated"): - MPScanStaticSet(si, potcar_functional="PBE", user_potcar_functional="PBE") - def tearDown(self): shutil.rmtree(self.tmp) warnings.simplefilter("default") @@ -1628,15 +1649,11 @@ def test_potcar(self): with pytest.raises(ValueError): MVLRelax52Set(self.struct, user_potcar_functional="PBE") - def test_potcar_functional_warning(self): - with pytest.warns(FutureWarning, match="argument is deprecated"): - MVLRelax52Set(self.struct, potcar_functional="PBE_52") - def test_as_from_dict(self): - d = self.mvl_rlx_set.as_dict() - v = dec.process_decoded(d) - assert isinstance(v, MVLRelax52Set) - assert v.incar["NSW"] == 500 + dct = self.mvl_rlx_set.as_dict() + vasp_input = dec.process_decoded(dct) + assert isinstance(vasp_input, MVLRelax52Set) + assert vasp_input.incar["NSW"] == 500 class LobsterSetTest(PymatgenTest):