Skip to content

Commit

Permalink
Breaking: Default user_potcar_settings to {"W": "W_sv"} in all in…
Browse files Browse the repository at this point in the history
…put sets if `user_potcar_functional == "PBE_54"` (#3022)

* DictSet drop deprecated kwarg potcar_functional: str = None, use user_potcar_functional: str = None instead

* default user_potcar_settings to {"W": "W_sv"} in all input sets if user_potcar_functional == "PBE_54"

* fix typos

* fix TypeError: 'NoneType' object is not iterable

in kwargs["user_potcar_settings"] = {"W": "W_sv"}.update(kwargs.get("user_potcar_settings", {}))

pymatgen/io/vasp/sets.py:2352

also address #3022 (comment) by moving {"W": "W_sv"} logic into DictSet

* remove test_potcar_functional_warning()

* VaspInputSet add static attr _valid_potcars: Sequence[str] | None = None

for DRY ValueError on invalid user_potcar_functional

* only default user_potcar_settings to {"W": "W_sv"} if structure contains tungsten

also fix #3022 (comment)

* pymatgen/io/vasp/tests/test_sets.py add test_potcar_special_defaults()
  • Loading branch information
janosh authored Jun 1, 2023
1 parent 3c91e69 commit d25319a
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 124 deletions.
4 changes: 2 additions & 2 deletions pymatgen/core/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, ...]:
Expand Down
145 changes: 66 additions & 79 deletions pymatgen/io/vasp/sets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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)

Expand All @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -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"):
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2707,6 +2692,8 @@ class MVLScanRelaxSet(MPRelaxSet):
kinetic energy density (partial)
"""

_valid_potcars = ("PBE_52", "PBE_54")

def __init__(self, structure: Structure, **kwargs):
"""
Args:
Expand All @@ -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 = {
Expand All @@ -2747,6 +2733,7 @@ class LobsterSet(MPRelaxSet):
"""

CONFIG = _load_yaml_config("MPRelaxSet")
_valid_potcars = ("PBE_52", "PBE_54")

def __init__(
self,
Expand All @@ -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,
):
"""
Expand All @@ -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
Expand All @@ -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:
Expand Down
16 changes: 8 additions & 8 deletions pymatgen/io/vasp/tests/test_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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"
Expand All @@ -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__":
Expand Down
Loading

0 comments on commit d25319a

Please sign in to comment.