move algo_tet changes from UnrecoverableError to `VaspErrorHandle…

…r` (#277)

* move algo_tet changes from UnrecoverableError to VaspErrorHandler and try setting algo=fast first

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* use vi correctly in VaspErrorHandler.check

* fix VaspErrorHandler increment algo_tet counter and always set ISMEAR = 0 if error_count["algo_tet"] > 0

* ensure UnconvergedErrorHandler does not create algo_tet error on next run

* pytest hide warnings

* add VaspErrorHandlerTest.test_algotet

* add test_files/INCAR.algo_tet_only
add test_files/vasp.algo_tet_only

* pylint

---------

custodian/vasp/handlers.py

@@ -617,6 +617,35 @@ def correct(self):
             if vi["INCAR"].get("ISYM", 2) > 0:
                 actions.append({"dict": "INCAR", "action": {"_set": {"ISYM": 0}}})
 
+        if "algo_tet" in self.errors:
+            # NOTE: This is the algo_tet handler response.
+            algo = vi["INCAR"].get("ALGO", "Normal").lower()
+            # ALGO=All/Damped / IALGO=5X often fails with ISMEAR < 0. There are two options VASP
+            # suggests: 1) Use ISMEAR = 0 (and a small sigma) to get the SCF to converge.
+            # 2) Use ALGO = Damped but only *after* an ISMEAR = 0 run where the wavefunction
+            # has been stored and read in for the subsequent run.
+            if (algo in ["all", "damped"] or (50 <= vi["INCAR"].get("IALGO", 38) <= 59)) and vi["INCAR"].get(
+                "ISMEAR", 1
+            ) < 0:
+                if self.error_count["algo_tet"] == 0:
+                    # first recovery attempt is to set ALGO to fast. Could fail again in which
+                    # case we end up here again if some other handler switches algo back to all/damped.
+                    # This time try the recovery below.
+                    actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Fast"}}})
+            #
+            # We will only hit the 2nd algo_teet error if the ALGO was changed back from Fast to All/Damped
+            # by e.g. NonConvergingErrorHandler
+            # NOTE this relies on self.errors being reset on empty set on every .check call
+            if self.error_count["algo_tet"] > 0:
+                actions.append({"dict": "INCAR", "action": {"_set": {"ISMEAR": 0, "SIGMA": 0.05}}})
+                if vi["INCAR"].get("NEDOS") or vi["INCAR"].get("EMIN") or vi["INCAR"].get("EMAX"):
+                    warnings.warn(
+                        "This looks like a DOS run. You may want to follow-up this job with ALGO = Damped"
+                        " and ISMEAR = -5, using the wavefunction from the current job.",
+                        UserWarning,
+                    )
+            self.error_count["algo_tet"] += 1
+
         VaspModder(vi=vi).apply_actions(actions)
         return {"errors": list(self.errors), "actions": actions}
 
@@ -1042,45 +1071,30 @@ def correct(self):
         algo = v.incar.get("ALGO", "Normal").lower()
         actions = []
         if not v.converged_electronic:
-            # NOTE: This is the algo_tet handler response.
-            if (
-                v.incar.get("ALGO", "Normal").lower() in ["all", "damped"] or (50 <= v.incar.get("IALGO", 38) <= 59)
-            ) and v.incar.get("ISMEAR", 1) < 0:
-                # ALGO=All/Damped / IALGO=5X often fails with ISMEAR < 0. There are two options VASP
-                # suggests: 1) Use ISMEAR = 0 (and a small sigma) to get the SCF to converge.
-                # 2) Use ALGO = Damped but only *after* an ISMEAR = 0 run where the wavefunction
-                # has been stored and read in for the subsequent run.
-                #
-                # For simplicity, we go with Option 1 here, but if the user wants high-quality
-                # DOS then they should consider running a subsequent job with ISMEAR = -5 and
-                # ALGO = Damped, provided the wavefunction has been stored.
-                actions.append({"dict": "INCAR", "action": {"_set": {"ISMEAR": 0, "SIGMA": 0.05}}})
-                if v.incar.get("NEDOS") or v.incar.get("EMIN") or v.incar.get("EMAX"):
-                    warnings.warn(
-                        "This looks like a DOS run. You may want to follow-up this job with ALGO = Damped"
-                        " and ISMEAR = -5, using the wavefunction from the current job.",
-                        UserWarning,
-                    )
             # NOTE: This is the amin error handler
             # Sometimes an AMIN warning can appear with large unit cell dimensions, so we'll address it now
             if np.max(v.final_structure.lattice.abc) > 50.0 and v.incar.get("AMIN", 0.1) > 0.01:
                 actions.append({"dict": "INCAR", "action": {"_set": {"AMIN": 0.01}}})
 
-            # If meta-GGA, go straight to Algo = All. Algo = All is recommended in the VASP
-            # manual and some meta-GGAs explicitly say to set Algo = All for proper convergence.
-            # I am using "--" as the check for METAGGA here because this is the default in the
-            # vasprun.xml file
-            if v.incar.get("METAGGA", "--") != "--" and algo != "all":
-                actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "All"}}})
+            if v.incar.get("ISMEAR", -1) >= 0 or not 50 <= v.incar.get("IALGO", 38) <= 59:
+                if v.incar.get("METAGGA", "--") != "--" and algo != "all":
+                    # If meta-GGA, go straight to Algo = All only if ISMEAR is greater or equal 0.
+                    # Algo = All is recommended in the VASP manual and some meta-GGAs explicitly
+                    # say to set Algo = All for proper convergence. I am using "--" as the check
+                    # for METAGGA here because this is the default in the vasprun.xml file
+                    actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "All"}}})
 
             # If a hybrid is used, do not set Algo = Fast or VeryFast. Hybrid calculations do not
             # support these algorithms, but no warning is printed.
             if v.incar.get("LHFCALC", False):
-                if algo != "all":
-                    actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "All"}}})
-                # See the VASP manual section on LHFCALC for more information.
-                elif algo != "damped":
-                    actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Damped", "TIME": 0.5}}})
+                if v.incar.get("ISMEAR", -1) >= 0 or not 50 <= v.incar.get("IALGO", 38) <= 59:
+                    if algo != "all":
+                        actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "All"}}})
+                    # See the VASP manual section on LHFCALC for more information.
+                    elif algo != "damped":
+                        actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Damped", "TIME": 0.5}}})
+                else:
+                    actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Normal"}}})
 
             # Ladder from VeryFast to Fast to Normal to All
             # (except for meta-GGAs and hybrids).

custodian/vasp/tests/test_handlers.py

@@ -61,6 +61,22 @@ def test_frozen_job(self):
         self.assertEqual(d["errors"], ["Frozen job"])
         self.assertEqual(Incar.from_file("INCAR")["ALGO"], "Normal")
 
+    def test_algotet(self):
+        shutil.copy("INCAR.algo_tet_only", "INCAR")
+        h = VaspErrorHandler("vasp.algo_tet_only")
+        h.check()
+        d = h.correct()
+        self.assertEqual(d["errors"], ["algo_tet"])
+        self.assertEqual(d["actions"], [{"action": {"_set": {"ALGO": "Fast"}}, "dict": "INCAR"}])
+        assert h.error_count["algo_tet"] == 1
+
+        # 2nd error should set ISMEAR to 0.
+        h.check()
+        d = h.correct()
+        self.assertEqual(d["errors"], ["algo_tet"])
+        assert h.error_count["algo_tet"] == 2
+        self.assertEqual(d["actions"], [{"action": {"_set": {"ISMEAR": 0, "SIGMA": 0.05}}, "dict": "INCAR"}])
+
     def test_subspace(self):
         h = VaspErrorHandler("vasp.subspace")
         h.check()
@@ -712,7 +728,6 @@ def test_check_correct_electronic(self):
         self.assertEqual(d["errors"], ["Unconverged"])
         self.assertEqual(
             [
-                {"dict": "INCAR", "action": {"_set": {"ISMEAR": 0, "SIGMA": 0.05}}},
                 {"dict": "INCAR", "action": {"_set": {"ALGO": "Damped", "TIME": 0.5}}},
             ],
             d["actions"],
@@ -750,14 +765,6 @@ def test_check_correct_scan(self):
         self.assertIn({"dict": "INCAR", "action": {"_set": {"ALGO": "All"}}}, d["actions"])
         os.remove("vasprun.xml")
 
-    def test_algotet(self):
-        shutil.copy("vasprun.xml.electronic_algotet", "vasprun.xml")
-        h = UnconvergedErrorHandler()
-        self.assertTrue(h.check())
-        d = h.correct()
-        self.assertEqual([{"dict": "INCAR", "action": {"_set": {"ISMEAR": 0, "SIGMA": 0.05}}}], d["actions"])
-        os.remove("vasprun.xml")
-
     def test_amin(self):
         shutil.copy("vasprun.xml.electronic_amin", "vasprun.xml")
         h = UnconvergedErrorHandler()

requirements-ci.txt

@@ -7,4 +7,4 @@ mypy
 pydocstyle
 flake8
 pylint
-black
+black

requirements.txt

@@ -1,4 +1,4 @@
 monty==2022.9.9
 ruamel.yaml==0.17.32
 sentry-sdk==1.14.0
-psutil==5.9.4
+psutil==5.9.4

setup.cfg

@@ -23,3 +23,6 @@ ignore_missing_imports = True
 
 [isort]
 profile = black
+
+[tool:pytest]
+addopts = "-p no:warnings"

test_files/INCAR.algo_tet_only

@@ -0,0 +1,24 @@
+ALGO = All
+EDIFF = 0.0004
+ENCUT = 520
+IBRION = 2
+ICHARG = 1
+ISIF = 3
+ISMEAR = -5
+ISPIN = 2
+LDAU = True
+LDAUJ = 0 0 0 0
+LDAUL = 0 2 2 0
+LDAUTYPE = 2
+LDAUU = 0 4.0 6 0
+LMAXMIX = 4
+LORBIT = 11
+LREAL = Auto
+LWAVE = False
+MAGMOM = 2*0.6 1*5 5*0.6
+NELM = 100
+NELMIN = 6
+NPAR = 1
+NSW = 99
+SIGMA = 0.05
+SYMPREC = 1e-5

test_files/vasp.algo_tet_only

@@ -0,0 +1,16 @@
+ -----------------------------------------------------------------------------
+|                                                                             |
+|           W    W    AA    RRRRR   N    N  II  N    N   GGGG   !!!           |
+|           W    W   A  A   R    R  NN   N  II  NN   N  G    G  !!!           |
+|           W    W  A    A  R    R  N N  N  II  N N  N  G       !!!           |
+|           W WW W  AAAAAA  RRRRR   N  N N  II  N  N N  G  GGG   !            |
+|           WW  WW  A    A  R   R   N   NN  II  N   NN  G    G                |
+|           W    W  A    A  R    R  N    N  II  N    N   GGGG   !!!           |
+|                                                                             |
+|      ALGO=A and IALGO=5X tend to fail with the tetrahedron method           |
+|      (e.g. Bloechls method ISMEAR=-5 is not variational)                    |
+|      please switch to IMSEAR=0-n, except for DOS calculations               |
+|      For DOS calculations use IALGO=53 after preconverging with ISMEAR>=0   |
+|          I HOPE YOU KNOW, WHAT YOU ARE  DOING                               |
+|                                                                             |
+ -----------------------------------------------------------------------------