Remove auto_nbands handling + fix bravais inconsistency handling

pyproject.toml

@@ -112,6 +112,7 @@ ignore = [
     "PTH",
     "RUF012", # Disable checks for mutable class args
     "SIM105", # Use contextlib.suppress(OSError) instead of try-except-pass
+    "PLC0415", # Imports at the top of a file
 ]
 pydocstyle.convention = "google"
 isort.split-on-trailing-comma = false

src/custodian/utils.py

@@ -1,11 +1,16 @@
 """Utility function and classes."""
 
+from __future__ import annotations
+
 import functools
 import logging
 import os
 import tarfile
 from glob import glob
-from typing import ClassVar
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from typing import Any, ClassVar
 
 
 def backup(filenames, prefix="error", directory="./") -> None:
@@ -72,7 +77,7 @@ class tracked_lru_cache:
     Allows Custodian to clear the cache after all the checks have been performed.
     """
 
-    cached_functions: ClassVar = set()
+    cached_functions: ClassVar[set[Any]] = set()
 
     def __init__(self, func) -> None:
         """

src/custodian/vasp/handlers.py

@@ -197,6 +197,19 @@ def check(self, directory="./"):
                         # e-density (brmix error)
                         if err == "brmix" and "NELECT" in incar:
                             continue
+
+                        # Treat auto_nbands only as a warning, do not fail a job
+                        if err == "auto_nbands":
+                            if nbands := self._get_nbands_from_outcar(directory):
+                                outcar = load_outcar(os.path.join(directory, "OUTCAR"))
+                                if (nelect := outcar.nelect) and (nbands > 2 * nelect):
+                                    warnings.warn(
+                                        "NBANDS seems to be too high. The electronic structure may be inaccurate. "
+                                        "You may want to rerun this job with a smaller number of cores.",
+                                        UserWarning,
+                                    )
+                            continue
+
                         self.errors.add(err)
                         error_msgs.add(msg)
         for msg in error_msgs:
@@ -535,12 +548,11 @@ def correct(self, directory="./"):
 
             self.error_count["zbrent"] += 1
 
-        if "too_few_bands" in self.errors:
-            nbands = None
-            nbands = vi["INCAR"]["NBANDS"] if "NBANDS" in vi["INCAR"] else self._get_nbands_from_outcar(directory)
-            if nbands:
-                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 "too_few_bands" in self.errors and (
+            nbands := vi["INCAR"].get("NBANDS") or self._get_nbands_from_outcar(directory)
+        ):
+            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 self.errors & {"pssyevx", "pdsyevx"} and vi["INCAR"].get("ALGO", "Normal").lower() != "normal":
             actions.append({"dict": "INCAR", "action": {"_set": {"ALGO": "Normal"}}})
@@ -683,16 +695,11 @@ def correct(self, directory="./"):
             # direct and reciprocal meshes being incompatible.
             # This is basically the same as bravais
             vasp_recommended_symprec = 1e-6
-            symprec = vi["INCAR"].get("SYMPREC", vasp_recommended_symprec)
-            if symprec < vasp_recommended_symprec:
+            symprec = vi["INCAR"].get("SYMPREC", 1e-5)  # Default SYMPREC is 1e-5
+            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": float(f"{symprec * 10:.1e}")}}})
-            else:
+            elif vi["INCAR"].get("ISYM") != 0:  # Default ISYM is variable, but never 0
                 actions.append({"dict": "INCAR", "action": {"_set": {"ISYM": 0}}})
-            self.error_count["bravais"] += 1
 
         if "nbands_not_sufficient" in self.errors:
             outcar = load_outcar(os.path.join(directory, "OUTCAR"))
@@ -758,50 +765,25 @@ def correct(self, directory="./"):
                     )
             self.error_count["algo_tet"] += 1
 
-        if "auto_nbands" in self.errors and (nbands := self._get_nbands_from_outcar(directory)):
-            outcar = load_outcar(os.path.join(directory, "OUTCAR"))
-
-            if (nelect := outcar.nelect) and (nbands > 2 * nelect):
-                self.error_count["auto_nbands"] += 1
-                warnings.warn(
-                    "NBANDS seems to be too high. The electronic structure may be inaccurate. "
-                    "You may want to rerun this job with a smaller number of cores.",
-                    UserWarning,
-                )
-
-            elif nbands := vi["INCAR"].get("NBANDS"):
-                kpar = vi["INCAR"].get("KPAR", 1)
-                ncore = vi["INCAR"].get("NCORE", 1)
-                # If the user set an NBANDS that isn't compatible with parallelization settings,
-                # increase NBANDS to ensure correct task distribution and issue a UserWarning.
-                # The number of ranks per band is (number of MPI ranks) / (KPAR * NCORE)
-                if (ranks := outcar.run_stats.get("cores")) and (rem_bands := nbands % (ranks // (kpar * ncore))) != 0:
-                    actions.append({"dict": "INCAR", "action": {"_set": {"NBANDS": nbands + rem_bands}}})
-                    warnings.warn(
-                        f"Your NBANDS={nbands} setting was incompatible with your parallelization "
-                        f"settings, KPAR={kpar}, NCORE={ncore}, over {ranks} ranks. "
-                        f"The number of bands has been decreased accordingly to {nbands + rem_bands}.",
-                        UserWarning,
-                    )
-
         VaspModder(vi=vi, directory=directory).apply_actions(actions)
         return {"errors": list(self.errors), "actions": actions}
 
     @staticmethod
     def _get_nbands_from_outcar(directory: str) -> int | None:
         nbands = None
-        with open(os.path.join(directory, "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).
-                if "NBANDS=" in line:
-                    try:
-                        d = line.split("=")
-                        nbands = int(d[-1].strip())
-                        break
-                    except (IndexError, ValueError):
-                        pass
+        if os.path.isfile(outcar_path := os.path.join(directory, "OUTCAR")):
+            with open(outcar_path) 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).
+                    if "NBANDS=" in line:
+                        try:
+                            d = line.split("=")
+                            nbands = int(d[-1].strip())
+                            break
+                        except (IndexError, ValueError):
+                            pass
         return nbands
 
 

tests/qchem/test_jobs.py

@@ -778,7 +778,7 @@ def test_OptFF(self) -> None:
             QCInput.from_file(f"{TEST_DIR}/5690_frag18/mol.qin.freq_2").as_dict()
             == QCInput.from_file(os.path.join(SCR_DIR, "mol.qin")).as_dict()
         )
-        with pytest.raises(ValueError, match="ERROR: Can't deal with multiple neg frequencies yet! Exiting..."):
+        with pytest.raises(ValueError, match=r"ERROR: Can't deal with multiple neg frequencies yet! Exiting..."):
             next(job)
 
 

tests/vasp/test_jobs.py

@@ -305,7 +305,7 @@ def test_terminate_exception_during_graceful_termination(self):
         self.mock_process.terminate.side_effect = OSError("Permission denied")
 
         # Act & Assert
-        with pytest.raises(OSError):
+        with pytest.raises(OSError, match="Permission denied"):
             self.vasp_job.terminate()
 
         self.mock_process.terminate.assert_called_once()
@@ -323,7 +323,7 @@ def test_terminate_exception_during_force_kill(self):
         self.mock_process.kill.return_value = None
 
         # Act & Assert
-        with pytest.raises(OSError):
+        with pytest.raises(OSError, match="Process not found"):
             self.vasp_job.terminate()
 
         self.mock_process.terminate.assert_called_once()