Add new error handlers, add tests for NonConvergingErrorHandler (#313)

* Add pdsyevx and pricelv error handling; tests for these and NonConvergingErrorHandler * Remove chdir methods * delete _copy_all_files_from_source helper, use shutil.copytree instead * support compressed files in VaspErrorHandler and compress tests/files/vasp.(p|ps)syevx(''->.gz) * bump ruff * Remove unnecessary fake POTCAR * Replace all POTCARs with fake ones using pymatgen.dev_scripts.potcar_scrambler.py, gzip all POTCARs with tests that permit gzipped output * temporarily unzip a few files to ensure tests pass * fix comment and ruff ignore D (missing docs) in tests --------- Co-authored-by: Janosh Riebesell <[email protected]>
materialsproject · Feb 2, 2024 · 5148246 · 5148246
1 parent eda62bb
commit 5148246
Show file tree

Hide file tree

Showing 75 changed files with 22,111 additions and 147,189 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -7,7 +7,7 @@ ci:
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.1.14
+    rev: v0.2.0
     hooks:
       - id: ruff
         args: [--fix, --ignore, D]

diff --git a/custodian/vasp/handlers.py b/custodian/vasp/handlers.py
@@ -18,6 +18,7 @@
 from math import prod
 
 import numpy as np
+from monty.io import zopen
 from monty.os.path import zpath
 from monty.serialization import loadfn
 from pymatgen.core.structure import Structure
@@ -92,7 +93,10 @@ class VaspErrorHandler(ErrorHandler):
         "zpotrf": ["LAPACK: Routine ZPOTRF failed", "Routine ZPOTRF ZTRTRI"],
         "amin": ["One of the lattice vectors is very long (>50 A), but AMIN"],
         "zbrent": ["ZBRENT: fatal internal in", "ZBRENT: fatal error in bracketing"],
+        # Note that PSSYEVX and PDSYEVX errors are identical up to LAPACK routine:
+        # P<prec>SYEVX uses <prec> = S(ingle) or D(ouble) precision
         "pssyevx": ["ERROR in subspace rotation PSSYEVX"],
+        "pdsyevx": ["ERROR in subspace rotation PDSYEVX"],
         "eddrmm": ["WARNING in EDDRMM: call to ZHEGV failed"],
         "edddav": ["Error EDDDAV: Call to ZHEGV failed"],
         "algo_tet": ["ALGO=A and IALGO=5X tend to fail"],
@@ -105,6 +109,7 @@ class VaspErrorHandler(ErrorHandler):
         "rhosyg": ["RHOSYG"],
         "posmap": ["POSMAP"],
         "point_group": ["group operation missing"],
+        "pricelv": ["PRICELV: current lattice and primitive lattice are incommensurate"],
         "symprec_noise": ["determination of the symmetry of your systems shows a strong"],
         "dfpt_ncore": ["PEAD routines do not work for NCORE", "remove the tag NPAR from the INCAR file"],
         "bravais": ["Inconsistent Bravais lattice"],
@@ -160,7 +165,7 @@ def check(self):
         incar = Incar.from_file("INCAR")
         self.errors = set()
         error_msgs = set()
-        with open(self.output_filename) as file:
+        with zopen(self.output_filename, mode="rt") as file:
             text = file.read()
 
             # Check for errors
@@ -454,8 +459,8 @@ def correct(self):
             if "NBANDS" in vi["INCAR"]:
                 nbands = vi["INCAR"]["NBANDS"]
             else:
-                with open("OUTCAR") as f:
-                    for line in f:
+                with open("OUTCAR") as file:
+                    for line in file:
                         # Have to take the last NBANDS line since sometimes VASP
                         # updates it automatically even if the user specifies it.
                         # The last one is marked by NBANDS= (no space).
@@ -470,7 +475,7 @@ def correct(self):
                 new_nbands = max(int(1.1 * nbands), nbands + 1)  # This handles the case when nbands is too low (< 8).
                 actions.append({"dict": "INCAR", "action": {"_set": {"NBANDS": new_nbands}}})
 
-        if "pssyevx" in self.errors and vi["INCAR"].get("ALGO", "Normal").lower() != "normal":
+        if self.errors & {"pssyevx", "pdsyevx"} and vi["INCAR"].get("ALGO", "Normal").lower() != "normal":
             actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Normal"}}})
 
         if "eddrmm" in self.errors:
@@ -564,18 +569,24 @@ def correct(self):
             else:
                 actions.append({"dict": "INCAR", "action": {"_set": {"ISYM": 0}}})
 
-        if "posmap" in self.errors:
+        if symprec_errors := self.errors & {"posmap", "pricelv"}:
             # VASP advises to decrease or increase SYMPREC by an order of magnitude
             # the default SYMPREC value is 1e-5
-            if self.error_count["posmap"] == 0:
+            # For PRICELV, see https://www.vasp.at/forum/viewtopic.php?p=25608
+            if all(self.error_count[key] == 0 for key in symprec_errors):
                 # first, reduce by 10x
                 orig_symprec = vi["INCAR"].get("SYMPREC", 1e-5)
                 actions.append({"dict": "INCAR", "action": {"_set": {"SYMPREC": orig_symprec / 10}}})
-            elif self.error_count["posmap"] == 1:
+            elif all(self.error_count[key] <= 1 for key in symprec_errors):
                 # next, increase by 100x (10x the original)
                 orig_symprec = vi["INCAR"].get("SYMPREC", 1e-6)
                 actions.append({"dict": "INCAR", "action": {"_set": {"SYMPREC": orig_symprec * 100}}})
-            self.error_count["posmap"] += 1
+            elif any(self.error_count[key] > 1 for key in symprec_errors) and vi["INCAR"].get("ISYM", 2) > 0:
+                # Failing that, disable symmetry altogether
+                actions.append({"dict": "INCAR", "action": {"_set": {"ISYM": 0}}})
+
+            for key in symprec_errors:
+                self.error_count[key] += 1
 
         if "point_group" in self.errors and vi["INCAR"].get("ISYM", 2) > 0:
             actions.append({"dict": "INCAR", "action": {"_set": {"ISYM": 0}}})
@@ -838,11 +849,11 @@ def correct(self):
         vi = VaspInput.from_directory(".")
 
         if "aliasing" in self.errors:
-            with open("OUTCAR") as f:
+            with open("OUTCAR") as file:
                 grid_adjusted = False
                 changes_dict = {}
                 r = re.compile(r".+aliasing errors.*(NG.)\s*to\s*(\d+)")
-                for line in f:
+                for line in file:
                     m = r.match(line)
                     if m:
                         changes_dict[m.group(1)] = int(m.group(2))
@@ -1118,12 +1129,27 @@ def correct(self):
 
         if actions:
             vi = VaspInput.from_directory(".")
+
+            # Check for PSMAXN errors - see extensive discussion here
+            # https://github.com/materialsproject/custodian/issues/133
+            # Only correct PSMAXN when run couldn't converge for any reason
+            errors = ["Unconverged"]
+            if os.path.isfile("OUTCAR"):
+                with open("OUTCAR") as file:
+                    outcar_as_str = file.read()
+                if "PSMAXN for non-local potential too small" in outcar_as_str:
+                    if vi["INCAR"].get("LREAL", False) not in [False, "False", "false"]:
+                        actions += [
+                            {"dict": "INCAR", "action": {"_set": {"LREAL": False}}},
+                        ]
+                    errors += ["psmaxn"]
+
             backup(VASP_BACKUP_FILES)
             VaspModder(vi=vi).apply_actions(actions)
-            return {"errors": ["Unconverged"], "actions": actions}
+            return {"errors": errors, "actions": actions}
 
         # Unfixable error. Just return None for actions.
-        return {"errors": ["Unconverged"], "actions": None}
+        return {"errors": errors, "actions": None}
 
 
 class IncorrectSmearingHandler(ErrorHandler):

diff --git a/pyproject.toml b/pyproject.toml
@@ -7,13 +7,13 @@ name = "custodian"
 version = "2024.1.9"
 description = "A simple JIT job management framework in Python."
 authors = [
-  { name = "Shyue Ping Ong", email = "[email protected]" },
+  { name = "Janosh Riebesell", email = "[email protected]" },
   { name = "Matthew Horton" },
   { name = "Samuel M. Blau" },
+  { name = "Shyue Ping Ong", email = "[email protected]" },
   { name = "Stephen Dacek" },
   { name = "William Davidson Richards" },
   { name = "Xiaohui Qu" },
-  { name = "Janosh Riebesell", email = "[email protected]" },
 ]
 maintainers = [{ name = "Janosh Riebesell" }, { name = "Shyue Ping Ong" }]
 readme = "README.md"
@@ -63,7 +63,7 @@ exclude = ["*.tests", "*.tests.*"]
 [tool.ruff]
 target-version = "py39"
 line-length = 120
-select = [
+lint.select = [
   "B",    # flake8-bugbear
   "C4",   # flake8-comprehensions
   "D",    # pydocstyle
@@ -94,7 +94,7 @@ select = [
   "W",    # pycodestyle warning
   "YTT",  # flake8-2020
 ]
-ignore = [
+lint.ignore = [
   "B023",    # Function definition does not bind loop variable
   "B028",    # No explicit stacklevel keyword argument found
   "B904",    # Within an except clause, raise exceptions with ...
@@ -111,12 +111,12 @@ ignore = [
   "RUF012",  # Disable checks for mutable class args. This is a non-problem.
   "SIM105",  # Use contextlib.suppress(OSError) instead of try-except-pass
 ]
-pydocstyle.convention = "google"
-isort.split-on-trailing-comma = false
+lint.pydocstyle.convention = "google"
+lint.isort.split-on-trailing-comma = false
 
-[tool.ruff.per-file-ignores]
+[tool.ruff.lint.per-file-ignores]
 "__init__.py" = ["F401"]
-"*/tests/*" = ["D"]
+"tests/*" = ["D"]
 "tasks.py" = ["D", "E"]
 
 [tool.pytest.ini_options]

diff --git a/tests/ansible/test_interpreter.py b/tests/ansible/test_interpreter.py
@@ -1,5 +1,9 @@
 """Created on Jun 1, 2012."""
 
+import pytest
+
+from custodian.ansible.actions import FileActions
+from custodian.ansible.interpreter import Modder
 
 __author__ = "Shyue Ping Ong"
 __copyright__ = "Copyright 2012, The Materials Project"
@@ -9,12 +13,6 @@
 __date__ = "Jun 1, 2012"
 
 
-import pytest
-
-from custodian.ansible.actions import FileActions
-from custodian.ansible.interpreter import Modder
-
-
 class TestModder:
     def test_dict_modify(self):
         modder = Modder()

diff --git a/tests/feff/test_jobs.py b/tests/feff/test_jobs.py
@@ -17,7 +17,7 @@
 TEST_DIR = f"{TEST_FILES}/feff_unconverged"
 
 
-def test_to_from_dict():
+def test_as_from_dict():
     f = FeffJob("hello")
     f2 = FeffJob.from_dict(f.as_dict())
     assert type(f) == type(f2)