Repository: materialsproject/custodian
Pull request #228 (2022-08-09 14:10:27)

Added CONTCAR to POSCAR copy for some electronic convergence errors
I noticed that for some troublesome relaxations, I would have electronic issues along the way,
mainly `edddav` and `eddrmm` errors. They did not occur immediately, but after potentially many ionic steps.

## Summary

* With this change to the `VaspErrorHandler` class I propose to copy CONTCAR to POSCAR (as it is done already for problems with ionic algorithms) also for electronic algorithm issues.
* However, it is important to check whether there is a valid CONTCAR to copy, which is not the case if the error occurs in the first iteration. Thus I check if the CONTCAR can be used to make a `pymatgen.io.vasp.inputs.Poscar` object, and only copy if that is the case.
* eddrmm, edddav, and zheev errors receive all the same treatment.

## TODO

* I have not yet written any unit tests for this, since I am not sure if this change is wanted. If the developers think this makes sense, I am happy to write some tests.


Patch:
diff --git a/custodian/vasp/handlers.py b/custodian/vasp/handlers.py
index 35155b99..5b6b7e0d 100644
--- a/custodian/vasp/handlers.py
+++ b/custodian/vasp/handlers.py
@@ -454,6 +454,13 @@ class VaspErrorHandler(ErrorHandler):
             actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Normal"}}})
         if "eddrmm" in self.errors:
             # RMM algorithm is not stable for this calculation
+            # Copy CONTCAR to POSCAR if CONTCAR has already been populated.
+            try:
+                is_contcar = Poscar.from_file("CONTCAR")
+            except Exception:
+                is_contcar = False
+            if is_contcar:
+                actions.append({"file": "CONTCAR", "action": {"_file_copy": {"dest": "POSCAR"}}})
             if vi["INCAR"].get("ALGO", "Normal").lower() in ["fast", "veryfast"]:
                 actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Normal"}}})
             else:
@@ -464,6 +471,13 @@ class VaspErrorHandler(ErrorHandler):
                 actions.append({"file": "WAVECAR", "action": {"_file_delete": {"mode": "actual"}}})
 
         if "edddav" in self.errors:
+            # Copy CONTCAR to POSCAR if CONTCAR has already been populated.
+            try:
+                is_contcar = Poscar.from_file("CONTCAR")
+            except Exception:
+                is_contcar = False
+            if is_contcar:
+                actions.append({"file": "CONTCAR", "action": {"_file_copy": {"dest": "POSCAR"}}})
             if vi["INCAR"].get("ICHARG", 0) < 10:
                 actions.append({"file": "CHGCAR", "action": {"_file_delete": {"mode": "actual"}}})
 
@@ -522,6 +536,13 @@ class VaspErrorHandler(ErrorHandler):
                 )
 
         if "zheev" in self.errors:
+            # Copy CONTCAR to POSCAR if CONTCAR has already been populated.
+            try:
+                is_contcar = Poscar.from_file("CONTCAR")
+            except Exception:
+                is_contcar = False
+            if is_contcar:
+                actions.append({"file": "CONTCAR", "action": {"_file_copy": {"dest": "POSCAR"}}})
             if vi["INCAR"].get("ALGO", "Normal").lower() != "exact":
                 actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Exact"}}})
         if "elf_kpar" in self.errors:


Test patch:
diff --git a/custodian/vasp/tests/test_handlers.py b/custodian/vasp/tests/test_handlers.py
index fa70847c..3e3ce360 100644
--- a/custodian/vasp/tests/test_handlers.py
+++ b/custodian/vasp/tests/test_handlers.py
@@ -283,6 +283,7 @@ class VaspErrorHandlerTest(unittest.TestCase):
         self.assertEqual(i["ALGO"], "Normal")
 
     def test_eddrmm(self):
+        shutil.copy("CONTCAR.eddav_eddrmm", "CONTCAR")
         h = VaspErrorHandler("vasp.eddrmm")
         self.assertEqual(h.check(), True)
         self.assertEqual(h.correct()["errors"], ["eddrmm"])
@@ -291,6 +292,9 @@ class VaspErrorHandlerTest(unittest.TestCase):
         self.assertEqual(h.correct()["errors"], ["eddrmm"])
         i = Incar.from_file("INCAR")
         self.assertEqual(i["POTIM"], 0.25)
+        p = Structure.from_file("POSCAR")
+        c = Structure.from_file("CONTCAR")
+        self.assertEqual(p, c)
 
     def test_nicht_konv(self):
         h = VaspErrorHandler("vasp.nicht_konvergent")
@@ -300,16 +304,23 @@ class VaspErrorHandlerTest(unittest.TestCase):
         self.assertEqual(i["LREAL"], False)
 
     def test_edddav(self):
+        shutil.copy("CONTCAR.eddav_eddrmm", "CONTCAR")
         h = VaspErrorHandler("vasp.edddav2")
         self.assertEqual(h.check(), True)
         self.assertEqual(h.correct()["errors"], ["edddav"])
         i = Incar.from_file("INCAR")
         self.assertEqual(i["NCORE"], 2)
+        p = Structure.from_file("POSCAR")
+        c = Structure.from_file("CONTCAR")
+        self.assertEqual(p, c)
 
         h = VaspErrorHandler("vasp.edddav")
         self.assertEqual(h.check(), True)
         self.assertEqual(h.correct()["errors"], ["edddav"])
         self.assertFalse(os.path.exists("CHGCAR"))
+        p = Structure.from_file("POSCAR")
+        c = Structure.from_file("CONTCAR")
+        self.assertEqual(p, c)
 
     def test_zpotrf(self):
         h = VaspErrorHandler("vasp.ztrtri")
diff --git a/test_files/CONTCAR.eddav_eddrmm b/test_files/CONTCAR.eddav_eddrmm
new file mode 100644
index 00000000..779d2eb3
--- /dev/null
+++ b/test_files/CONTCAR.eddav_eddrmm
@@ -0,0 +1,16 @@
+Na2 Fe1 Ni1 O4
+1.0
+-2.816100 0.000000 0.000000
+-0.000000 -4.877628 0.000000
+-1.408050 0.812938 4.684533
+Na Fe Ni O
+2 1 1 4
+direct
+0.000000 0.000000 0.010000 Na
+0.500000 0.500000 0.010000 Na
+0.250000 0.250000 0.510000 Fe
+0.750000 0.750000 0.510000 Ni
+0.859265 0.380245 0.271470 O
+0.359265 0.880245 0.271470 O
+0.640735 0.119755 0.708530 O
+0.140735 0.619755 0.708530 O