Better DFTB+ error checking (#1101)

Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
Quantum-Accelerators · Oct 25, 2023 · 172020d · 172020d
1 parent befd56a
commit 172020d
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.3.10]
+
+### Changed
+
+- DFTB+ `MaxSccIterations` set to 200 by default
+- SCC errors in DFTB+ relaxations will be reported with higher priority than a relaxation error
+
 ## [0.3.9]
 
 ### Changed

diff --git a/src/quacc/recipes/dftb/core.py b/src/quacc/recipes/dftb/core.py
@@ -40,6 +40,7 @@ def static_job(
         ```python
         {
             "Hamiltonian_": "xTB" if "xtb" in method.lower() else "DFTB",
+            "Hamiltonian_MaxSccIterations": 200,
             "Hamiltonian_Method": method if "xtb" in method.lower() else None,
             "kpts": kpts or ((1, 1, 1) if atoms.pbc.any() else None),
         }
@@ -67,6 +68,7 @@ def static_job(
 
     defaults = {
         "Hamiltonian_": "xTB" if "xtb" in method.lower() else "DFTB",
+        "Hamiltonian_MaxSccIterations": 200,
         "Hamiltonian_Method": method if "xtb" in method.lower() else None,
         "kpts": kpts or ((1, 1, 1) if atoms.pbc.any() else None),
     }
@@ -79,10 +81,6 @@ def static_job(
         copy_files=copy_files,
     )
 
-    if SETTINGS.CHECK_CONVERGENCE and check_logfile(LOG_FILE, "SCC is NOT converged"):
-        msg = "SCC is not converged"
-        raise ValueError(msg)
-
     return summary
 
 
@@ -105,6 +103,7 @@ def relax_job(
         ```python
         {
             "Hamiltonian_": "xTB" if "xtb" in method.lower() else "DFTB",
+            "Hamiltonian_MaxSccIterations": 200,
             "Hamiltonian_Method": method if "xtb" in method.lower() else None,
             "kpts": kpts or ((1, 1, 1) if atoms.pbc.any() else None),
             "Driver_": "GeometryOptimization",
@@ -139,6 +138,7 @@ def relax_job(
 
     defaults = {
         "Hamiltonian_": "xTB" if "xtb" in method.lower() else "DFTB",
+        "Hamiltonian_MaxSccIterations": 200,
         "Hamiltonian_Method": method if "xtb" in method.lower() else None,
         "kpts": kpts or ((1, 1, 1) if atoms.pbc.any() else None),
         "Driver_": "GeometryOptimization",
@@ -149,21 +149,19 @@ def relax_job(
 
     summary = _base_job(
         atoms,
+        check_relax=True,
         defaults=defaults,
         calc_swaps=calc_swaps,
         additional_fields={"name": "DFTB+ Relax"},
         copy_files=copy_files,
     )
 
-    if SETTINGS.CHECK_CONVERGENCE and not check_logfile(LOG_FILE, "Geometry converged"):
-        msg = "Geometry did not converge"
-        raise ValueError(msg)
-
     return summary
 
 
 def _base_job(
     atoms: Atoms,
+    check_relax: bool = False,
     defaults: dict | None = None,
     calc_swaps: dict | None = None,
     additional_fields: dict | None = None,
@@ -176,6 +174,8 @@ def _base_job(
     ----------
     atoms
         Atoms object
+    check_relax
+        Check whether a relaxation's geometry optimization has converged.
     defaults
         The default calculator parameters to use.
     calc_swaps
@@ -197,6 +197,14 @@ def _base_job(
     atoms.calc = Dftb(**flags)
     final_atoms = run_calc(atoms, geom_file=GEOM_FILE, copy_files=copy_files)
 
+    if SETTINGS.CHECK_CONVERGENCE:
+        if check_logfile(LOG_FILE, "SCC is NOT converged"):
+            msg = "SCC is not converged"
+            raise ValueError(msg)
+        if check_relax and not check_logfile(LOG_FILE, "Geometry converged"):
+            msg = "Geometry did not converge"
+            raise ValueError(msg)
+
     return summarize_run(
         final_atoms,
         input_atoms=atoms,

diff --git a/tests/core/recipes/dftb_recipes/test_dftb_recipes.py b/tests/core/recipes/dftb_recipes/test_dftb_recipes.py
@@ -4,7 +4,6 @@
 import pytest
 from ase.build import bulk, molecule
 
-from quacc import SETTINGS
 from quacc.recipes.dftb.core import relax_job, static_job
 
 DFTBPLUS_EXISTS = bool(which("dftb+"))
@@ -15,24 +14,30 @@
 )
 
 
-def test_static_job(tmpdir):
+def test_static_job_water(tmpdir):
     tmpdir.chdir()
 
     atoms = molecule("H2O")
     output = static_job(atoms)
     assert output["natoms"] == len(atoms)
     assert output["parameters"]["Hamiltonian_"] == "xTB"
     assert output["parameters"]["Hamiltonian_Method"] == "GFN2-xTB"
+    assert output["parameters"]["Hamiltonian_MaxSccIterations"] == 200
     assert output["results"]["energy"] == pytest.approx(-137.9677759924738)
     assert (
         np.array_equal(output["atoms"].get_positions(), atoms.get_positions()) is True
     )
 
+
+def test_static_job_cu_supercell(tmpdir):
+    tmpdir.chdir()
+
     atoms = bulk("Cu") * (3, 3, 3)
     output = static_job(atoms)
     assert output["nsites"] == len(atoms)
     assert output["parameters"]["Hamiltonian_"] == "xTB"
     assert output["parameters"]["Hamiltonian_Method"] == "GFN2-xTB"
+    assert output["parameters"]["Hamiltonian_MaxSccIterations"] == 200
     assert (
         output["parameters"]["Hamiltonian_KPointsAndWeights_"].strip()
         == "SupercellFolding"
@@ -46,11 +51,16 @@ def test_static_job(tmpdir):
     )
     assert np.array_equal(output["atoms"].cell.array, atoms.cell.array) is True
 
+
+def test_static_job_cu_kpts(tmpdir):
+    tmpdir.chdir()
+
     atoms = bulk("Cu")
     output = static_job(atoms, kpts=(3, 3, 3))
     assert output["nsites"] == len(atoms)
     assert output["parameters"]["Hamiltonian_"] == "xTB"
     assert output["parameters"]["Hamiltonian_Method"] == "GFN2-xTB"
+    assert output["parameters"]["Hamiltonian_MaxSccIterations"] == 200
     assert (
         output["parameters"]["Hamiltonian_KPointsAndWeights_"].strip()
         == "SupercellFolding"
@@ -64,12 +74,16 @@ def test_static_job(tmpdir):
     )
     assert np.array_equal(output["atoms"].cell.array, atoms.cell.array) is True
 
+
+def test_static_errors(tmpdir):
+    tmpdir.chdir()
+
     with pytest.raises(ValueError):
         atoms = molecule("H2O")
-        output = static_job(atoms, calc_swaps={"Hamiltonian_MaxSccIterations": 1})
+        static_job(atoms, calc_swaps={"Hamiltonian_MaxSccIterations": 1})
 
 
-def test_relax_job(tmpdir):
+def test_relax_job_water(tmpdir):
     tmpdir.chdir()
 
     atoms = molecule("H2O")
@@ -78,19 +92,26 @@ def test_relax_job(tmpdir):
     assert output["natoms"] == len(atoms)
     assert output["parameters"]["Hamiltonian_"] == "xTB"
     assert output["parameters"]["Hamiltonian_Method"] == "GFN2-xTB"
+    assert output["parameters"]["Hamiltonian_MaxSccIterations"] == 200
     assert output["results"]["energy"] == pytest.approx(-137.97654214864497)
     assert (
         np.array_equal(output["atoms"].get_positions(), atoms.get_positions()) is False
     )
     assert np.array_equal(output["atoms"].cell.array, atoms.cell.array) is True
 
+
+def test_relax_job_cu_supercell(tmpdir):
+    tmpdir.chdir()
     atoms = bulk("Cu") * (2, 1, 1)
     atoms[0].position += 0.1
 
-    output = relax_job(atoms, kpts=(3, 3, 3))
+    output = relax_job(
+        atoms, kpts=(3, 3, 3), calc_swaps={"Hamiltonian_MaxSccIterations": 100}
+    )
     assert output["nsites"] == len(atoms)
     assert output["parameters"]["Hamiltonian_"] == "xTB"
     assert output["parameters"]["Hamiltonian_Method"] == "GFN2-xTB"
+    assert output["parameters"]["Hamiltonian_MaxSccIterations"] == 100
     assert (
         output["parameters"]["Hamiltonian_KPointsAndWeights_"].strip()
         == "SupercellFolding"
@@ -105,12 +126,16 @@ def test_relax_job(tmpdir):
     )
     assert np.array_equal(output["atoms"].cell.array, atoms.cell.array) is True
 
+
+def test_relax_job_cu_supercell_cell_relax(tmpdir):
+    tmpdir.chdir()
     atoms = bulk("Cu") * (2, 1, 1)
     atoms[0].position += 0.1
     output = relax_job(atoms, method="GFN1-xTB", kpts=(3, 3, 3), relax_cell=True)
     assert output["nsites"] == len(atoms)
     assert output["parameters"]["Hamiltonian_"] == "xTB"
     assert output["parameters"]["Hamiltonian_Method"] == "GFN1-xTB"
+    assert output["parameters"]["Hamiltonian_MaxSccIterations"] == 200
     assert (
         output["parameters"]["Hamiltonian_KPointsAndWeights_"].strip()
         == "SupercellFolding"
@@ -125,7 +150,9 @@ def test_relax_job(tmpdir):
     )
     assert np.array_equal(output["atoms"].cell.array, atoms.cell.array) is False
 
+def test_relax_job_cu_supercell_errors(tmpdir):
+    tmpdir.chdir()
     with pytest.raises(ValueError):
         atoms = bulk("Cu") * (2, 1, 1)
         atoms[0].position += 0.5
-        relax_job(atoms, kpts=(3, 3, 3), calc_swaps={"MaxSteps": 1})
+        relax_job(atoms, kpts=(3, 3, 3), calc_swaps={"MaxSteps": 1, "Hamiltonian_MaxSccIterations": 100})