Update vasprun.converged_ionic logic when EDIFFG=0, REDO of PR ma…

…terialsproject#3765 (materialsproject#3783)

* init commit

* add back type hints accidentally removed

* pre-commit auto-fixes

* Fix lobster test

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: esoteric-ephemera <[email protected]>

pymatgen/io/vasp/outputs.py

@@ -609,15 +609,24 @@ def converged_electronic(self) -> bool:
     def converged_ionic(self) -> bool:
         """
         Returns:
-            bool: True if ionic step convergence has been reached, i.e. that vasp
+            bool: True if ionic step convergence has been reached, i.e. VASP
                 exited before reaching the max ionic steps for a relaxation run.
-                In case IBRION=0 (MD) True if the max ionic steps are reached.
+                In case IBRION=0 (MD) or EDIFFG=0, returns True if the max ionic steps are reached.
         """
         nsw = self.parameters.get("NSW", 0)
         ibrion = self.parameters.get("IBRION", -1 if nsw in (-1, 0) else 0)
         if ibrion == 0:
             return nsw <= 1 or self.md_n_steps == nsw
 
+        # context re EDIFFG: the use case for EDIFFG=0 is to ensure a relaxation runs for
+        # NSW steps (the non-AIMD way to generate a relaxation trajectory with DFT). In
+        # that case, user isn't worried about convergence w.r.t. forces or energy. The
+        # next if statement prevents custodian from trying to correct the calc because
+        # Vasprun.converged_ionic = False.
+        ediffg = self.parameters.get("EDIFFG", 1)
+        if ibrion in {1, 2} and ediffg == 0:
+            return nsw <= 1 or nsw == len(self.ionic_steps)
+
         return nsw <= 1 or len(self.ionic_steps) < nsw
 
     @property

tests/io/vasp/test_outputs.py

@@ -76,6 +76,19 @@ def test_vasprun_md(self):
         assert vasp_run.md_n_steps == 10
         assert vasp_run.converged_ionic
 
+    def test_vasprun_ediffg_set_to_0(self):
+        # Test for case where EDIFFG is set to 0. This should pass if all ionic steps
+        # complete and are electronically converged.
+        print(list(os.walk(VASP_OUT_DIR)))
+        vasp_run = Vasprun(f"{VASP_OUT_DIR}/vasprun.ediffg_set_to_0.xml.gz")
+        assert len(vasp_run.ionic_steps) == 3
+        assert vasp_run.final_energy == approx(-34.60164204)
+        assert vasp_run.converged_ionic is True
+        assert vasp_run.converged_electronic is True
+        assert vasp_run.converged is True
+        assert vasp_run.parameters["EDIFFG"] == 0
+        assert vasp_run.parameters["EDIFF"] == 1e-5
+
     def test_bad_random_seed(self):
         vasp_run = Vasprun(f"{VASP_OUT_DIR}/vasprun.bad_random_seed.xml.gz")
         assert vasp_run.incar["ISMEAR"] == 0

tests/io/vasp/test_sets.py

@@ -1960,7 +1960,7 @@ def test_potcar(self):
         assert self.lobsterset2.potcar.symbols == ["Fe_pv", "P", "O"]
         # test if error raised contains correct potcar symbol for K element as PBE_54 set
         with pytest.raises(
-            OSError, match="You do not have the right POTCAR with functional='PBE_54' and symbol='K_sv'"
+            RuntimeError, match="You do not have the right POTCAR with functional='PBE_54' and symbol='K_sv'"
         ):
             _ = self.lobsterset9.potcar.symbols