From 5992c6c269b745f83a1410406194948c79dcf580 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Wed, 3 May 2023 18:02:21 -0700 Subject: [PATCH 01/12] fix AttributeError no .exception in pytest.raises() exc_info (closes #2971) --- .../analysis/tests/test_interface_reactions.py | 14 +++++++------- pymatgen/io/vasp/tests/test_sets.py | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pymatgen/analysis/tests/test_interface_reactions.py b/pymatgen/analysis/tests/test_interface_reactions.py index 3e0cd51f8dd..de87c4a0f2e 100644 --- a/pymatgen/analysis/tests/test_interface_reactions.py +++ b/pymatgen/analysis/tests/test_interface_reactions.py @@ -145,13 +145,13 @@ def setUp(self): norm=True, use_hull_energy=False, ) - with pytest.raises(Exception) as context1: + with pytest.raises(Exception) as exc_info: _ = InterfacialReactivity(Composition("Li2O2"), Composition("Li"), pd=self.gpd, norm=True) - assert ( - str(context1.exception) == "Please use the GrandPotentialInterfacialReactivity " - "class for interfacial reactions with open elements!" - ) - with pytest.raises(Exception) as context2: + assert ( + str(exc_info.value) == "Please use the GrandPotentialInterfacialReactivity " + "class for interfacial reactions with open elements!" + ) + with pytest.raises(Exception) as exc_info: _ = GrandPotentialInterfacialReactivity( Composition("O2"), Composition("Mn"), @@ -160,7 +160,7 @@ def setUp(self): norm=False, include_no_mixing_energy=True, ) - assert str(context2.exception) == "Please provide non-grand phase diagram to compute no_mixing_energy!" + assert str(exc_info.value) == "Please provide non-grand phase diagram to compute no_mixing_energy!" self.ir = [ir_0, ir_1, ir_2, ir_3, ir_4, ir_5, ir_6, ir_7, ir_8, ir_9, ir_10, ir_11, ir_12] diff --git a/pymatgen/io/vasp/tests/test_sets.py b/pymatgen/io/vasp/tests/test_sets.py index 9d85ebeaa79..d465af2791d 100644 --- a/pymatgen/io/vasp/tests/test_sets.py +++ b/pymatgen/io/vasp/tests/test_sets.py @@ -500,9 +500,9 @@ def test_valid_magmom_struct(self): struct = self.structure.copy() get_valid_magmom_struct(structure=struct, inplace=True, spin_mode="v") struct.insert(0, "Li", [0, 0, 0], properties={"magmom": 10.0}) - with pytest.raises(TypeError) as context: + with pytest.raises(TypeError) as exc_info: get_valid_magmom_struct(structure=struct, inplace=True, spin_mode="a") - assert "Magmom type conflict" in str(context.exception) + assert "Magmom type conflict" in str(exc_info.value) # Test the behavior of MPRelaxSet to atomacically fill in the missing magmom struct = self.structure.copy() @@ -510,10 +510,10 @@ def test_valid_magmom_struct(self): struct.insert(0, "Li", [0, 0, 0]) vis = MPRelaxSet(struct, user_potcar_settings={"Fe": "Fe"}, validate_magmom=False) - with pytest.raises(TypeError) as context: + with pytest.raises(TypeError) as exc_info: print(vis.get_vasp_input()) - assert "argument must be a string" in str(context.exception) + assert "argument must be a string" in str(exc_info.value) vis = MPRelaxSet(struct, user_potcar_settings={"Fe": "Fe"}, validate_magmom=True) assert vis.get_vasp_input()["INCAR"]["MAGMOM"] == [1.0] * len(struct) From 739decf5ce9f01d6d012681f6920c5a843b79a76 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Wed, 3 May 2023 18:04:50 -0700 Subject: [PATCH 02/12] pymatgen/io/vasp/tests/test_sets.py only skip tests that are actually failing PMG_VASP_PSP_DIR --- pymatgen/io/vasp/tests/test_sets.py | 50 ++++++++++++----------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/pymatgen/io/vasp/tests/test_sets.py b/pymatgen/io/vasp/tests/test_sets.py index d465af2791d..b9f162378de 100644 --- a/pymatgen/io/vasp/tests/test_sets.py +++ b/pymatgen/io/vasp/tests/test_sets.py @@ -15,7 +15,7 @@ from _pytest.monkeypatch import MonkeyPatch from monty.json import MontyDecoder from monty.serialization import loadfn -from pytest import approx +from pytest import approx, mark from pymatgen.analysis.structure_matcher import StructureMatcher from pymatgen.core import SETTINGS, Lattice, Species, Structure @@ -57,8 +57,9 @@ dec = MontyDecoder() -if SETTINGS.get("PMG_VASP_PSP_DIR") is None: - raise unittest.SkipTest(f"PMG_VASP_PSP_DIR is not set. Skipping tests in {__file__}.") + +NO_PSP_DIR = SETTINGS.get("PMG_VASP_PSP_DIR") is None +skip_if_no_psp_dir = mark.skipif(NO_PSP_DIR, reason="PMG_VASP_PSP_DIR is not set.") class SetChangeCheckTest(PymatgenTest): @@ -174,11 +175,13 @@ def test_potcar_validation(self): with pytest.warns(BadInputSetWarning, match="not known by pymatgen"): MITRelaxSet(structure).potcar + @skip_if_no_psp_dir def test_lda_potcar(self): structure = Structure(self.lattice, ["P", "Fe"], self.coords) p = MITRelaxSet(structure, user_potcar_functional="LDA").potcar assert p.functional == "LDA" + @skip_if_no_psp_dir def test_nelect(self): coords = [[0] * 3, [0.5] * 3, [0.75] * 3] lattice = Lattice.cubic(4) @@ -205,6 +208,7 @@ def test_nelect(self): assert MITRelaxSet(s).nelect == approx(16) assert MPRelaxSet(s).nelect == approx(22) + @skip_if_no_psp_dir def test_get_incar(self): incar = self.mpset.incar @@ -222,13 +226,7 @@ def test_get_incar(self): # Silicon structure for testing. latt = Lattice( - np.array( - [ - [3.8401979337, 0.00, 0.00], - [1.9200989668, 3.3257101909, 0.00], - [0.00, -2.2171384943, 3.1355090603], - ] - ) + [[3.8401979337, 0.00, 0.00], [1.9200989668, 3.3257101909, 0.00], [0.00, -2.2171384943, 3.1355090603]] ) struct = Structure(latt, [si, si], coords) incar = MPRelaxSet(struct).incar @@ -331,11 +329,7 @@ def test_get_incar(self): s = Structure(lattice, ["Fe", "Cl", "S"], coords) incar = MITRelaxSet( s, - user_incar_settings={ - "LDAU": True, - "LDAUL": {"Fe": 3}, - "LDAUU": {"Fe": 1.8}, - }, + user_incar_settings={"LDAU": True, "LDAUL": {"Fe": 3}, "LDAUU": {"Fe": 1.8}}, ).incar assert incar["LDAUL"] == [3.0, 0, 0] assert incar["LDAUU"] == [1.8, 0, 0] @@ -354,13 +348,7 @@ def test_get_incar(self): # Silicon structure for testing. latt = Lattice( - np.array( - [ - [3.8401979337, 0.00, 0.00], - [1.9200989668, 3.3257101909, 0.00], - [0.00, -2.2171384943, 3.1355090603], - ] - ) + [[3.8401979337, 0.00, 0.00], [1.9200989668, 3.3257101909, 0.00], [0.00, -2.2171384943, 3.1355090603]] ) struct = Structure(latt, [si, si], coords, charge=1) mpr = MPRelaxSet(struct, use_structure_charge=True) @@ -410,6 +398,7 @@ def test_get_kpoints(self): assert kpoints.kpts == [[2, 4, 5]] assert kpoints.style == Kpoints.supported_modes.Gamma + @skip_if_no_psp_dir def test_get_vasp_input(self): d = self.mitset.get_vasp_input() assert d["INCAR"]["ISMEAR"] == -5 @@ -420,17 +409,17 @@ def test_get_vasp_input(self): assert d["INCAR"]["ISMEAR"] == 0 def test_MPMetalRelaxSet(self): - mpmetalset = MPMetalRelaxSet(self.get_structure("Sn")) - incar = mpmetalset.incar + mp_metal_set = MPMetalRelaxSet(self.get_structure("Sn")) + incar = mp_metal_set.incar assert incar["ISMEAR"] == 1 assert incar["SIGMA"] == 0.2 - kpoints = mpmetalset.kpoints + kpoints = mp_metal_set.kpoints self.assertArrayAlmostEqual(kpoints.kpts[0], [5, 5, 5]) def test_as_from_dict(self): mitset = MITRelaxSet(self.structure) - mpset = MPRelaxSet(self.structure) - mpuserset = MPRelaxSet( + mp_set = MPRelaxSet(self.structure) + mp_user_set = MPRelaxSet( self.structure, user_incar_settings={"MAGMOM": {"Fe": 10, "S": -5, "Mn3+": 100}}, ) @@ -439,11 +428,11 @@ def test_as_from_dict(self): v = dec.process_decoded(d) assert v._config_dict["INCAR"]["LDAUU"]["O"]["Fe"] == 4 - d = mpset.as_dict() + d = mp_set.as_dict() v = dec.process_decoded(d) assert v._config_dict["INCAR"]["LDAUU"]["O"]["Fe"] == 5.3 - d = mpuserset.as_dict() + d = mp_user_set.as_dict() v = dec.process_decoded(d) # self.assertEqual(type(v), MPVaspInputSet) assert v.user_incar_settings["MAGMOM"] == {"Fe": 10, "S": -5, "Mn3+": 100} @@ -456,6 +445,7 @@ def test_hubbard_off_and_ediff_override(self): # even if U is turned off (thanks Andrew Rosen for reporting) assert p.incar["LMAXMIX"] == 4 + @skip_if_no_psp_dir def test_write_input(self): self.mitset.write_input(".", make_dir_if_not_present=True) for f in ["INCAR", "KPOINTS", "POSCAR", "POTCAR"]: @@ -476,11 +466,13 @@ def test_write_input(self): for f in ["INCAR", "KPOINTS", "POSCAR", "POTCAR.spec"]: os.remove(f) + @skip_if_no_psp_dir def test_user_potcar_settings(self): vis = MPRelaxSet(self.structure, user_potcar_settings={"Fe": "Fe"}) potcar = vis.potcar assert potcar.symbols == ["Fe", "P", "O"] + @skip_if_no_psp_dir def test_valid_magmom_struct(self): # First test the helper function struct = self.structure.copy() From 256f9dbec504624a3317acad4b66e8a02d1a758f Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Wed, 3 May 2023 18:05:48 -0700 Subject: [PATCH 03/12] simplify: no need to convert list to array before passing to Lattice --- pymatgen/core/tests/test_lattice.py | 28 +++++++++++++--------------- pymatgen/core/tests/test_sites.py | 2 +- pymatgen/io/tests/test_cif.py | 24 ++++++++++-------------- pymatgen/io/vasp/inputs.py | 2 +- 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/pymatgen/core/tests/test_lattice.py b/pymatgen/core/tests/test_lattice.py index 13e2ca5402f..ca4e96dab00 100644 --- a/pymatgen/core/tests/test_lattice.py +++ b/pymatgen/core/tests/test_lattice.py @@ -185,7 +185,7 @@ def test_get_lll_reduced_lattice(self): lattice = Lattice([1.0, 1, 1, -1.0, 0, 2, 3.0, 5, 6]) reduced_latt = lattice.get_lll_reduced_lattice() - expected_ans = Lattice(np.array([0.0, 1.0, 0.0, 1.0, 0.0, 1.0, -2.0, 0.0, 1.0]).reshape((3, 3))) + expected_ans = Lattice([0.0, 1.0, 0.0, 1.0, 0.0, 1.0, -2.0, 0.0, 1.0]).reshape((3, 3)) assert round(abs(np.linalg.det(np.linalg.solve(expected_ans.matrix, reduced_latt.matrix)) - 1), 7) == 0 self.assertArrayAlmostEqual(sorted(reduced_latt.abc), sorted(expected_ans.abc)) assert round(abs(reduced_latt.volume - lattice.volume), 7) == 0 @@ -201,19 +201,17 @@ def test_get_lll_reduced_lattice(self): 14.253000, ] expected_ans = Lattice( - np.array( - [ - -4.298850, - 2.481942, - 0.000000, - 2.865900, - 4.963884, - 0.000000, - 0.000000, - 0.000000, - 14.253000, - ] - ) + [ + -4.298850, + 2.481942, + 0.000000, + 2.865900, + 4.963884, + 0.000000, + 0.000000, + 0.000000, + 14.253000, + ] ) reduced_latt = Lattice(latt).get_lll_reduced_lattice() assert round(abs(np.linalg.det(np.linalg.solve(expected_ans.matrix, reduced_latt.matrix)) - 1), 7) == 0 @@ -481,7 +479,7 @@ def test_get_distance_and_image_strict(self): for _ in range(10): lengths = [np.random.randint(1, 100) for i in range(3)] lattice = [np.random.rand(3) * lengths[i] for i in range(3)] - lattice = Lattice(np.array(lattice)) + lattice = Lattice(lattice) f1 = np.random.rand(3) f2 = np.random.rand(3) diff --git a/pymatgen/core/tests/test_sites.py b/pymatgen/core/tests/test_sites.py index 01b5fa2a0f6..d4eded397c2 100644 --- a/pymatgen/core/tests/test_sites.py +++ b/pymatgen/core/tests/test_sites.py @@ -127,7 +127,7 @@ def test_distance_and_image(self): assert round(abs(distance - 19.461500456028563), 5) == 0 # Test that old and new distance algo give the same ans for # "standard lattices" - lattice = Lattice(np.array([[1, 0, 0], [0, 1, 0], [0, 0, 1]])) + lattice = Lattice([[1, 0, 0], [0, 1, 0], [0, 0, 1]]) site1 = PeriodicSite("Fe", np.array([0.01, 0.02, 0.03]), lattice) site2 = PeriodicSite("Fe", np.array([0.99, 0.98, 0.97]), lattice) assert round(abs(get_distance_and_image_old(site1, site2)[0] - site1.distance_and_image(site2)[0]), 7) == 0 diff --git a/pymatgen/io/tests/test_cif.py b/pymatgen/io/tests/test_cif.py index 8c48b268323..ecbbc3884b9 100644 --- a/pymatgen/io/tests/test_cif.py +++ b/pymatgen/io/tests/test_cif.py @@ -552,13 +552,11 @@ def test_disordered(self): coords.append(np.array([0, 0, 0])) coords.append(np.array([0.75, 0.5, 0.75])) lattice = Lattice( - np.array( - [ - [3.8401979337, 0.00, 0.00], - [1.9200989668, 3.3257101909, 0.00], - [0.00, -2.2171384943, 3.1355090603], - ] - ) + [ + [3.8401979337, 0.00, 0.00], + [1.9200989668, 3.3257101909, 0.00], + [0.00, -2.2171384943, 3.1355090603], + ] ) struct = Structure(lattice, [si, {si: 0.5, n: 0.5}], coords) writer = CifWriter(struct) @@ -612,13 +610,11 @@ def test_specie_cifwriter(self): coords.append(np.array([0.75, 0.5, 0.75])) coords.append(np.array([0, 0, 0])) lattice = Lattice( - np.array( - [ - [3.8401979337, 0.00, 0.00], - [1.9200989668, 3.3257101909, 0.00], - [0.00, -2.2171384943, 3.1355090603], - ] - ) + [ + [3.8401979337, 0.00, 0.00], + [1.9200989668, 3.3257101909, 0.00], + [0.00, -2.2171384943, 3.1355090603], + ] ) struct = Structure(lattice, [n, {si3: 0.5, n: 0.5}, si4], coords) writer = CifWriter(struct) diff --git a/pymatgen/io/vasp/inputs.py b/pymatgen/io/vasp/inputs.py index 54da79f251e..2a27aac67cd 100644 --- a/pymatgen/io/vasp/inputs.py +++ b/pymatgen/io/vasp/inputs.py @@ -1178,7 +1178,7 @@ def monkhorst_automatic(kpts: tuple[int, int, int] = (2, 2, 2), shift: tuple[flo grid. Args: - kpts: Subdivisions N_1, N_2 and N_3 along reciprocal lattice + kpts: Subdivisions N_1, N_2, N_3 along reciprocal lattice vectors. Defaults to (2,2,2) shift: Shift to be applied to the kpoints. Defaults to (0,0,0). From d9bbfdad96ad89f3afe38c5e079309692672346d Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Wed, 3 May 2023 18:06:15 -0700 Subject: [PATCH 04/12] fix operator precedence in DictSet.kpoints --- pymatgen/io/vasp/sets.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pymatgen/io/vasp/sets.py b/pymatgen/io/vasp/sets.py index 51b9fb02f76..a538f447d32 100644 --- a/pymatgen/io/vasp/sets.py +++ b/pymatgen/io/vasp/sets.py @@ -631,10 +631,8 @@ def kpoints(self) -> Kpoints | None: # Return None if KSPACING is present in the INCAR, because this will # cause VASP to generate the kpoints automatically if ( - self.user_incar_settings.get("KSPACING") - or self._config_dict["INCAR"].get("KSPACING") - and self.user_kpoints_settings == {} - ): + self.user_incar_settings.get("KSPACING") or self._config_dict["INCAR"].get("KSPACING") + ) and self.user_kpoints_settings == {}: return None settings = self.user_kpoints_settings or self._config_dict.get("KPOINTS") @@ -1142,7 +1140,7 @@ def kpoints(self) -> Kpoints | None: kpoints = super().kpoints # Prefer to use k-point scheme from previous run - # except for when lepsilon = True is specified + # unless lepsilon = True is specified if kpoints is not None and self.prev_kpoints and self.prev_kpoints.style != kpoints.style: if (self.prev_kpoints.style == Kpoints.supported_modes.Monkhorst) and (not self.lepsilon): k_div = [kp + 1 if kp % 2 == 1 else kp for kp in kpoints.kpts[0]] # type: ignore From aa8eee4c72a24113dd6a057e4e47eae9e0ba295a Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Thu, 4 May 2023 08:46:09 -0700 Subject: [PATCH 05/12] refactor glob.glob imports --- pymatgen/analysis/transition_state.py | 10 +++--- pymatgen/apps/borg/hive.py | 21 +++++------- pymatgen/cli/pmg_config.py | 10 +++--- pymatgen/command_line/bader_caller.py | 6 ++-- pymatgen/command_line/chargemol_caller.py | 4 +-- pymatgen/command_line/critic2_caller.py | 4 +-- pymatgen/command_line/enumlib_caller.py | 4 +-- pymatgen/io/cp2k/outputs.py | 40 +++++++++++------------ pymatgen/io/lammps/outputs.py | 4 +-- pymatgen/io/vasp/inputs.py | 4 +-- pymatgen/io/vasp/outputs.py | 4 +-- pymatgen/io/vasp/sets.py | 23 +++++-------- pymatgen/io/vasp/tests/test_sets.py | 4 +-- 13 files changed, 64 insertions(+), 74 deletions(-) diff --git a/pymatgen/analysis/transition_state.py b/pymatgen/analysis/transition_state.py index ea80f2fdf05..30a95dac915 100644 --- a/pymatgen/analysis/transition_state.py +++ b/pymatgen/analysis/transition_state.py @@ -8,8 +8,8 @@ from __future__ import annotations -import glob import os +from glob import glob import numpy as np from monty.json import MSONable, jsanitize @@ -254,14 +254,14 @@ def from_dir(cls, root_dir, relaxation_dirs=None, **kwargs): terminal_dirs.append([os.path.join(root_dir, d) for d in ["initial", "final"]]) for i, d in neb_dirs: - outcar = glob.glob(os.path.join(d, "OUTCAR*")) - contcar = glob.glob(os.path.join(d, "CONTCAR*")) - poscar = glob.glob(os.path.join(d, "POSCAR*")) + outcar = glob(os.path.join(d, "OUTCAR*")) + contcar = glob(os.path.join(d, "CONTCAR*")) + poscar = glob(os.path.join(d, "POSCAR*")) terminal = i in [0, neb_dirs[-1][0]] if terminal: for ds in terminal_dirs: od = ds[0] if i == 0 else ds[1] - outcar = glob.glob(os.path.join(od, "OUTCAR*")) + outcar = glob(os.path.join(od, "OUTCAR*")) if outcar: outcar = sorted(outcar) outcars.append(Outcar(outcar[-1])) diff --git a/pymatgen/apps/borg/hive.py b/pymatgen/apps/borg/hive.py index 9dda3f527a1..2f9669578b8 100644 --- a/pymatgen/apps/borg/hive.py +++ b/pymatgen/apps/borg/hive.py @@ -5,11 +5,11 @@ from __future__ import annotations import abc -import glob import json import logging import os import warnings +from glob import glob from monty.io import zopen from monty.json import MSONable @@ -118,9 +118,9 @@ def assimilate(self, path): """ files = os.listdir(path) if "relax1" in files and "relax2" in files: - filepath = glob.glob(os.path.join(path, "relax2", "vasprun.xml*"))[0] + filepath = glob(os.path.join(path, "relax2", "vasprun.xml*"))[0] else: - vasprun_files = glob.glob(os.path.join(path, "vasprun.xml*")) + vasprun_files = glob(os.path.join(path, "vasprun.xml*")) filepath = None if len(vasprun_files) == 1: filepath = vasprun_files[0] @@ -159,11 +159,8 @@ def get_valid_paths(self, path): (not parent.endswith("/relax1")) and (not parent.endswith("/relax2")) and ( - len(glob.glob(os.path.join(parent, "vasprun.xml*"))) > 0 - or ( - len(glob.glob(os.path.join(parent, "POSCAR*"))) > 0 - and len(glob.glob(os.path.join(parent, "OSZICAR*"))) > 0 - ) + len(glob(os.path.join(parent, "vasprun.xml*"))) > 0 + or (len(glob(os.path.join(parent, "POSCAR*"))) > 0 and len(glob(os.path.join(parent, "OSZICAR*"))) > 0) ) ): return [parent] @@ -233,13 +230,13 @@ def assimilate(self, path): if "relax1" in files and "relax2" in files: for filename in ("INCAR", "POTCAR", "POSCAR"): search_str = os.path.join(path, "relax1", filename + "*") - files_to_parse[filename] = glob.glob(search_str)[0] + files_to_parse[filename] = glob(search_str)[0] for filename in ("CONTCAR", "OSZICAR"): search_str = os.path.join(path, "relax2", filename + "*") - files_to_parse[filename] = glob.glob(search_str)[-1] + files_to_parse[filename] = glob(search_str)[-1] else: for filename in filenames: - files = sorted(glob.glob(os.path.join(path, filename + "*"))) + files = sorted(glob(os.path.join(path, filename + "*"))) if len(files) == 1 or filename in ("INCAR", "POTCAR") or len(files) == 1 and filename == "DYNMAT": files_to_parse[filename] = files[0] elif len(files) > 1: @@ -440,7 +437,7 @@ def _get_transformation_history(path): """ Checks for a transformations.json* file and returns the history. """ - trans_json = glob.glob(os.path.join(path, "transformations.json*")) + trans_json = glob(os.path.join(path, "transformations.json*")) if trans_json: try: with zopen(trans_json[0]) as f: diff --git a/pymatgen/cli/pmg_config.py b/pymatgen/cli/pmg_config.py index 85be07ab9a7..c2d128c01f0 100644 --- a/pymatgen/cli/pmg_config.py +++ b/pymatgen/cli/pmg_config.py @@ -6,11 +6,11 @@ from __future__ import annotations -import glob import os import shutil import subprocess from argparse import Namespace +from glob import glob from typing import Literal from urllib.request import urlretrieve @@ -31,8 +31,6 @@ def setup_cp2k_data(cp2k_data_dirs: list[str]) -> None: raise SystemExit(0) print("Generating pymatgen resource directory for CP2K...") - import glob - from monty.json import jsanitize from ruamel import yaml @@ -40,8 +38,8 @@ def setup_cp2k_data(cp2k_data_dirs: list[str]) -> None: from pymatgen.io.cp2k.inputs import GaussianTypeOrbitalBasisSet, GthPotential from pymatgen.io.cp2k.utils import chunk - basis_files = glob.glob(os.path.join(data_dir, "*BASIS*")) - potential_files = glob.glob(os.path.join(data_dir, "*POTENTIAL*")) + basis_files = glob(os.path.join(data_dir, "*BASIS*")) + potential_files = glob(os.path.join(data_dir, "*POTENTIAL*")) settings: dict[str, dict] = {str(el): {"potentials": {}, "basis_sets": {}} for el in Element} @@ -141,7 +139,7 @@ def setup_potcars(potcar_dirs: list[str]): basename = os.path.basename(parent) basename = name_mappings.get(basename, basename) for subdir in subdirs: - filenames = glob.glob(os.path.join(parent, subdir, "POTCAR*")) + filenames = glob(os.path.join(parent, subdir, "POTCAR*")) if len(filenames) > 0: try: base_dir = os.path.join(target_dir, basename) diff --git a/pymatgen/command_line/bader_caller.py b/pymatgen/command_line/bader_caller.py index 17d516cde7c..b01cc73a502 100644 --- a/pymatgen/command_line/bader_caller.py +++ b/pymatgen/command_line/bader_caller.py @@ -14,11 +14,11 @@ from __future__ import annotations -import glob import os import shutil import subprocess import warnings +from glob import glob from shutil import which import numpy as np @@ -418,7 +418,7 @@ def from_path(cls, path, suffix=""): def _get_filepath(filename): name_pattern = filename + suffix + "*" if filename != "POTCAR" else filename + "*" - paths = glob.glob(os.path.join(path, name_pattern)) + paths = glob(os.path.join(path, name_pattern)) fpath = None if len(paths) >= 1: # using reverse=True because, if multiple files are present, @@ -478,7 +478,7 @@ def bader_analysis_from_path(path, suffix=""): """ def _get_filepath(filename, warning, path=path, suffix=suffix): - paths = glob.glob(os.path.join(path, filename + suffix + "*")) + paths = glob(os.path.join(path, filename + suffix + "*")) if not paths: warnings.warn(warning) return None diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py index 010a392076a..4616c5f15e8 100644 --- a/pymatgen/command_line/chargemol_caller.py +++ b/pymatgen/command_line/chargemol_caller.py @@ -42,11 +42,11 @@ """ from __future__ import annotations -import glob import os import shutil import subprocess import warnings +from glob import glob from shutil import which import numpy as np @@ -155,7 +155,7 @@ def _get_filepath(path, filename, suffix=""): (str): Absolute path to the file. """ name_pattern = f"{filename}{suffix}*" if filename != "POTCAR" else f"{filename}*" - paths = glob.glob(os.path.join(path, name_pattern)) + paths = glob(os.path.join(path, name_pattern)) fpath = None if len(paths) >= 1: # using reverse=True because, if multiple files are present, diff --git a/pymatgen/command_line/critic2_caller.py b/pymatgen/command_line/critic2_caller.py index 0b051345bd5..c6a085a336f 100644 --- a/pymatgen/command_line/critic2_caller.py +++ b/pymatgen/command_line/critic2_caller.py @@ -39,12 +39,12 @@ from __future__ import annotations -import glob import logging import os import subprocess import warnings from enum import Enum +from glob import glob from shutil import which import numpy as np @@ -322,7 +322,7 @@ def get_filepath(filename, warning, path, suffix): path: Path to search suffix: Suffixes to search. """ - paths = glob.glob(os.path.join(path, filename + suffix + "*")) + paths = glob(os.path.join(path, filename + suffix + "*")) if not paths: warnings.warn(warning) return None diff --git a/pymatgen/command_line/enumlib_caller.py b/pymatgen/command_line/enumlib_caller.py index f7aee65c2e2..a859561e32f 100644 --- a/pymatgen/command_line/enumlib_caller.py +++ b/pymatgen/command_line/enumlib_caller.py @@ -27,12 +27,12 @@ from __future__ import annotations import fractions -import glob import itertools import logging import math import re import subprocess +from glob import glob from shutil import which from threading import Timer @@ -364,7 +364,7 @@ def _get_structures(self, num_structs): ordered_structure = None # to fix pylint E0601 inv_org_latt = None - for file in glob.glob("vasp.*"): + for file in glob("vasp.*"): with open(file) as f: data = f.read() data = re.sub(r"scale factor", "1", data) diff --git a/pymatgen/io/cp2k/outputs.py b/pymatgen/io/cp2k/outputs.py index 48833428068..99c321d2618 100644 --- a/pymatgen/io/cp2k/outputs.py +++ b/pymatgen/io/cp2k/outputs.py @@ -6,11 +6,11 @@ from __future__ import annotations -import glob import logging import os import re import warnings +from glob import glob from itertools import chain import numpy as np @@ -246,8 +246,8 @@ def parse_files(self): Identify files present in the directory with the cp2k output file. Looks for trajectories, dos, and cubes """ - self.filenames["DOS"] = glob.glob(os.path.join(self.dir, "*.dos*")) - pdos = glob.glob(os.path.join(self.dir, "*pdos*")) + self.filenames["DOS"] = glob(os.path.join(self.dir, "*.dos*")) + pdos = glob(os.path.join(self.dir, "*pdos*")) self.filenames["PDOS"] = [] self.filenames["LDOS"] = [] for p in pdos: @@ -255,22 +255,22 @@ def parse_files(self): self.filenames["LDOS"].append(p) else: self.filenames["PDOS"].append(p) - self.filenames["band_structure"] = glob.glob(os.path.join(self.dir, "*BAND.bs*")) - self.filenames["trajectory"] = glob.glob(os.path.join(self.dir, "*pos*.xyz*")) - self.filenames["forces"] = glob.glob(os.path.join(self.dir, "*frc*.xyz*")) - self.filenames["stress"] = glob.glob(os.path.join(self.dir, "*stress*")) - self.filenames["cell"] = glob.glob(os.path.join(self.dir, "*.cell*")) - self.filenames["ener"] = glob.glob(os.path.join(self.dir, "*.ener*")) - self.filenames["electron_density"] = glob.glob(os.path.join(self.dir, "*ELECTRON_DENSITY*.cube*")) - self.filenames["spin_density"] = glob.glob(os.path.join(self.dir, "*SPIN_DENSITY*.cube*")) - self.filenames["v_hartree"] = glob.glob(os.path.join(self.dir, "*hartree*.cube*")) - self.filenames["hyperfine_tensor"] = glob.glob(os.path.join(self.dir, "*HYPERFINE*eprhyp*")) - self.filenames["g_tensor"] = glob.glob(os.path.join(self.dir, "*GTENSOR*data*")) - self.filenames["spinspin_tensor"] = glob.glob(os.path.join(self.dir, "*K*data*")) - self.filenames["chi_tensor"] = glob.glob(os.path.join(self.dir, "*CHI*data*")) - self.filenames["nmr_shift"] = glob.glob(os.path.join(self.dir, "*SHIFT*data*")) - self.filenames["raman"] = glob.glob(os.path.join(self.dir, "*raman*data*")) - restart = glob.glob(os.path.join(self.dir, "*restart*")) + self.filenames["band_structure"] = glob(os.path.join(self.dir, "*BAND.bs*")) + self.filenames["trajectory"] = glob(os.path.join(self.dir, "*pos*.xyz*")) + self.filenames["forces"] = glob(os.path.join(self.dir, "*frc*.xyz*")) + self.filenames["stress"] = glob(os.path.join(self.dir, "*stress*")) + self.filenames["cell"] = glob(os.path.join(self.dir, "*.cell*")) + self.filenames["ener"] = glob(os.path.join(self.dir, "*.ener*")) + self.filenames["electron_density"] = glob(os.path.join(self.dir, "*ELECTRON_DENSITY*.cube*")) + self.filenames["spin_density"] = glob(os.path.join(self.dir, "*SPIN_DENSITY*.cube*")) + self.filenames["v_hartree"] = glob(os.path.join(self.dir, "*hartree*.cube*")) + self.filenames["hyperfine_tensor"] = glob(os.path.join(self.dir, "*HYPERFINE*eprhyp*")) + self.filenames["g_tensor"] = glob(os.path.join(self.dir, "*GTENSOR*data*")) + self.filenames["spinspin_tensor"] = glob(os.path.join(self.dir, "*K*data*")) + self.filenames["chi_tensor"] = glob(os.path.join(self.dir, "*CHI*data*")) + self.filenames["nmr_shift"] = glob(os.path.join(self.dir, "*SHIFT*data*")) + self.filenames["raman"] = glob(os.path.join(self.dir, "*raman*data*")) + restart = glob(os.path.join(self.dir, "*restart*")) self.filenames["restart.bak"] = [] self.filenames["restart"] = [] for r in restart: @@ -279,7 +279,7 @@ def parse_files(self): else: self.filenames["restart"].append(r) - wfn = glob.glob(os.path.join(self.dir, "*.wfn*")) + glob.glob(os.path.join(self.dir, "*.kp*")) + wfn = glob(os.path.join(self.dir, "*.wfn*")) + glob(os.path.join(self.dir, "*.kp*")) self.filenames["wfn.bak"] = [] for w in wfn: if "bak" in w.split("/")[-1]: diff --git a/pymatgen/io/lammps/outputs.py b/pymatgen/io/lammps/outputs.py index 96a56868b4e..56260c9ea04 100644 --- a/pymatgen/io/lammps/outputs.py +++ b/pymatgen/io/lammps/outputs.py @@ -5,8 +5,8 @@ from __future__ import annotations -import glob import re +from glob import glob from io import StringIO import numpy as np @@ -109,7 +109,7 @@ def parse_lammps_dumps(file_pattern): LammpsDump for each available snapshot. """ - files = glob.glob(file_pattern) + files = glob(file_pattern) if len(files) > 1: pattern = file_pattern.replace("*", "([0-9]+)").replace("\\", "\\\\") files = sorted(files, key=lambda f: int(re.match(pattern, f).group(1))) diff --git a/pymatgen/io/vasp/inputs.py b/pymatgen/io/vasp/inputs.py index 2a27aac67cd..5234d428063 100644 --- a/pymatgen/io/vasp/inputs.py +++ b/pymatgen/io/vasp/inputs.py @@ -5,7 +5,6 @@ from __future__ import annotations -import glob import itertools import json import logging @@ -16,6 +15,7 @@ import warnings from collections import namedtuple from enum import Enum +from glob import glob from hashlib import md5, sha256 from typing import Any, Literal, Sequence @@ -244,7 +244,7 @@ def from_file(filename, check_for_POTCAR=True, read_velocities=True) -> Poscar: dirname = os.path.dirname(os.path.abspath(filename)) names = None if check_for_POTCAR: - potcars = glob.glob(os.path.join(dirname, "*POTCAR*")) + potcars = glob(os.path.join(dirname, "*POTCAR*")) if potcars: try: potcar = Potcar.from_file(sorted(potcars)[0]) diff --git a/pymatgen/io/vasp/outputs.py b/pymatgen/io/vasp/outputs.py index b85b47b0cb3..f9cf60a9ecf 100644 --- a/pymatgen/io/vasp/outputs.py +++ b/pymatgen/io/vasp/outputs.py @@ -5,7 +5,6 @@ from __future__ import annotations import datetime -import glob import itertools import logging import math @@ -15,6 +14,7 @@ import xml.etree.ElementTree as ET from collections import defaultdict from dataclasses import dataclass +from glob import glob from io import StringIO from pathlib import Path from typing import DefaultDict, Literal @@ -4114,7 +4114,7 @@ def get_band_structure_from_vasp_multiple_branches(dir_name, efermi=None, projec # TODO: Add better error handling!!! if os.path.exists(os.path.join(dir_name, "branch_0")): # get all branch dir names - branch_dir_names = [os.path.abspath(d) for d in glob.glob(f"{dir_name}/branch_*") if os.path.isdir(d)] + branch_dir_names = [os.path.abspath(d) for d in glob(f"{dir_name}/branch_*") if os.path.isdir(d)] # sort by the directory name (e.g, branch_10) sorted_branch_dir_names = sorted(branch_dir_names, key=lambda x: int(x.split("_")[-1])) diff --git a/pymatgen/io/vasp/sets.py b/pymatgen/io/vasp/sets.py index a538f447d32..1636b2df818 100644 --- a/pymatgen/io/vasp/sets.py +++ b/pymatgen/io/vasp/sets.py @@ -38,13 +38,13 @@ from __future__ import annotations import abc -import glob import itertools import os import re import shutil import warnings from copy import deepcopy +from glob import glob from itertools import chain from pathlib import Path from zipfile import ZipFile @@ -134,12 +134,7 @@ def get_vasp_input(self) -> VaspInput: Returns: VaspInput """ - return VaspInput( - incar=self.incar, - kpoints=self.kpoints, - poscar=self.poscar, - potcar=self.potcar, - ) + return VaspInput(incar=self.incar, kpoints=self.kpoints, poscar=self.poscar, potcar=self.potcar) def write_input( self, @@ -1454,7 +1449,7 @@ def override_from_prev_calc(self, prev_calc_dir="."): files_to_transfer = {} if self.copy_chgcar: - chgcars = sorted(glob.glob(str(Path(prev_calc_dir) / "CHGCAR*"))) + chgcars = sorted(glob(str(Path(prev_calc_dir) / "CHGCAR*"))) if chgcars: files_to_transfer["CHGCAR"] = str(chgcars[-1]) @@ -1680,7 +1675,7 @@ def override_from_prev_calc(self, prev_calc_dir="."): files_to_transfer = {} if self.copy_chgcar: - chgcars = sorted(glob.glob(str(Path(prev_calc_dir) / "CHGCAR*"))) + chgcars = sorted(glob(str(Path(prev_calc_dir) / "CHGCAR*"))) if chgcars: files_to_transfer["CHGCAR"] = str(chgcars[-1]) @@ -1829,7 +1824,7 @@ def override_from_prev_calc(self, prev_calc_dir="."): files_to_transfer = {} if self.copy_chgcar: - chgcars = sorted(glob.glob(str(Path(prev_calc_dir) / "CHGCAR*"))) + chgcars = sorted(glob(str(Path(prev_calc_dir) / "CHGCAR*"))) if chgcars: files_to_transfer["CHGCAR"] = str(chgcars[-1]) @@ -2095,7 +2090,7 @@ def override_from_prev_calc(self, prev_calc_dir="."): files_to_transfer = {} if self.copy_wavecar: for fname in ("WAVECAR", "WAVEDER", "WFULL"): - w = sorted(glob.glob(str(Path(prev_calc_dir) / (fname + "*")))) + w = sorted(glob(str(Path(prev_calc_dir) / (fname + "*")))) if w: if fname == "WFULL": for f in w: @@ -2847,8 +2842,8 @@ def get_vasprun_outcar(path, parse_dos=True, parse_eigen=True): :return: """ path = Path(path) - vruns = list(glob.glob(str(path / "vasprun.xml*"))) - outcars = list(glob.glob(str(path / "OUTCAR*"))) + vruns = list(glob(str(path / "vasprun.xml*"))) + outcars = list(glob(str(path / "OUTCAR*"))) if len(vruns) == 0 or len(outcars) == 0: raise ValueError(f"Unable to get vasprun.xml/OUTCAR from prev calculation in {path}") @@ -3227,7 +3222,7 @@ def override_from_prev_calc(self, prev_calc_dir=".", **kwargs): files_to_transfer = {} if self.copy_wavecar: for fname in ("WAVECAR", "WAVEDER"): - w = sorted(glob.glob(str(Path(prev_calc_dir) / (fname + "*")))) + w = sorted(glob(str(Path(prev_calc_dir) / (fname + "*")))) if w: files_to_transfer[fname] = str(w[-1]) diff --git a/pymatgen/io/vasp/tests/test_sets.py b/pymatgen/io/vasp/tests/test_sets.py index b9f162378de..e3f9cc6c681 100644 --- a/pymatgen/io/vasp/tests/test_sets.py +++ b/pymatgen/io/vasp/tests/test_sets.py @@ -1,12 +1,12 @@ from __future__ import annotations -import glob import hashlib import os import shutil import tempfile import unittest import warnings +from glob import glob from pathlib import Path from zipfile import ZipFile @@ -71,7 +71,7 @@ def test_sets_changed(self): # For sets starting with "MVL" this is @shyuep, for sets starting # with "MP" this is @shyuep and @mkhorton. os.chdir(MODULE_DIR / "..") - input_sets = glob.glob("*.yaml") + input_sets = glob("*.yaml") hashes = {} for input_set in input_sets: with open(input_set) as f: From 325d864521e9987640ae68a90466734e1f2f4a6c Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Fri, 5 May 2023 13:41:41 -0700 Subject: [PATCH 06/12] issue warning in class DictSet if input structure contains Yb and input set uses Yb_2 PSP --- pymatgen/io/vasp/MPAbsorptionSet.yaml | 6 ++++-- pymatgen/io/vasp/MPHSERelaxSet.yaml | 5 +++-- pymatgen/io/vasp/MPRelaxSet.yaml | 6 ++++-- pymatgen/io/vasp/MVLRelax52Set.yaml | 6 ++++-- pymatgen/io/vasp/sets.py | 11 ++++++++++- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/pymatgen/io/vasp/MPAbsorptionSet.yaml b/pymatgen/io/vasp/MPAbsorptionSet.yaml index c815c6508d6..9a382b4d510 100644 --- a/pymatgen/io/vasp/MPAbsorptionSet.yaml +++ b/pymatgen/io/vasp/MPAbsorptionSet.yaml @@ -110,8 +110,10 @@ POTCAR: W: W_pv Xe: Xe Y: Y_sv - # 2023-05-02: change Yb_2 to Yb_3 as Yb_2 gives incorrect thermodynamics for most systems with Yb3+ + # 2023-05-02: change Yb_2 to Yb_3 if POTCAR_FUNCTIONAL=PBE_54 else issue warning if Yb present + # since Yb_3 didn't exist prior to PBE_54 + # reason: Yb_2 gives incorrect thermodynamics for most systems with Yb3+ # https://github.com/materialsproject/pymatgen/issues/2968 - Yb: Yb_3 + Yb: Yb_2 Zn: Zn Zr: Zr_sv diff --git a/pymatgen/io/vasp/MPHSERelaxSet.yaml b/pymatgen/io/vasp/MPHSERelaxSet.yaml index 30411041add..1e5b89c671c 100644 --- a/pymatgen/io/vasp/MPHSERelaxSet.yaml +++ b/pymatgen/io/vasp/MPHSERelaxSet.yaml @@ -109,8 +109,9 @@ POTCAR: W: W_pv Xe: Xe Y: Y_sv - # 2023-05-02: change Yb_2 to Yb_3 as Yb_2 gives incorrect thermodynamics for most systems with Yb3+ + # 2023-05-02: change Yb_2 to Yb_3 if POTCAR_FUNCTIONAL=PBE_54 else remove + # as Yb_2 gives incorrect thermodynamics for most systems with Yb3+ # https://github.com/materialsproject/pymatgen/issues/2968 - Yb: Yb_3 + Yb: Yb_2 Zn: Zn Zr: Zr_sv diff --git a/pymatgen/io/vasp/MPRelaxSet.yaml b/pymatgen/io/vasp/MPRelaxSet.yaml index 90ddca66074..bfe7f242bf0 100644 --- a/pymatgen/io/vasp/MPRelaxSet.yaml +++ b/pymatgen/io/vasp/MPRelaxSet.yaml @@ -168,8 +168,10 @@ POTCAR: W: W_pv Xe: Xe Y: Y_sv - # 2023-05-02: change Yb_2 to Yb_3 as Yb_2 gives incorrect thermodynamics for most systems with Yb3+ + # 2023-05-02: change Yb_2 to Yb_3 if POTCAR_FUNCTIONAL=PBE_54 else issue warning if Yb present + # since Yb_3 didn't exist prior to PBE_54 + # reason: Yb_2 gives incorrect thermodynamics for most systems with Yb3+ # https://github.com/materialsproject/pymatgen/issues/2968 - Yb: Yb_3 + Yb: Yb_2 Zn: Zn Zr: Zr_sv diff --git a/pymatgen/io/vasp/MVLRelax52Set.yaml b/pymatgen/io/vasp/MVLRelax52Set.yaml index 2f5f048dae6..40ebc1c9fa3 100644 --- a/pymatgen/io/vasp/MVLRelax52Set.yaml +++ b/pymatgen/io/vasp/MVLRelax52Set.yaml @@ -174,8 +174,10 @@ POTCAR: W: W_pv Xe: Xe Y: Y_sv - # 2023-05-02: change Yb_2 to Yb_3 as Yb_2 gives incorrect thermodynamics for most systems with Yb3+ + # 2023-05-02: change Yb_2 to Yb_3 if POTCAR_FUNCTIONAL=PBE_54 else issue warning if Yb present + # since Yb_3 didn't exist prior to PBE_54 + # reason: Yb_2 gives incorrect thermodynamics for most systems with Yb3+ # https://github.com/materialsproject/pymatgen/issues/2968 - Yb: Yb_3 + Yb: Yb_2 Zn: Zn Zr: Zr_sv diff --git a/pymatgen/io/vasp/sets.py b/pymatgen/io/vasp/sets.py index 1636b2df818..008b01c8c87 100644 --- a/pymatgen/io/vasp/sets.py +++ b/pymatgen/io/vasp/sets.py @@ -212,7 +212,7 @@ def as_dict(self, verbosity=2): def _load_yaml_config(fname): - config = loadfn(str(MODULE_DIR / (f"{fname}.yaml"))) + config = loadfn(MODULE_DIR / (f"{fname}.yaml")) if "PARENT" in config: parent_config = _load_yaml_config(config["PARENT"]) for k, v in parent_config.items(): @@ -335,6 +335,15 @@ def __init__( validate_magmom (bool): Ensure that the missing magmom values are filled in with the VASP default value of 1.0 """ + struct_has_Yb = any(specie.symbol == "Yb" for site in structure for specie in site.species) + uses_Yb_2_psp = self.CONFIG["POTCAR"]["Yb"].lower() == "yb_2" + if struct_has_Yb and uses_Yb_2_psp: + warnings.warn( + "The structure contains Ytterbium (Yb) and this InputSet uses the Yb_2 PSP.\n" + "Yb_2 is known to often give bad results since Yb has oxidation state 3+ in most compounds.\n" + "See https://github.com/materialsproject/pymatgen/issues/2968 for details.", + BadInputSetWarning, + ) if reduce_structure: structure = structure.get_reduced_structure(reduce_structure) if sort_structure: From b236140e680d7982dbbb56cfeb152b4927da316b Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Fri, 5 May 2023 13:42:42 -0700 Subject: [PATCH 07/12] test all pre-PBE_54 input sets issue a Yb_2 warning --- pymatgen/io/vasp/tests/test_sets.py | 61 ++++++++++++++++++----------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/pymatgen/io/vasp/tests/test_sets.py b/pymatgen/io/vasp/tests/test_sets.py index e3f9cc6c681..476eab909b5 100644 --- a/pymatgen/io/vasp/tests/test_sets.py +++ b/pymatgen/io/vasp/tests/test_sets.py @@ -31,6 +31,7 @@ MITRelaxSet, MPAbsorptionSet, MPHSEBSSet, + MPHSERelaxSet, MPMetalRelaxSet, MPNMRSet, MPNonSCFSet, @@ -46,6 +47,7 @@ MVLRelax52Set, MVLScanRelaxSet, MVLSlabSet, + VaspInputSet, batch_write_input, get_structure_from_prev_run, get_valid_magmom_struct, @@ -62,40 +64,55 @@ skip_if_no_psp_dir = mark.skipif(NO_PSP_DIR, reason="PMG_VASP_PSP_DIR is not set.") +@pytest.mark.parametrize( + "input_set", + [MPRelaxSet, MPHSERelaxSet, MVLRelax52Set, MPAbsorptionSet], +) +def test_Yb_2_warning(input_set: VaspInputSet) -> None: + structure = Structure( + lattice=Lattice.cubic(5), + species=("Yb", "O"), + coords=((0, 0, 0), (0.5, 0.5, 0.5)), + ) + + with pytest.warns(BadInputSetWarning) as record: + input_set(structure) + + expected = "The structure contains Ytterbium (Yb) and this InputSet uses the Yb_2" + assert expected in str(record[0].message) + + class SetChangeCheckTest(PymatgenTest): def test_sets_changed(self): - # WARNING! - # These tests will fail when you change an input set. - # They are included as a sanity check: if you want to change - # an input set, please make sure to notify the users for that set. - # For sets starting with "MVL" this is @shyuep, for sets starting - # with "MP" this is @shyuep and @mkhorton. + msg = ( + "WARNING! These tests will fail when you change an input set. They are included " + "as a sanity check: if you want to change an input set, please make sure to " + "notify the users for that set. For sets starting with 'MVL' this is @shyuep, for " + "sets starting with 'MP' this is @shyuep and @mkhorton. " + ) os.chdir(MODULE_DIR / "..") input_sets = glob("*.yaml") hashes = {} for input_set in input_sets: - with open(input_set) as f: - hashes[input_set] = hashlib.sha1(f.read().encode("utf-8")).hexdigest() + with open(input_set) as file: + text = file.read().encode("utf-8") + hashes[input_set] = hashlib.sha1(text).hexdigest() + known_hashes = { - "MVLGWSet.yaml": "f4df9516cf7dd923b37281172c662a70fa32bebc", - "MVLRelax52Set.yaml": "eb538ffb45c0cd13f13df48afc1e71c44d2e34b2", - "MPHSERelaxSet.yaml": "2bb969e64b57ff049077c8ec10e64f94c9c97f42", + "MVLGWSet.yaml": "104ae93c3b3be19a13b0ee46ebdd0f40ceb96597", + "MVLRelax52Set.yaml": "4cfc6b1bd0548e45da3bde4a9c65b3249da13ecd", + "MPHSERelaxSet.yaml": "0d0d96a620461071cfd416ec9d5d6a8d2dfd0855", "VASPIncarBase.yaml": "19762515f8deefb970f2968fca48a0d67f7964d4", - "MPSCANRelaxSet.yaml": "dfa9fee19178cb38c6a121ce6096db40693478e8", - "MPRelaxSet.yaml": "4ea97d776fbdc7e168036f73e9176012a56c0a45", + "MPSCANRelaxSet.yaml": "6bdda39a8802166ed5407916dc217b138b3602e9", + "MPRelaxSet.yaml": "f2949cdc5dc8cd0bee6d39a5df0d6a6b7c144821", "MITRelaxSet.yaml": "1a0970f8cad9417ec810f7ab349dc854eaa67010", "vdW_parameters.yaml": "04bb09bb563d159565bcceac6a11e8bdf0152b79", - "MPAbsorptionSet.yaml": "e86e405a014a7af41490cc7b99609f99f2ddd5b0", + "MPAbsorptionSet.yaml": "5931e1cb3cf8ba809b3d4f4a5960d728c682adf1", } - assert hashes == known_hashes, ( - "These tests will fail when you change an input set. " - "They are included as a sanity check: if you want to " - "change an input set, please make sure to notify the " - "users for that set. " - 'For sets starting with "MVL" this is @shyuep, ' - 'for sets starting with "MP" this is @shyuep and @mkhorton.' - ) + assert hashes == known_hashes, msg + for input_set in hashes: + assert hashes[input_set] == known_hashes[input_set], msg class MITMPRelaxSetTest(PymatgenTest): From caf604237899b479b214f9bed150a653c857ea14 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Fri, 5 May 2023 19:57:34 -0700 Subject: [PATCH 08/12] fix CI error assert str(exc_info.value) == "Please provide non-grand phase diagram to compute no_mixing_energy!" --- pymatgen/analysis/interface_reactions.py | 2 ++ .../tests/test_interface_reactions.py | 15 +++++----- pymatgen/core/tests/test_lattice.py | 28 +++---------------- 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/pymatgen/analysis/interface_reactions.py b/pymatgen/analysis/interface_reactions.py index cea5ed67ea4..c1f88ab6ed3 100644 --- a/pymatgen/analysis/interface_reactions.py +++ b/pymatgen/analysis/interface_reactions.py @@ -660,6 +660,8 @@ def __init__( """ if not isinstance(grand_pd, GrandPotentialPhaseDiagram): raise ValueError("Please use the InterfacialReactivity class if using a regular phase diagram!") + if not isinstance(pd_non_grand, PhaseDiagram): + raise ValueError("Please provide non-grand phase diagram to compute no_mixing_energy!") super().__init__( c1=c1, c2=c2, pd=grand_pd, norm=norm, use_hull_energy=use_hull_energy, bypass_grand_warning=True diff --git a/pymatgen/analysis/tests/test_interface_reactions.py b/pymatgen/analysis/tests/test_interface_reactions.py index de87c4a0f2e..b8b4d0b9d94 100644 --- a/pymatgen/analysis/tests/test_interface_reactions.py +++ b/pymatgen/analysis/tests/test_interface_reactions.py @@ -157,8 +157,8 @@ def setUp(self): Composition("Mn"), grand_pd=self.gpd, pd_non_grand=None, - norm=False, - include_no_mixing_energy=True, + # norm=False, + # include_no_mixing_energy=True, ) assert str(exc_info.value) == "Please provide non-grand phase diagram to compute no_mixing_energy!" @@ -166,15 +166,14 @@ def setUp(self): def test_get_entry_energy(self): comp = Composition("MnO3") - with warnings.catch_warnings(record=True) as w: + with warnings.catch_warnings(record=True) as record: warnings.simplefilter("always") energy = InterfacialReactivity._get_entry_energy(self.pd, comp) - assert len(w) == 1 + assert len(record) == 1 assert ( - "The reactant MnO3 has no matching entry with" - " negative formation energy, instead convex " - "hull energy for this composition will be used" - " for reaction energy calculation." in str(w[-1].message) + "The reactant MnO3 has no matching entry with negative formation energy, " + "instead convex hull energy for this composition will be used" + " for reaction energy calculation." in str(record[-1].message) ) test1 = np.isclose(energy, -30, atol=1e-03) assert test1, f"_get_entry_energy: energy for {comp.reduced_formula} is wrong!" diff --git a/pymatgen/core/tests/test_lattice.py b/pymatgen/core/tests/test_lattice.py index ca4e96dab00..2e9bc302df0 100644 --- a/pymatgen/core/tests/test_lattice.py +++ b/pymatgen/core/tests/test_lattice.py @@ -182,36 +182,16 @@ def _identical(a, b, c, alpha, beta, gamma): assert not _identical(2, 3, 4, 100, 100, 100) def test_get_lll_reduced_lattice(self): - lattice = Lattice([1.0, 1, 1, -1.0, 0, 2, 3.0, 5, 6]) + lattice = Lattice([1, 1, 1, -1, 0, 2, 3, 5, 6]) reduced_latt = lattice.get_lll_reduced_lattice() - expected_ans = Lattice([0.0, 1.0, 0.0, 1.0, 0.0, 1.0, -2.0, 0.0, 1.0]).reshape((3, 3)) + expected_ans = Lattice([[0, 1, 0], [1, 0, 1], [-2, 0, 1]]) assert round(abs(np.linalg.det(np.linalg.solve(expected_ans.matrix, reduced_latt.matrix)) - 1), 7) == 0 self.assertArrayAlmostEqual(sorted(reduced_latt.abc), sorted(expected_ans.abc)) assert round(abs(reduced_latt.volume - lattice.volume), 7) == 0 - latt = [ - 7.164750, - 2.481942, - 0.000000, - -4.298850, - 2.481942, - 0.000000, - 0.000000, - 0.000000, - 14.253000, - ] + latt = [7.164750, 2.481942, 0.000000, -4.298850, 2.481942, 0.000000, 0.000000, 0.000000, 14.253000] expected_ans = Lattice( - [ - -4.298850, - 2.481942, - 0.000000, - 2.865900, - 4.963884, - 0.000000, - 0.000000, - 0.000000, - 14.253000, - ] + [-4.298850, 2.481942, 0.000000, 2.865900, 4.963884, 0.000000, 0.000000, 0.000000, 14.253000] ) reduced_latt = Lattice(latt).get_lll_reduced_lattice() assert round(abs(np.linalg.det(np.linalg.solve(expected_ans.matrix, reduced_latt.matrix)) - 1), 7) == 0 From 8c932301fcc0db34fdf1388cc8c683687136eff5 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Fri, 5 May 2023 20:28:49 -0700 Subject: [PATCH 09/12] fix AttributeError: 'CommentedMap' object has no attribute 'lower' --- pymatgen/io/vasp/sets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pymatgen/io/vasp/sets.py b/pymatgen/io/vasp/sets.py index 008b01c8c87..57f6201ea77 100644 --- a/pymatgen/io/vasp/sets.py +++ b/pymatgen/io/vasp/sets.py @@ -336,7 +336,7 @@ def __init__( in with the VASP default value of 1.0 """ struct_has_Yb = any(specie.symbol == "Yb" for site in structure for specie in site.species) - uses_Yb_2_psp = self.CONFIG["POTCAR"]["Yb"].lower() == "yb_2" + uses_Yb_2_psp = self.CONFIG["POTCAR"]["Yb"] == "Yb_2" if struct_has_Yb and uses_Yb_2_psp: warnings.warn( "The structure contains Ytterbium (Yb) and this InputSet uses the Yb_2 PSP.\n" From 359fed2e5f8295b4be3e11922cae3b48c11a8b26 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 6 May 2023 07:12:05 -0700 Subject: [PATCH 10/12] test_sets.py sprinkle more skip_if_no_psp_dir to skip all tests in CI requiring POTCARs to pass --- pymatgen/io/vasp/tests/test_sets.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pymatgen/io/vasp/tests/test_sets.py b/pymatgen/io/vasp/tests/test_sets.py index 476eab909b5..12ce005521f 100644 --- a/pymatgen/io/vasp/tests/test_sets.py +++ b/pymatgen/io/vasp/tests/test_sets.py @@ -691,6 +691,7 @@ def setUp(self): self.tmp = tempfile.mkdtemp() warnings.simplefilter("ignore") + @skip_if_no_psp_dir def test_init(self): prev_run = self.TEST_FILES_DIR / "relaxation" # check boltztrap mode @@ -742,6 +743,7 @@ def test_init(self): vis.write_input(self.tmp) assert not os.path.exists(os.path.join(self.tmp, "CHGCAR")) + @skip_if_no_psp_dir def test_override_from_prev(self): prev_run = self.TEST_FILES_DIR / "relaxation" @@ -911,6 +913,7 @@ def test_as_from_dict(self): assert v._config_dict["INCAR"]["PREC"] == "Low" +@skip_if_no_psp_dir class MVLNPTMDSetTest(PymatgenTest): def setUp(self): file_path = self.TEST_FILES_DIR / "POSCAR" @@ -987,6 +990,7 @@ def test_as_from_dict(self): v = dec.process_decoded(d) assert v._config_dict["INCAR"]["IMAGES"] == 2 + @skip_if_no_psp_dir def test_write_input(self): self.vis.write_input(".", write_cif=True, write_endpoint_inputs=True, write_path_cif=True) assert os.path.exists("INCAR") @@ -1072,6 +1076,7 @@ def test_incar(self): assert vis.incar.get("QUAD_EFG") == [-40.1] +@skip_if_no_psp_dir class MVLSlabSetTest(PymatgenTest): def setUp(self): s = self.get_structure("Li2O") @@ -1161,6 +1166,7 @@ def test_incar(self): assert "NPAR" not in incar +@skip_if_no_psp_dir class MVLGWSetTest(PymatgenTest): def setUp(self): self.tmp = tempfile.mkdtemp() @@ -1311,6 +1317,7 @@ def test_incar(self): scan_rvv10_set = MVLScanRelaxSet(self.struct, vdw="rVV10") assert scan_rvv10_set.incar["BPARAM"] == 15.7 + @skip_if_no_psp_dir def test_potcar(self): assert self.mvl_scan_set.potcar.functional == "PBE_52" @@ -1408,6 +1415,7 @@ def test_other_vdw(self): assert "LUSE_VDW" not in scan_vdw_set.incar assert "IVDW" not in scan_vdw_set.incar + @skip_if_no_psp_dir def test_potcar(self): assert self.mp_scan_set.potcar.functional == "PBE_52" @@ -1425,6 +1433,7 @@ def test_as_from_dict(self): assert v._config_dict["INCAR"]["METAGGA"] == "R2SCAN" assert v.user_incar_settings["NSW"] == 500 + @skip_if_no_psp_dir def test_write_input(self): self.mp_scan_set.write_input(".") assert os.path.exists("INCAR") @@ -1546,6 +1555,7 @@ def tearDown(self): class FuncTest(PymatgenTest): + @skip_if_no_psp_dir def test_batch_write_input(self): structures = [ PymatgenTest.get_structure("Li2O"), @@ -1559,6 +1569,7 @@ def test_batch_write_input(self): shutil.rmtree(d) +@skip_if_no_psp_dir class MVLGBSetTest(PymatgenTest): def setUp(self): filepath = self.TEST_FILES_DIR / "Li.cif" @@ -1605,6 +1616,7 @@ def test_incar(self): assert "NSW" in incar assert incar["LREAL"] == "Auto" + @skip_if_no_psp_dir def test_potcar(self): assert self.mvl_rlx_set.potcar.functional == "PBE_54" assert "Fe" in self.mvl_rlx_set.potcar.symbols @@ -1685,6 +1697,7 @@ def test_kpoints(self): kpoints3 = self.lobsterset3.kpoints assert kpoints3.comment.split(" ")[6], 6000 + @skip_if_no_psp_dir def test_potcar(self): # PBE_54 is preferred at the moment assert self.lobsterset1.potcar_functional == "PBE_54" @@ -1707,6 +1720,7 @@ def test_as_from_dict(self): assert lobsterset_new.potcar_functional == "PBE_54" +@skip_if_no_psp_dir class MPAbsorptionSetTest(PymatgenTest): def setUp(self): self.tmp = tempfile.mkdtemp() From 490fbc50f69f7172ebae2cf74f0196b72edc3d8e Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 6 May 2023 07:19:24 -0700 Subject: [PATCH 11/12] ruff enable PIE (flake8-pie) fix 3 PIE804 Unnecessary `dict` kwargs --- pymatgen/io/ase.py | 6 +++--- pyproject.toml | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pymatgen/io/ase.py b/pymatgen/io/ase.py index de0d510f184..d15e10049f6 100644 --- a/pymatgen/io/ase.py +++ b/pymatgen/io/ase.py @@ -87,11 +87,11 @@ def get_atoms(structure, **kwargs): charges = structure.site_properties["final_charge"] if "final_charge" in structure.site_properties else None if magmoms or charges: if magmoms and charges: - calc = SinglePointDFTCalculator(atoms, **{"magmoms": magmoms, "charges": charges}) + calc = SinglePointDFTCalculator(atoms, magmoms=magmoms, charges=charges) elif magmoms: - calc = SinglePointDFTCalculator(atoms, **{"magmoms": magmoms}) + calc = SinglePointDFTCalculator(atoms, magmoms=magmoms) elif charges: - calc = SinglePointDFTCalculator(atoms, **{"charges": charges}) + calc = SinglePointDFTCalculator(atoms, charges=charges) atoms.calc = calc # Get the oxidation states from the structure diff --git a/pyproject.toml b/pyproject.toml index 28e11a7de95..62493093d84 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,6 +32,7 @@ select = [ "E", # pycodestyle "F", # pyflakes "I", # isort + "PIE", # flake8-pie "PLE", # pylint error "PLW", # pylint warning "Q", # flake8-quotes From d1171dde59e4d2fdc65ede72ba70749cefe8db13 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 6 May 2023 14:20:39 +0000 Subject: [PATCH 12/12] pre-commit auto-fixes --- pymatgen/core/tests/test_trajectory.py | 10 +++++----- pymatgen/io/lammps/inputs.py | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pymatgen/core/tests/test_trajectory.py b/pymatgen/core/tests/test_trajectory.py index ba5eade4c93..351ef18065e 100644 --- a/pymatgen/core/tests/test_trajectory.py +++ b/pymatgen/core/tests/test_trajectory.py @@ -71,7 +71,7 @@ def _get_species_and_coords(self): def test_single_index_slice(self): assert all(self.traj[i] == self.structures[i] for i in range(0, len(self.structures), 19)) - assert all([self.traj_mols[i] == self.molecules[i] for i in range(0, len(self.molecules))]) + assert all(self.traj_mols[i] == self.molecules[i] for i in range(0, len(self.molecules))) def test_slice(self): sliced_traj = self.traj[2:99:3] @@ -94,7 +94,7 @@ def test_slice(self): sliced_traj_from_mols = Trajectory.from_molecules(self.molecules[0:2]) if len(sliced_traj) == len(sliced_traj_from_mols): - assert all([sliced_traj[i] == sliced_traj_from_mols[i] for i in range(len(sliced_traj))]) + assert all(sliced_traj[i] == sliced_traj_from_mols[i] for i in range(len(sliced_traj))) else: raise AssertionError @@ -102,7 +102,7 @@ def test_slice(self): sliced_traj_from_mols = Trajectory.from_molecules(self.molecules[:-2]) if len(sliced_traj) == len(sliced_traj_from_mols): - assert all([sliced_traj[i] == sliced_traj_from_mols[i] for i in range(len(sliced_traj))]) + assert all(sliced_traj[i] == sliced_traj_from_mols[i] for i in range(len(sliced_traj))) else: raise AssertionError @@ -119,7 +119,7 @@ def test_list_slice(self): sliced_traj_from_mols = Trajectory.from_molecules([self.molecules[i] for i in [1, 3]]) if len(sliced_traj) == len(sliced_traj_from_mols): - assert all([sliced_traj[i] == sliced_traj_from_mols[i] for i in range(len(sliced_traj))]) + assert all(sliced_traj[i] == sliced_traj_from_mols[i] for i in range(len(sliced_traj))) else: raise AssertionError @@ -133,7 +133,7 @@ def test_conversion(self): self.traj_mols.to_displacements() self.traj_mols.to_positions() - assert all([mol == self.molecules[i] for i, mol in enumerate(self.traj_mols)]) + assert all(mol == self.molecules[i] for i, mol in enumerate(self.traj_mols)) def test_site_properties(self): lattice, species, coords = self._get_lattice_species_and_coords() diff --git a/pymatgen/io/lammps/inputs.py b/pymatgen/io/lammps/inputs.py index 6a1487d58a9..4236f473c44 100644 --- a/pymatgen/io/lammps/inputs.py +++ b/pymatgen/io/lammps/inputs.py @@ -168,7 +168,7 @@ def set_args(self, command: str, argument: str, stage_name: str | None = None, h how = list(range(N)) elif isinstance(how, int): how = [how] - elif not (isinstance(how, list) and all([isinstance(h, int) for h in how])): + elif not (isinstance(how, list) and all(isinstance(h, int) for h in how)): raise ValueError("""The argument 'how' should be a 'first', 'all', an integer or a list of integers.""") # Look for occurrences in the relevant stages @@ -321,7 +321,7 @@ def merge_stages(self, stage_names: list[str]): Args: stage_names (list): list of strings giving the names of the stages to be merged. """ - if not all([stage in self.stages_names for stage in stage_names]): + if not all(stage in self.stages_names for stage in stage_names): raise ValueError("At least one of the stages to be merged is not in the LammpsInputFile.") indices_stages_to_merge = [self.stages_names.index(stage) for stage in stage_names] @@ -567,7 +567,7 @@ def from_string(cls, contents: str, ignore_comments: bool = False, keep_stages: for block in blocks: keep_block = True # Block of comment(s) - if all([line[0] == "#" for line in block]): + if all(line[0] == "#" for line in block): if ignore_comments: keep_block = False else: @@ -670,8 +670,8 @@ def _check_stage_format(stage: dict): if not isinstance(stage["commands"], list): raise TypeError("The provided commands should be a list.") if len(stage["commands"]) >= 1 and ( - not all([len(cmdargs) == 2 for cmdargs in stage["commands"]]) - or not all([isinstance(cmd, str) and isinstance(arg, str) for (cmd, arg) in stage["commands"]]) + not all(len(cmdargs) == 2 for cmdargs in stage["commands"]) + or not all(isinstance(cmd, str) and isinstance(arg, str) for (cmd, arg) in stage["commands"]) ): raise ValueError("The provided commands should be a list of 2-strings tuples.") @@ -773,7 +773,7 @@ def _clean_lines(string_list: list, ignore_comments: bool = False) -> list: Returns: List of strings """ - if len(string_list) == 0 or all([s == "" for s in string_list]): + if len(string_list) == 0 or all(s == "" for s in string_list): raise ValueError("The list of strings should contain some non-empty strings.") # Remove the first comment line possibly added by previous input creations