Repository: materialsproject/custodian (python)

Issue:
[VASP] [Bug] Second posmap error does not change SYMPREC
When the VASP `"posmap"` error occurs twice, [SYMPREC should be changed to 1e-4](https://github.com/materialsproject/custodian/blob/master/custodian/vasp/handlers.py#L413). However, it stays 1e-6 because the error counter was never updated. A test was also not written for the second SYMPREC error handler. 

Edit: Will be closed when #183 is merged.

Pull request description:
[VASP] Increase posmap error count
Closes #186:

- Previously, `self.error_count["posmap"]` was never updated even though there were two sequential INCAR swaps to consider. This caused the second fix to never take place, and there was no test to check for it. There is now a test and the issue is fixed.
- I more clearly set `self.error_count["subspacematrix"] == 1` in the if/else block instead of relying on `else`.

Patch:
diff --git a/custodian/vasp/handlers.py b/custodian/vasp/handlers.py
index 547f8ccf..12017f78 100644
--- a/custodian/vasp/handlers.py
+++ b/custodian/vasp/handlers.py
@@ -30,7 +30,7 @@ from custodian.custodian import ErrorHandler
 from custodian.utils import backup
 from custodian.vasp.interpreter import VaspModder
 
-__author__ = "Shyue Ping Ong, William Davidson Richards, Anubhav Jain, " "Wei Chen, Stephen Dacek"
+__author__ = "Shyue Ping Ong, William Davidson Richards, Anubhav Jain, Wei Chen, Stephen Dacek, Andrew Rosen"
 __version__ = "0.1"
 __maintainer__ = "Shyue Ping Ong"
 __email__ = "ongsp@ucsd.edu"
@@ -295,7 +295,7 @@ class VaspErrorHandler(ErrorHandler):
         if self.errors.intersection(["subspacematrix"]):
             if self.error_count["subspacematrix"] == 0:
                 actions.append({"dict": "INCAR", "action": {"_set": {"LREAL": False}}})
-            else:
+            elif self.error_count["subspacematrix"] == 1:
                 actions.append({"dict": "INCAR", "action": {"_set": {"PREC": "Accurate"}}})
             self.error_count["subspacematrix"] += 1
 
@@ -400,7 +400,7 @@ class VaspErrorHandler(ErrorHandler):
                 actions.append({"dict": "INCAR", "action": {"_set": {"ISMEAR": 0, "SIGMA": 0.05}}})
 
         if "zheev" in self.errors:
-            if vi["INCAR"].get("ALGO", "Fast").lower() != "exact":
+            if vi["INCAR"].get("ALGO", "Normal").lower() != "exact":
                 actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Exact"}}})
         if "elf_kpar" in self.errors:
             actions.append({"dict": "INCAR", "action": {"_set": {"KPAR": 1}}})
@@ -421,9 +421,7 @@ class VaspErrorHandler(ErrorHandler):
                 # 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}}})
-            else:
-                # if we have already corrected twice, there's nothing else to do
-                pass
+            self.error_count["posmap"] += 1
 
         if "point_group" in self.errors:
             actions.append({"dict": "INCAR", "action": {"_set": {"ISYM": 0}}})
@@ -914,7 +912,7 @@ class IncorrectSmearingHandler(ErrorHandler):
     Check if a calculation is a metal (zero bandgap), has been run with
     ISMEAR=-5, and is not a static calculation, which is only appropriate for
     semiconductors. If this occurs, this handler will rerun the calculation
-    using the smearing settings appropriate for metals (ISMEAR=-2, SIGMA=0.2).
+    using the smearing settings appropriate for metals (ISMEAR=2, SIGMA=0.2).
     """
 
     is_monitor = False
@@ -1083,7 +1081,7 @@ class LargeSigmaHandler(ErrorHandler):
 class MaxForceErrorHandler(ErrorHandler):
     """
     Checks that the desired force convergence has been achieved. Otherwise
-    restarts the run with smaller EDIFF. (This is necessary since energy
+    restarts the run with smaller EDIFFG. (This is necessary since energy
     and force convergence criteria cannot be set simultaneously)
     """
 


Test patch:
diff --git a/custodian/vasp/tests/test_handlers.py b/custodian/vasp/tests/test_handlers.py
index ac175c67..bde7866a 100644
--- a/custodian/vasp/tests/test_handlers.py
+++ b/custodian/vasp/tests/test_handlers.py
@@ -275,6 +275,11 @@ class VaspErrorHandlerTest(unittest.TestCase):
         i = Incar.from_file("INCAR")
         self.assertAlmostEqual(i["SYMPREC"], 1e-6)
 
+        self.assertEqual(h.check(), True)
+        self.assertEqual(h.correct()["errors"], ["posmap"])
+        i = Incar.from_file("INCAR")
+        self.assertAlmostEqual(i["SYMPREC"], 1e-4)
+
     def test_posmap_vasp6(self):
         h = VaspErrorHandler("vasp6.posmap")
         self.assertEqual(h.check(), True)
@@ -282,6 +287,11 @@ class VaspErrorHandlerTest(unittest.TestCase):
         i = Incar.from_file("INCAR")
         self.assertAlmostEqual(i["SYMPREC"], 1e-6)
 
+        self.assertEqual(h.check(), True)
+        self.assertEqual(h.correct()["errors"], ["posmap"])
+        i = Incar.from_file("INCAR")
+        self.assertAlmostEqual(i["SYMPREC"], 1e-4)
+
     def test_point_group(self):
         h = VaspErrorHandler("vasp.point_group")
         self.assertEqual(h.check(), True)