Repository: materialsproject/custodian
Pull request #284 (2023-08-09 21:05:39)

Tweak unconverged, bravais and den-/tet error handling
225a1e8d UnconvergedErrorHandler only set algo to normal if ISMEAR>=0
608530b2 mv custodian/feff/tests/test_handler{,s}.py
c11ab49e tweak bravais error handling in VaspErrorHandler
283fe9d2 improve den-/tet error handling in case of not using kspacing
bc022852 fix VaspErrorHandlerTest.test_bravais

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/materialsproject/custodian/pull/284?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject) Report
Patch coverage: **`100.00%`** and no project coverage change.
> Comparison is base [(`7695d4d`)](https://app.codecov.io/gh/materialsproject/custodian/commit/7695d4d0f5daba7b6e5ec12ff54dacb65e97b88e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject) 66.04% compared to head [(`608530b`)](https://app.codecov.io/gh/materialsproject/custodian/pull/284?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject) 66.04%.

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           master     #284   +/-   ##
=======================================
  Coverage   66.04%   66.04%           
=======================================
  Files          51       51           
  Lines        5549     5549           
=======================================
  Hits         3665     3665           
  Misses       1884     1884           
```


| [Files Changed](https://app.codecov.io/gh/materialsproject/custodian/pull/284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject) | Coverage Δ | |
|---|---|---|
| [custodian/feff/tests/test\_handlers.py](https://app.codecov.io/gh/materialsproject/custodian/pull/284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject#diff-Y3VzdG9kaWFuL2ZlZmYvdGVzdHMvdGVzdF9oYW5kbGVycy5weQ==) | `100.00% <ø> (ø)` | |
| [custodian/vasp/handlers.py](https://app.codecov.io/gh/materialsproject/custodian/pull/284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject#diff-Y3VzdG9kaWFuL3Zhc3AvaGFuZGxlcnMucHk=) | `78.89% <100.00%> (ø)` | |


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/materialsproject/custodian/pull/284?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject).


Patch:
diff --git a/README.md b/README.md
index e05336fd..891f426a 100644
--- a/README.md
+++ b/README.md
@@ -33,11 +33,11 @@ calculations.
 The version at the Python Package Index (PyPI) is always the latest stable release that will be hopefully, be
 relatively bug-free. Install as follows:
 
-```shell
+```sh
 pip install custodian
 ```
 
-Some plugins (e.g., vasp management) require additional setup (please see [pymatgen docs]).
+Some plugins (e.g., VASP management) require additional setup (please see [pymatgen docs]).
 
 ## Developmental version
 
@@ -45,13 +45,13 @@ The bleeding edge developmental version is at the custodian's [Github repo](http
 version is likely to be more buggy, but may contain new features. The Github version include test files as well for
 complete unit testing. After cloning the source, you can type
 
-```bash
+```sh
 python setup.py install
 ```
 
 or to install the package in developmental mode::
 
-```bash
+```sh
 python setup.py develop
 ```
 
diff --git a/custodian/feff/tests/test_handler.py b/custodian/feff/tests/test_handlers.py
similarity index 100%
rename from custodian/feff/tests/test_handler.py
rename to custodian/feff/tests/test_handlers.py
diff --git a/custodian/vasp/handlers.py b/custodian/vasp/handlers.py
index 41650b16..47a57138 100644
--- a/custodian/vasp/handlers.py
+++ b/custodian/vasp/handlers.py
@@ -7,14 +7,13 @@ by modifying the input files.
 import datetime
 import logging
 import multiprocessing
-import operator
 import os
 import re
 import shutil
 import time
 import warnings
 from collections import Counter
-from functools import reduce
+from math import prod
 
 import numpy as np
 from monty.os.path import zpath
@@ -178,16 +177,27 @@ class VaspErrorHandler(ErrorHandler):
         vi = VaspInput.from_directory(".")
 
         if self.errors.intersection(["tet", "dentet"]):
-            if vi["INCAR"].get("KSPACING"):
-                # decrease KSPACING by 20% in each direction (approximately double no. of kpoints)
-                actions.append(
-                    {
-                        "dict": "INCAR",
-                        "action": {"_set": {"KSPACING": vi["INCAR"].get("KSPACING") * 0.8}},
-                    }
-                )
+            # follow advice in this thread
+            # https://vasp.at/forum/viewtopic.php?f=3&t=416&p=4047&hilit=dentet#p4047
+            err_type = "tet" if "tet" in self.errors else "dentet"
+            if self.error_count[err_type] == 0:
+                if vi["INCAR"].get("KSPACING"):
+                    # decrease KSPACING by 20% in each direction (approximately double no. of kpoints)
+                    action = {"_set": {"KSPACING": vi["INCAR"].get("KSPACING") * 0.8}}
+                    actions.append({"dict": "INCAR", "action": action})
+                elif vi["KPOINTS"] and vi["KPOINTS"].num_kpts < 1:
+                    # increase KPOINTS by 20% in each direction (approximately double no. of kpoints)
+                    new_kpts = tuple(int(round(num * 1.2, 0)) for num in vi["KPOINTS"].kpts[0])
+                    actions.append({"dict": "KPOINTS", "action": {"_set": {"kpoints": (new_kpts,)}}})
+                elif vi["KPOINTS"] and vi["KPOINTS"].num_kpts >= 1:
+                    n_kpts = vi["KPOINTS"].num_kpts * 1.2
+                    new_kpts = tuple([int(round(n_kpts**1 / 3, 0))] * 3)
+                    actions.append(
+                        {"dict": "KPOINTS", "action": {"_set": {"generation_style": "Gamma", "kpoints": (new_kpts,)}}}
+                    )
             else:
                 actions.append({"dict": "INCAR", "action": {"_set": {"ISMEAR": 0, "SIGMA": 0.05}}})
+            self.error_count[err_type] += 1
 
         if "inv_rot_mat" in self.errors and vi["INCAR"].get("SYMPREC", 1e-5) > 1e-8:
             actions.append({"dict": "INCAR", "action": {"_set": {"SYMPREC": 1e-8}}})
@@ -342,12 +352,8 @@ class VaspErrorHandler(ErrorHandler):
 
         if "rot_matrix" in self.errors:
             if vi["KPOINTS"] and vi["KPOINTS"].style == Kpoints.supported_modes.Monkhorst:
-                actions.append(
-                    {
-                        "dict": "KPOINTS",
-                        "action": {"_set": {"generation_style": "Gamma"}},
-                    }
-                )
+                action = {"_set": {"generation_style": "Gamma"}}
+                actions.append({"dict": "KPOINTS", "action": action})
             elif vi["INCAR"].get("ISYM", 2) > 0:
                 actions.append({"dict": "INCAR", "action": {"_set": {"ISYM": 0}}})
 
@@ -478,6 +484,7 @@ class VaspErrorHandler(ErrorHandler):
             if vi["INCAR"].get("ICHARG", 0) < 10:
                 actions.append({"file": "CHGCAR", "action": {"_file_delete": {"mode": "actual"}}})
                 actions.append({"file": "WAVECAR", "action": {"_file_delete": {"mode": "actual"}}})
+            self.error_count["eddrmm"] += 1
 
         if "edddav" in self.errors:
             # Copy CONTCAR to POSCAR if CONTCAR has already been populated.
@@ -578,13 +585,19 @@ class VaspErrorHandler(ErrorHandler):
 
         if "bravais" in self.errors:
             # VASP recommends refining the lattice parameters or changing SYMPREC.
-            # Appears to occurs when SYMPREC is very low, so we will change it to
-            # the default if it's not already. If it's the default, we will x 10.
-            symprec = vi["INCAR"].get("SYMPREC", 1e-5)
-            if symprec < 1e-5:
-                actions.append({"dict": "INCAR", "action": {"_set": {"SYMPREC": 1e-5}}})
-            else:
+            # Appears to occur when SYMPREC is very low, so we change it to
+            # the default if it's not already. If it's the default, we x10.
+            vasp_recommended_symprec = 1e-6  # https://www.vasp.at/forum/viewtopic.php?f=3&t=19109
+            symprec = vi["INCAR"].get("SYMPREC", vasp_recommended_symprec)
+            if symprec < vasp_recommended_symprec:
+                actions.append({"dict": "INCAR", "action": {"_set": {"SYMPREC": vasp_recommended_symprec}}})
+            elif symprec < 1e-4:
+                # try 10xing symprec twice, then set ISYM=0 to not impose potentially artificial symmetry from
+                # too loose symprec on charge density
                 actions.append({"dict": "INCAR", "action": {"_set": {"SYMPREC": symprec * 10}}})
+            else:
+                actions.append({"dict": "INCAR", "action": {"_set": {"ISYM": 0}}})
+            self.error_count["bravais"] += 1
 
         if "nbands_not_sufficient" in self.errors:
             # There is something very wrong about the value of NBANDS. We don't make
@@ -748,7 +761,7 @@ class StdErrHandler(ErrorHandler):
         vi = VaspInput.from_directory(".")
 
         if "kpoints_trans" in self.errors and self.error_count["kpoints_trans"] == 0:
-            m = reduce(operator.mul, vi["KPOINTS"].kpts[0])
+            m = prod(vi["KPOINTS"].kpts[0])
             m = max(int(round(m ** (1 / 3))), 1)
             if vi["KPOINTS"] and vi["KPOINTS"].style.name.lower().startswith("m"):
                 m += m % 2
@@ -1002,7 +1015,7 @@ class MeshSymmetryErrorHandler(ErrorHandler):
         """Perform corrections."""
         backup(VASP_BACKUP_FILES | {self.output_filename})
         vi = VaspInput.from_directory(".")
-        m = reduce(operator.mul, vi["KPOINTS"].kpts[0])
+        m = prod(vi["KPOINTS"].kpts[0])
         m = max(int(round(m ** (1 / 3))), 1)
         if vi["KPOINTS"] and vi["KPOINTS"].style.name.lower().startswith("m"):
             m += m % 2
@@ -1080,7 +1093,7 @@ class UnconvergedErrorHandler(ErrorHandler):
                     actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Fast"}}})
                 elif algo == "fast":
                     actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Normal"}}})
-                elif algo == "normal":
+                elif algo == "normal" and (v.incar.get("ISMEAR", -1) >= 0 or not 50 <= v.incar.get("IALGO", 38) <= 59):
                     actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "All"}}})
                 else:
                     # Try mixing as last resort


Test patch:
diff --git a/custodian/vasp/tests/test_handlers.py b/custodian/vasp/tests/test_handlers.py
index 1b37be21..d7c2e6b8 100644
--- a/custodian/vasp/tests/test_handlers.py
+++ b/custodian/vasp/tests/test_handlers.py
@@ -93,6 +93,11 @@ class VaspErrorHandlerTest(unittest.TestCase):
 
     def test_check_correct(self):
         h = VaspErrorHandler("vasp.teterror")
+        h.check()
+        d = h.correct()
+        assert d["errors"] == ["tet"]
+        assert d["actions"] == [{"action": {"_set": {"kpoints": ((10, 2, 2),)}}, "dict": "KPOINTS"}]
+
         h.check()
         d = h.correct()
         assert d["errors"] == ["tet"]
@@ -139,6 +144,11 @@ class VaspErrorHandlerTest(unittest.TestCase):
 
     def test_dentet(self):
         h = VaspErrorHandler("vasp.dentet")
+        h.check()
+        d = h.correct()
+        assert d["errors"] == ["dentet"]
+        assert d["actions"] == [{"action": {"_set": {"kpoints": ((10, 2, 2),)}}, "dict": "KPOINTS"}]
+
         h.check()
         d = h.correct()
         assert d["errors"] == ["dentet"]
@@ -411,7 +421,7 @@ class VaspErrorHandlerTest(unittest.TestCase):
         assert h.check() is True
         assert h.correct()["errors"] == ["bravais"]
         i = Incar.from_file("INCAR")
-        assert i["SYMPREC"] == 1e-05
+        assert i["SYMPREC"] == 1e-6
 
     def test_posmap(self):
         h = VaspErrorHandler("vasp.posmap")
@@ -486,11 +496,19 @@ class VaspErrorHandlerTest(unittest.TestCase):
     def test_bzint_vasp6(self):
         # the BZINT error message is formatted differently in VASP6 compared to VASP5
         h = VaspErrorHandler("vasp6.bzint")
+        assert h.check() is True
+        d = h.correct()
+        assert d["errors"] == ["tet"]
+        incar = Incar.from_file("INCAR")
+        assert incar["ISMEAR"] == -5
+        assert incar["SIGMA"] == 0.05
+        assert d["actions"] == [{"action": {"_set": {"kpoints": ((10, 2, 2),)}}, "dict": "KPOINTS"}]
+
         assert h.check() is True
         assert h.correct()["errors"] == ["tet"]
-        i = Incar.from_file("INCAR")
-        assert i["ISMEAR"] == 0
-        assert i["SIGMA"] == 0.05
+        incar = Incar.from_file("INCAR")
+        assert incar["ISMEAR"] == 0
+        assert incar["SIGMA"] == 0.05
 
     def test_too_large_kspacing(self):
         shutil.copy("INCAR.kspacing", "INCAR")
@@ -703,6 +721,15 @@ class UnconvergedErrorHandlerTest(unittest.TestCase):
         assert type(h2) == UnconvergedErrorHandler
         assert h2.output_filename == "random_name.xml"
 
+    def test_correct_normal_with_condition(self):
+        shutil.copy("vasprun.xml.electronic_normal", "vasprun.xml")  # Reuse an existing file
+        h = UnconvergedErrorHandler()
+        assert h.check()
+        d = h.correct()
+        assert d["errors"] == ["Unconverged"]
+        assert d == {"actions": [{"action": {"_set": {"ALGO": "All"}}, "dict": "INCAR"}], "errors": ["Unconverged"]}
+        os.remove("vasprun.xml")
+
     @classmethod
     def tearDown(cls):
         shutil.move("INCAR.orig", "INCAR")