Fixed execution of packmol in relative path. (#4145)

src/pymatgen/io/packmol.py

@@ -15,13 +15,13 @@
 
 from __future__ import annotations
 
-import os
 import subprocess
 from pathlib import Path
 from shutil import which
 from typing import TYPE_CHECKING
 
 import numpy as np
+from monty.os import cd
 
 from pymatgen.core import Molecule
 from pymatgen.io.core import InputGenerator, InputSet
@@ -50,42 +50,39 @@ def run(self, path: PathLike, timeout: float = 30) -> None:
             ValueError: if packmol does not succeed in packing the box.
             TimeoutExpiredError: if packmol does not finish within the timeout.
         """
-        wd = os.getcwd()
         if not which("packmol"):
             raise RuntimeError(
                 "Running a PackmolSet requires the executable 'packmol' to be in the path. Please "
                 "download packmol from https://github.com/leandromartinez98/packmol and follow the "
                 "instructions in the README to compile. Don't forget to add the packmol binary to your path"
             )
-        try:
-            os.chdir(path)
-            with open(self.inputfile, encoding="utf-8") as infile:
-                proc = subprocess.run(
-                    ["packmol"],
-                    stdin=infile,
-                    check=True,
-                    timeout=timeout,
-                    capture_output=True,
-                )
-            # This workaround is needed because packmol can fail to find
-            # a solution but still return a zero exit code.
-            # See https://github.com/m3g/packmol/issues/28
-            if "ERROR" in proc.stdout.decode():
-                if "Could not open file." in proc.stdout.decode():
-                    raise ValueError(
-                        "Your packmol might be too old to handle paths with spaces."
-                        "Please try again with a newer version or use paths without spaces."
+        with cd(path):
+            try:
+                with open(self.inputfile, encoding="utf-8") as infile:
+                    proc = subprocess.run(
+                        ["packmol"],
+                        stdin=infile,
+                        check=True,
+                        timeout=timeout,
+                        capture_output=True,
                     )
-                msg = proc.stdout.decode().split("ERROR")[-1]
-                raise ValueError(f"Packmol failed with return code 0 and stdout: {msg}")
-
-        except subprocess.CalledProcessError as exc:
-            raise ValueError(f"Packmol failed with error code {exc.returncode} and stderr: {exc.stderr}") from exc
-        else:
-            with open(Path(path, self.stdoutfile), mode="w", encoding="utf-8") as out:
-                out.write(proc.stdout.decode())
-        finally:
-            os.chdir(wd)
+                # This workaround is needed because packmol can fail to find
+                # a solution but still return a zero exit code.
+                # See https://github.com/m3g/packmol/issues/28
+                if "ERROR" in proc.stdout.decode():
+                    if "Could not open file." in proc.stdout.decode():
+                        raise ValueError(
+                            "Your packmol might be too old to handle paths with spaces."
+                            "Please try again with a newer version or use paths without spaces."
+                        )
+                    msg = proc.stdout.decode().split("ERROR")[-1]
+                    raise ValueError(f"Packmol failed with return code 0 and stdout: {msg}")
+
+            except subprocess.CalledProcessError as exc:
+                raise ValueError(f"Packmol failed with error code {exc.returncode} and stderr: {exc.stderr}") from exc
+            else:
+                with open(self.stdoutfile, mode="w", encoding="utf-8") as out:
+                    out.write(proc.stdout.decode())
 
     @classmethod
     def from_directory(cls, directory: PathLike) -> None:

tests/io/test_packmol.py

@@ -91,11 +91,22 @@ def test_packmol_with_path(self):
                 {"name": "LiTFSi", "number": 20, "coords": p2},
             ],
         )
-        pw.write_input(self.tmp_path)
-        pw.run(self.tmp_path)
+        # PymatgenTest makes each test change to a temporary directory
+        # Check here we can run in the current directory
+        pw.write_input(".")
+        pw.run(".")
         assert os.path.isfile(f"{self.tmp_path}/packmol_out.xyz")
+        assert os.path.isfile(f"{self.tmp_path}/packmol.stdout")
         out = Molecule.from_file(f"{self.tmp_path}/packmol_out.xyz")
         assert out.composition.num_atoms == 10 * 15 + 20 * 16
+        # PymatgenTest makes each test change to a temporary directory
+        # Check here we can run in a relative directory
+        pw.write_input("somedir")
+        pw.run("somedir")
+        assert os.path.isfile(f"{self.tmp_path}/somedir/packmol_out.xyz")
+        out = Molecule.from_file(f"{self.tmp_path}/somedir/packmol_out.xyz")
+        assert os.path.isfile(f"{self.tmp_path}/somedir/packmol.stdout")
+        assert out.composition.num_atoms == 10 * 15 + 20 * 16
 
     def test_control_params(self):
         """