Clearer error handling (#1104)

Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -4,6 +4,12 @@ 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.11]
+
+## Changed
+
+- Improved error handling
+
 ## [0.3.10]
 
 ### Changed

src/quacc/calculators/vasp.py

@@ -251,7 +251,7 @@ def _manage_environment(self) -> str:
         if SETTINGS.VASP_VDW:
             os.environ["ASE_VASP_VDW"] = str(SETTINGS.VASP_VDW)
         if self.user_calc_params.get("luse_vdw") and "ASE_VASP_VDW" not in os.environ:
-            raise EnvironmentError(
+            raise OSError(
                 "VASP_VDW setting was not provided, yet you requested a vdW functional."
             )
 

src/quacc/recipes/dftb/core.py

@@ -195,13 +195,13 @@ def _base_job(
 
     if SETTINGS.CHECK_CONVERGENCE:
         if check_logfile(LOG_FILE, "SCC is NOT converged"):
-            msg = "SCC is not converged"
-            raise ValueError(msg)
+            msg = f"SCC is not converged in {LOG_FILE}"
+            raise RuntimeError(msg)
         if flags.get("Driver_") == "GeometryOptimization" and not check_logfile(
             LOG_FILE, "Geometry converged"
         ):
-            msg = "Geometry did not converge"
-            raise ValueError(msg)
+            msg = f"Geometry optimization did not complete in {LOG_FILE}"
+            raise RuntimeError(msg)
 
     return summarize_run(
         final_atoms,

src/quacc/recipes/gulp/core.py

@@ -248,7 +248,7 @@ def _base_job(
         and not final_atoms.calc.get_opt_state()
     ):
         msg = "Optimization did not converge."
-        raise ValueError(msg)
+        raise RuntimeError(msg)
 
     return summarize_run(
         final_atoms,

src/quacc/runners/calc.py

@@ -66,7 +66,11 @@ def run_calc(
     atoms, tmpdir, job_results_dir = _calc_setup(atoms, copy_files=copy_files)
 
     # Run calculation via get_potential_energy()
-    atoms.get_potential_energy()
+    try:
+        atoms.get_potential_energy()
+    except Exception:
+        msg = f"Calculation failed. Check the logfiles at {Path.cwd()}"
+        raise RuntimeError(msg)
 
     # Most ASE calculators do not update the atoms object in-place with a call
     # to .get_potential_energy(), which is important if an internal optimizer is
@@ -168,7 +172,11 @@ def run_ase_opt(
 
     # Run calculation
     with traj, optimizer(atoms, **optimizer_kwargs) as dyn:
-        dyn.run(fmax=fmax, steps=max_steps, **run_kwargs)
+        try:
+            dyn.run(fmax=fmax, steps=max_steps, **run_kwargs)
+        except Exception:
+            msg = f"Calculation failed. Check the logfiles at {Path.cwd()}"
+            raise RuntimeError(msg)
 
     # Store the trajectory atoms
     dyn.traj_atoms = read(traj_filename, index=":")
@@ -214,7 +222,13 @@ def run_ase_vib(
 
     # Run calculation
     vib = Vibrations(atoms, name=str(tmpdir / "vib"), **vib_kwargs)
-    vib.run()
+    try:
+        vib.run()
+    except Exception:
+        msg = f"Calculation failed. Check the logfiles at {Path.cwd()}"
+        raise RuntimeError(msg)
+
+    # Summarize run
     vib.summary(log=str(tmpdir / "vib_summary.log"))
 
     # Perform cleanup operations

src/quacc/schemas/ase.py

@@ -163,8 +163,8 @@ def summarize_opt_run(
     # Check convergence
     is_converged = dyn.converged()
     if check_convergence and not is_converged:
-        msg = "Optimization did not converge."
-        raise ValueError(msg)
+        msg = f"Optimization did not converge. Refer to {Path.cwd()}"
+        raise RuntimeError(msg)
 
     # Get trajectory
     if not trajectory:

src/quacc/schemas/cclib.py

@@ -111,8 +111,8 @@ def cclib_summarize_run(
     metadata = attributes["metadata"]
 
     if check_convergence and attributes.get("optdone") is False:
-        msg = "Optimization not complete."
-        raise ValueError(msg)
+        msg = f"Optimization not complete. Refer to {dir_path}"
+        raise RuntimeError(msg)
 
     # Now we construct the input Atoms object. Note that this is not necessarily
     # the same as the initial Atoms from the relaxation because the DFT
@@ -316,7 +316,7 @@ def _cclib_calculate(
         if not proatom_dir:
             if os.getenv("PROATOM_DIR") is None:
                 msg = "PROATOM_DIR environment variable or proatom_dir kwarg needs to be set."
-                raise ValueError(msg)
+                raise OSError(msg)
             proatom_dir = os.path.expandvars(os.environ["PROATOM_DIR"])
         if not Path(proatom_dir).exists():
             msg = f"Protatom directory {proatom_dir} does not exist. Returning None."

src/quacc/schemas/vasp.py

@@ -85,7 +85,9 @@ def vasp_summarize_run(
 
     # Check for calculation convergence
     if check_convergence and vasp_task_doc["state"] != "successful":
-        raise ValueError("VASP calculation did not converge. Will not store task data.")
+        raise RuntimeError(
+            f"VASP calculation did not converge. Will not store task data. Refer to {dir_path}"
+        )
 
     base_task_doc = summarize_run(atoms, prep_next_run=prep_next_run, store=False)
 
@@ -229,7 +231,7 @@ def _chargemol_runner(
     # Check environment variable
     if atomic_densities_path is None and "DDEC6_ATOMIC_DENSITIES_DIR" not in os.environ:
         msg = "DDEC6_ATOMIC_DENSITIES_DIR environment variable not defined."
-        raise EnvironmentError(msg)
+        raise OSError(msg)
 
     # Run Chargemol analysis
     chargemol_stats = ChargemolAnalysis(

src/quacc/utils/files.py

@@ -112,7 +112,7 @@ def load_yaml_calc(yaml_path: str | Path) -> dict:
 
     if not yaml_path.exists():
         msg = f"Cannot find {yaml_path}"
-        raise ValueError(msg)
+        raise FileNotFoundError(msg)
 
     # Load YAML file
     with yaml_path.open() as stream:

tests/core/calculators/vasp/test_vasp.py

@@ -409,7 +409,7 @@ def test_lasph():
 def test_vdw():
     atoms = bulk("Cu")
 
-    with pytest.raises(EnvironmentError):
+    with pytest.raises(OSError):
         Vasp(atoms, xc="beef-vdw")
 
 
@@ -736,5 +736,5 @@ def test_bad():
     with pytest.raises(ValueError):
         Vasp(atoms, auto_kpts={"test": [100]})
 
-    with pytest.raises(ValueError):
+    with pytest.raises(FileNotFoundError):
         Vasp(atoms, preset="BadRelaxSet")

tests/core/recipes/dftb_recipes/test_dftb_recipes.py

@@ -78,7 +78,7 @@ def test_static_job_cu_kpts(tmpdir):
 def test_static_errors(tmpdir):
     tmpdir.chdir()
 
-    with pytest.raises(ValueError):
+    with pytest.raises(RuntimeError):
         atoms = molecule("H2O")
         static_job(atoms, calc_swaps={"Hamiltonian_MaxSccIterations": 1})
 
@@ -153,11 +153,22 @@ def test_relax_job_cu_supercell_cell_relax(tmpdir):
 
 def test_relax_job_cu_supercell_errors(tmpdir):
     tmpdir.chdir()
-    with pytest.raises(ValueError):
+    with pytest.raises(RuntimeError):
         atoms = bulk("Cu") * (2, 1, 1)
         atoms[0].position += 0.5
         relax_job(
             atoms,
             kpts=(3, 3, 3),
             calc_swaps={"MaxSteps": 1, "Hamiltonian_MaxSccIterations": 100},
         )
+
+
+def test_child_errors(tmpdir):
+    tmpdir.chdir()
+    with pytest.raises(RuntimeError):
+        atoms = bulk("Cu")
+        static_job(atoms)
+
+    with pytest.raises(RuntimeError):
+        atoms = bulk("Cu")
+        relax_job(atoms)

tests/core/recipes/orca_recipes/test_orca_fails/test_failed_orca_recipes.py

@@ -8,5 +8,5 @@ def test_static_job(tmpdir):
     tmpdir.chdir()
 
     atoms = molecule("H2")
-    with pytest.raises(ValueError):
+    with pytest.raises(RuntimeError):
         relax_job(atoms, 0, 1)

tests/core/recipes/qchem_recipes/test_qchem_recipes.py

@@ -501,7 +501,7 @@ def test_irc_job_v1(monkeypatch, tmpdir, test_atoms):
 
 def test_irc_job_v2(tmpdir, test_atoms):
     tmpdir.chdir()
-    with pytest.raises(ValueError):
+    with pytest.raises(RuntimeError):
         irc_job(test_atoms, 0, 1, "straight")
 
     with pytest.raises(ValueError):

tests/core/schemas/test_ase.py

@@ -132,7 +132,7 @@ def test_summarize_opt_run(tmpdir):
     dyn.run(steps=5)
     traj = read("test.traj", index=":")
 
-    with pytest.raises(ValueError):
+    with pytest.raises(RuntimeError):
         summarize_opt_run(dyn)
 
     # Make sure info tags are handled appropriately

tests/core/schemas/test_cclib_schema.py

@@ -220,7 +220,7 @@ def test_cclib_calculate(tmpdir, cclib_obj):
             proatom_dir="does_not_exists",
         )
 
-    with pytest.raises(ValueError):
+    with pytest.raises(OSError):
         _cclib_calculate(
             cclib_obj,
             method="ddec6",

tests/core/schemas/test_vasp_pop_analysis.py

@@ -97,7 +97,7 @@ def test_chargemol_erorr(tmpdir):
     tmpdir.chdir()
     prep_files()
 
-    with pytest.raises(EnvironmentError):
+    with pytest.raises(OSError):
         _chargemol_runner()
 
     os.remove("CHGCAR")