Implemented requested changes and updated tests

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

Author: jonathan-schoeps

src/mindlessgen/qm/orca.py

@@ -32,9 +32,6 @@ def __init__(
             raise TypeError("orca_path should be a string or a Path object.")
         self.cfg = orcacfg
         self.xtb_cfg = xtb_config
-        self.xtb_driver_enabled = bool(xtb_config) and bool(
-            getattr(self.cfg, "use_xtb_driver", False)
-        )
         # must be explicitly initialized in current parallelization implementation
         # as accessing parent class variables might not be possible
         self.tmp_dir = self.__class__.get_temporary_directory()
@@ -61,7 +58,7 @@ def optimize(
             xyz_filename = "molecule.xyz"
             molecule.write_xyz_to_file(temp_path / xyz_filename)
 
-            if self._should_use_xtb_driver():
+            if self.cfg.use_xtb_driver:
                 optimized_molecule = self.optimize_xtb_driver(
                     temp_path=temp_path,
                     molecule=molecule,
@@ -70,39 +67,39 @@ def optimize(
                     max_cycles=max_cycles,
                     verbosity=verbosity,
                 )
-            else:
-                inputname = "orca_opt.inp"
-                orca_input = self._gen_input(
-                    molecule,
-                    xyz_filename,
-                    temp_path,
-                    ncores,
-                    True,
-                    max_cycles,
-                )
-                if verbosity > 1:
-                    print("ORCA input file:\n##################")
-                    print(orca_input)
-                    print("##################")
-                with open(temp_path / inputname, "w", encoding="utf8") as f:
-                    f.write(orca_input)
-                # run orca
-                arguments = [
-                    inputname,
-                ]
-                orca_log_out, orca_log_err, return_code = self._run(
-                    temp_path=temp_path, arguments=arguments
+                return optimized_molecule
+            inputname = "orca_opt.inp"
+            orca_input = self._gen_input(
+                molecule,
+                xyz_filename,
+                temp_path,
+                ncores,
+                True,
+                max_cycles,
+            )
+            if verbosity > 1:
+                print("ORCA input file:\n##################")
+                print(orca_input)
+                print("##################")
+            with open(temp_path / inputname, "w", encoding="utf8") as f:
+                f.write(orca_input)
+            # run orca
+            arguments = [
+                inputname,
+            ]
+            orca_log_out, orca_log_err, return_code = self._run(
+                temp_path=temp_path, arguments=arguments
+            )
+            if verbosity > 2:
+                print(orca_log_out)
+            if return_code != 0:
+                raise RuntimeError(
+                    f"ORCA failed with return code {return_code}:\n{orca_log_err}"
                 )
-                if verbosity > 2:
-                    print(orca_log_out)
-                if return_code != 0:
-                    raise RuntimeError(
-                        f"ORCA failed with return code {return_code}:\n{orca_log_err}"
-                    )
-                # read the optimized molecule from the output file
-                xyzfile = Path(temp_path / inputname).resolve().with_suffix(".xyz")
-                optimized_molecule = molecule.copy()
-                optimized_molecule.read_xyz_from_file(xyzfile)
+            # read the optimized molecule from the output file
+            xyzfile = Path(temp_path / inputname).resolve().with_suffix(".xyz")
+            optimized_molecule = molecule.copy()
+            optimized_molecule.read_xyz_from_file(xyzfile)
             return optimized_molecule
 
     def singlepoint(self, molecule: Molecule, ncores: int, verbosity: int = 1) -> str:
@@ -221,17 +218,6 @@ def _gen_input(
         orca_input += f"* xyzfile {molecule.charge} {molecule.uhf + 1} {xyzfile}\n"
         return orca_input
 
-    def _should_use_xtb_driver(self) -> bool:
-        """
-        Determine whether the xTB driver should be used for this optimization.
-        """
-        if not self.xtb_driver_enabled or not self.xtb_cfg:
-            return False
-        if hasattr(self.xtb_cfg, "has_constraints"):
-            return self.xtb_cfg.has_constraints()
-        constraints = getattr(self.xtb_cfg, "distance_constraints", None)
-        return bool(constraints)
-
     def optimize_xtb_driver(
         self,
         temp_path: Path,
@@ -260,6 +246,9 @@ def optimize_xtb_driver(
             print("ORCA input file:\n##################")
             print(orca_input)
             print("##################")
+            print("XTB input file:\n##################")
+            print(xtb_input)
+            print("##################")
         with open(temp_path / inputname, "w", encoding="utf8") as f:
             f.write(orca_input)
         # run orca with xTB as a driver
@@ -296,55 +285,24 @@ def _run_xtb_driver(
         """
         Run the optimization through the xTB external driver when constraints are requested.
         """
-        xtb_executable = self._get_xtb_executable()
+        xtb_executable = get_xtb_path()
+        if self.xtb_cfg is None:
+            raise RuntimeError(
+                "xTB driver requested but no xTB configuration provided."
+            )
+        xtb_runner = XTB(path=xtb_executable, xtb_config=self.xtb_cfg)
         arguments = [
-            str(xtb_executable),
             geometry_filename,
             "--opt",
         ]
         opt_level = getattr(self.cfg, "optlevel", None)
         if opt_level not in (None, ""):
             arguments.append(str(opt_level))
         arguments.extend(["--orca", "-I", xcontrol_name])
-        try:
-            xtb_out = sp.run(
-                arguments,
-                cwd=temp_path,
-                capture_output=True,
-                check=True,
-            )
-            xtb_log_out = xtb_out.stdout.decode("utf8", errors="replace")
-            xtb_log_err = xtb_out.stderr.decode("utf8", errors="replace")
-            return xtb_log_out, xtb_log_err, 0
-        except sp.CalledProcessError as e:
-            xtb_log_out = e.stdout.decode("utf8", errors="replace")
-            xtb_log_err = e.stderr.decode("utf8", errors="replace")
-            return xtb_log_out, xtb_log_err, e.returncode
-
-    def _get_xtb_executable(self) -> Path:
-        """
-        Determine the path to the xTB executable for external ORCA optimizations.
-        """
-        candidates: list[ORCAConfig | XTBConfig | None] = [self.xtb_cfg, self.cfg]
-        for source in candidates:
-            if source is None:
-                continue
-            for attr_name in ("xtb_path",):
-                candidate = getattr(source, attr_name, None)
-                if not candidate:
-                    continue
-                try:
-                    return get_xtb_path(candidate)
-                except ImportError as exc:
-                    raise RuntimeError(
-                        f"xTB executable defined via '{attr_name}' could not be found."
-                    ) from exc
-        try:
-            return get_xtb_path(None)
-        except ImportError as exc:
-            raise RuntimeError(
-                "xTB executable not found. Required for constrained ORCA optimizations."
-            ) from exc
+        xtb_log_out, xtb_log_err, returncode = xtb_runner._run(
+            temp_path=temp_path, arguments=arguments
+        )
+        return xtb_log_out, xtb_log_err, returncode
 
     def _write_xtb_input(
         self, molecule: Molecule, xtb_input: Path, input_file: str
@@ -356,7 +314,7 @@ def _write_xtb_input(
             raise RuntimeError(
                 "xTB configuration missing but constraints were requested."
             )
-        xtb_path = self._get_xtb_executable()
+        xtb_path = get_xtb_path()
         xtb_writer = XTB(xtb_path, self.xtb_cfg)
         generated = xtb_writer._prepare_distance_constraint_file(
             molecule, xtb_input.parent

test/test_qm/test_orca.py

@@ -44,8 +44,11 @@ def _factory(cfg=None, xtb_cfg_param=None):
 
 
 def test_run_xtb_driver_success(monkeypatch, tmp_path, make_orca):
-    orca = make_orca(cfg=DummyORCAConfig(optlevel="tight"))
-    monkeypatch.setattr(orca, "_get_xtb_executable", lambda: Path("/fake/xtb"))
+    orca = make_orca(
+        cfg=DummyORCAConfig(optlevel="tight"),
+        xtb_cfg_param=DummyXTBConfig(),
+    )
+    monkeypatch.setattr("mindlessgen.qm.orca.get_xtb_path", lambda: Path("/fake/xtb"))
     captured = {}
 
     def fake_run(args, cwd, capture_output, check):
@@ -72,8 +75,8 @@ def fake_run(args, cwd, capture_output, check):
 
 def test_run_xtb_driver_failure_returns_error(monkeypatch, tmp_path, make_orca):
     """Ensure the ORCA wrapper surfaces errors from the xTB driver."""
-    orca = make_orca()
-    monkeypatch.setattr(orca, "_get_xtb_executable", lambda: Path("/fake/xtb"))
+    orca = make_orca(xtb_cfg_param=DummyXTBConfig())
+    monkeypatch.setattr("mindlessgen.qm.orca.get_xtb_path", lambda: Path("/fake/xtb"))
 
     def fake_run(*_, **kwargs):
         del kwargs
@@ -86,55 +89,19 @@ def fake_run(*_, **kwargs):
     assert (out, err, code) == ("bad", "worse", 1)
 
 
-def test_get_xtb_executable_raises_when_missing(monkeypatch, make_orca):
+def test_run_xtb_driver_requires_xtb_cfg(monkeypatch, tmp_path, make_orca):
     orca = make_orca()
-
-    def fake_get_xtb_path(candidate):
-        raise ImportError("not found")
-
-    monkeypatch.setattr("mindlessgen.qm.orca.get_xtb_path", fake_get_xtb_path)
-    with pytest.raises(RuntimeError, match="xTB executable not found"):
-        orca._get_xtb_executable()
-
-
-def test_get_xtb_executable_prefers_xtb_cfg_path(monkeypatch, make_orca):
-    xtb_cfg_constraints = DummyXTBConfig()
-    xtb_cfg_constraints.xtb_path = "xtb_from_xtb_cfg"
-    orca = make_orca(xtb_cfg_param=xtb_cfg_constraints)
-    called = {}
-
-    def fake_get_xtb_path(candidate):
-        called.setdefault("candidates", []).append(candidate)
-        return Path("/resolved/xtb_cfg")
-
-    monkeypatch.setattr("mindlessgen.qm.orca.get_xtb_path", fake_get_xtb_path)
-    assert orca._get_xtb_executable() == Path("/resolved/xtb_cfg")
-    assert called["candidates"][0] == "xtb_from_xtb_cfg"
-
-
-def test_should_use_xtb_driver_checks_distance_constraints(make_orca):
-    cfg = DummyORCAConfig(use_xtb_driver=True)
-    xtb_constraints = DummyXTBConfig(distance_constraints=[object()])
-    orca = make_orca(cfg=cfg, xtb_cfg_param=xtb_constraints)
-    assert orca._should_use_xtb_driver() is True
-
-    no_constraints = DummyXTBConfig(distance_constraints=[])
-    orca_no_constraints = make_orca(cfg=cfg, xtb_cfg_param=no_constraints)
-    assert orca_no_constraints._should_use_xtb_driver() is False
-
-    cfg_disabled = DummyORCAConfig(use_xtb_driver=False)
-    orca_disabled = make_orca(cfg=cfg_disabled, xtb_cfg_param=xtb_constraints)
-    assert orca_disabled._should_use_xtb_driver() is False
-    orca_missing_xtb = make_orca(cfg=cfg, xtb_cfg_param=None)
-    assert orca_missing_xtb._should_use_xtb_driver() is False
+    monkeypatch.setattr("mindlessgen.qm.orca.get_xtb_path", lambda: Path("/fake/xtb"))
+    with pytest.raises(RuntimeError, match="xTB driver requested"):
+        orca._run_xtb_driver(tmp_path, "geom.xyz", "ctrl.inp", ncores=1)
 
 
 def test_write_xtb_input_creates_expected_file(monkeypatch, tmp_path, make_orca):
     xtb_cfg_instance = DummyXTBConfig(
         distance_constraints=["dummy"], distance_constraint_force_constant=0.7
     )
     orca = make_orca(xtb_cfg_param=xtb_cfg_instance)
-    monkeypatch.setattr(orca, "_get_xtb_executable", lambda: Path("/fake/xtb"))
+    monkeypatch.setattr("mindlessgen.qm.orca.get_xtb_path", lambda: Path("/fake/xtb"))
 
     def fake_prepare(self, molecule, temp_dir):
         assert temp_dir == tmp_path