Repository: Quantum-Accelerators/quacc
Pull request #1313 (2023-12-07 20:42:15)

`mv *` files from `SCRATCH_DIR` to `RESULTS_DIR` instead of `cp -r`, and make `SCRATCH_DIR` optional
## Required Checklist

- [X] I have read the [Contributing Guide](https://quantum-accelerators.github.io/quacc/dev/contributing.html). Don't lie! 😉
- [X] My PR is on a custom branch and is _not_ named `main`.
- [x] I have added relevant unit tests. Note: Your PR will likely not be merged without proper tests.

If you are an external contributor, you will see a comment from @buildbot-princeton. You can ignore this, as it is solely for admin purposes.

## Summary of Changes

The default `SCRATCH_DIR` was changed to `None`, such that everything is done in the same directory as `RESULTS_DIR` unless otherwise specified. This should make the "first calculation" easier on the new user, as everything will happen "before their eyes."

Additionally, an `mv *` style of operation is done instead of `cp -r` when transferring files from `SCRATCH_DIR` to `RESULTS_DIR`, assuming the former is specified. This should speed up the process substantially.

Andrew-S-Rosen: @tomdemeyere: As described in the above summary, I modified the behavior of the I/O to address a few of your concerns. Please don't hesitate to let me know if you recommend any modifications. 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c0db3c0..69904afa 100644
--- 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.4.3]
+
+### Changed
+
+- The default for the `SCRATCH_DIR` setting is now `None` instead of `~/.quacc_scratch`. In other words, the default is to not use a scratch directory. This should help with onboarding, and the ability to set it as `None`. Setting the `SCRATCH_DIR` to a given `Path` will still work as expected.
+- Files are no longer recursively _copied_ from `SCRATCH_DIR` to `RESULTS_DIR`. Instead, they are recursively moved. This should speed up I/O operations.
+
 ## [0.4.2]
 
 ### Added
diff --git a/src/quacc/runners/prep.py b/src/quacc/runners/prep.py
index 559c413b..c6091518 100644
--- a/src/quacc/runners/prep.py
+++ b/src/quacc/runners/prep.py
@@ -4,10 +4,10 @@ from __future__ import annotations
 import os
 from datetime import datetime, timezone
 from pathlib import Path
-from shutil import rmtree
+from shutil import move, rmtree
 from tempfile import mkdtemp
 
-from monty.shutil import copy_r, gzip_dir
+from monty.shutil import gzip_dir, remove
 
 from quacc import SETTINGS
 from quacc.utils.files import copy_decompress, make_unique_dir
@@ -28,11 +28,15 @@ def calc_setup(copy_files: list[str | Path] | None = None) -> tuple[Path, Path]:
     -------
     Path
         The path to the tmpdir, where the calculation will be run. It will be
-        deleted after the calculation is complete.
+        deleted after the calculation is complete. By default, this will be
+        located within the `SETTINGS.SCRATCH_DIR`, but if that is not set, it will
+        be located within the `SETTINGS.RESULTS_DIR`.
     Path
         The path to the results_dir, where the files will ultimately be stored.
         A symlink to the tmpdir will be made here during the calculation for
-        convenience.
+        convenience. By defualt, this will be the `SETTINGS.RESULTS_DIR`, but if
+        `SETTINGS.CREATE_UNIQUE_WORKDIR` is set, it will be a unique directory
+        within the `SETTINGS.RESULTS_DIR`.
     """
 
     # Set where to store the results
@@ -41,15 +45,14 @@ def calc_setup(copy_files: list[str | Path] | None = None) -> tuple[Path, Path]:
         if SETTINGS.CREATE_UNIQUE_WORKDIR
         else SETTINGS.RESULTS_DIR
     )
+    tmpdir_base = SETTINGS.SCRATCH_DIR or SETTINGS.RESULTS_DIR
 
-    # Create a tmpdir for the calculation within the scratch_dir
+    # Create a tmpdir for the calculation
     time_now = datetime.now(timezone.utc).strftime("%Y-%m-%d-%H-%M-%S-%f")
-    tmpdir = Path(
-        mkdtemp(prefix=f"quacc-tmp-{time_now}-", dir=SETTINGS.SCRATCH_DIR)
-    ).resolve()
+    tmpdir = Path(mkdtemp(prefix=f"quacc-tmp-{time_now}-", dir=tmpdir_base)).resolve()
 
     # Create a symlink to the tmpdir in the results_dir
-    if os.name != "nt" and SETTINGS.SCRATCH_DIR != SETTINGS.RESULTS_DIR:
+    if os.name != "nt" and SETTINGS.SCRATCH_DIR:
         symlink = job_results_dir / f"{tmpdir.name}-symlink"
         symlink.unlink(missing_ok=True)
         symlink.symlink_to(tmpdir, target_is_directory=True)
@@ -63,7 +66,7 @@ def calc_setup(copy_files: list[str | Path] | None = None) -> tuple[Path, Path]:
     return tmpdir, job_results_dir
 
 
-def calc_cleanup(tmpdir: str | Path, job_results_dir: str | Path) -> None:
+def calc_cleanup(tmpdir: Path, job_results_dir: Path) -> None:
     """
     Perform cleanup operations for a calculation, including gzipping files, copying
     files back to the original directory, and removing the tmpdir.
@@ -90,8 +93,11 @@ def calc_cleanup(tmpdir: str | Path, job_results_dir: str | Path) -> None:
     if SETTINGS.GZIP_FILES:
         gzip_dir(tmpdir)
 
-    # Copy files back to job_results_dir
-    copy_r(tmpdir, job_results_dir)
+    # Move files from tmpdir to job_results_dir
+    for file_name in os.listdir(tmpdir):
+        if Path(job_results_dir / file_name).exists():
+            remove(job_results_dir / file_name)
+        move(tmpdir / file_name, job_results_dir / file_name)
 
     # Remove symlink to tmpdir
     symlink_path = job_results_dir / f"{tmpdir.name}-symlink"
diff --git a/src/quacc/settings.py b/src/quacc/settings.py
index 0e9e3b1a..587c9185 100644
--- a/src/quacc/settings.py
+++ b/src/quacc/settings.py
@@ -70,15 +70,19 @@ class QuaccSettings(BaseSettings):
     RESULTS_DIR: Path = Field(
         Path.cwd(),
         description=(
-            "Directory to store I/O-based calculation results in."
+            "Directory to permanently store I/O-based calculation results in."
             "Note that this behavior may be modified by the chosen workflow engine."
             "For instance, Covalent specifies the base directory as the `workdir` "
             "of a local executor or the `remote_workdir` of a remote executor."
             "In this case, the `RESULTS_DIR` will be a subdirectory of that directory."
         ),
     )
-    SCRATCH_DIR: Path = Field(
-        Path("~/.scratch"), description="Scratch directory for calculations."
+    SCRATCH_DIR: Optional[Path] = Field(
+        None,
+        description="Directory to run the calculations in. If set to None, calculations "
+        "will be run in a temporary directory within `RESULTS_DIR`. If a `Path` is supplied, "
+        "calculations will be run in a temporary directory within `SCRATCH_DIR`. Results are "
+        "always moved back to `RESULTS_DIR` after the calculation is complete.",
     )
     CREATE_UNIQUE_WORKDIR: bool = Field(
         False,
@@ -295,6 +299,9 @@ class QuaccSettings(BaseSettings):
     @field_validator("RESULTS_DIR", "SCRATCH_DIR")
     @classmethod
     def resolve_and_make_paths(cls, v):
+        if v is None:
+            return v
+
         v = Path(os.path.expandvars(v)).expanduser().resolve()
         if not v.exists():
             os.makedirs(v)


Test patch:
diff --git a/tests/local/recipes/tblite_recipes/test_tblite_recipes.py b/tests/local/recipes/tblite_recipes/test_tblite_recipes.py
index 70411be2..097f368b 100644
--- a/tests/local/recipes/tblite_recipes/test_tblite_recipes.py
+++ b/tests/local/recipes/tblite_recipes/test_tblite_recipes.py
@@ -10,7 +10,7 @@ from quacc.recipes.tblite.core import freq_job, relax_job, static_job
 pytest.importorskip("tblite.ase")
 
 
-def test_static_job(tmp_path, monkeypatch):
+def test_static_job_v1(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
     atoms = molecule("H2O")
@@ -21,6 +21,11 @@ def test_static_job(tmp_path, monkeypatch):
     assert output["results"]["energy"] == pytest.approx(-137.96777594361672)
     assert np.array_equal(output["atoms"].get_positions(), atoms.get_positions())
 
+
+def test_static_job_v2(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+
+    atoms = molecule("H2O")
     output = static_job(atoms, method="GFN1-xTB")
     assert output["parameters"]["method"] == "GFN1-xTB"
     assert output["results"]["energy"] == pytest.approx(-156.96750578831137)
@@ -51,7 +56,7 @@ def test_relax_job_cell(tmp_path, monkeypatch):
     )
 
 
-def test_freq_job(tmp_path, monkeypatch):
+def test_freq_job_v1(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
     atoms = molecule("H2O")
@@ -79,6 +84,10 @@ def test_freq_job(tmp_path, monkeypatch):
     assert output["results"]["entropy"] == pytest.approx(0.0019584992229988523)
     assert output["results"]["gibbs_energy"] == pytest.approx(0.05367081893346437)
 
+
+def test_freq_job_v2(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+
     atoms = molecule("H")
     atoms.set_initial_magnetic_moments([0.0])
     initial_atoms = deepcopy(atoms)
@@ -100,6 +109,10 @@ def test_freq_job(tmp_path, monkeypatch):
     assert output["results"]["entropy"] == pytest.approx(0.0011292352752446438)
     assert output["results"]["gibbs_energy"] == pytest.approx(-1.2724500713131577)
 
+
+def test_freq_job_v3(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+
     atoms = molecule("CH3")
     initial_atoms = deepcopy(atoms)
     output = freq_job(atoms, energy=-10.0, temperature=1000, pressure=20)
@@ -133,9 +146,10 @@ def test_freq_job(tmp_path, monkeypatch):
 
 
 def test_unique_workdir(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
     DEFAULT_SETTINGS = SETTINGS.model_copy()
 
     SETTINGS.CREATE_UNIQUE_WORKDIR = True
-    test_static_job(tmp_path, monkeypatch)
+    test_static_job_v1(tmp_path, monkeypatch)
     test_relax_job(tmp_path, monkeypatch)
     SETTINGS.CREATE_UNIQUE_WORKDIR = DEFAULT_SETTINGS.CREATE_UNIQUE_WORKDIR
diff --git a/tests/local/runners/test_prep.py b/tests/local/runners/test_prep.py
new file mode 100644
index 00000000..8178f7dd
--- /dev/null
+++ b/tests/local/runners/test_prep.py
@@ -0,0 +1,85 @@
+import os
+from pathlib import Path
+
+from quacc.runners.prep import calc_cleanup, calc_setup
+
+
+def make_files():
+    with open("file1.txt", "w") as f:
+        f.write("file1")
+    with open("file2.txt", "w") as f:
+        f.write("file2")
+
+
+def make_files2():
+    p = Path("quacc-tmp-1234")
+    os.makedirs(p)
+
+
+def test_calc_setup(tmp_path, monkeypatch):
+    from quacc import SETTINGS
+
+    DEFAULT_SETTINGS = SETTINGS.model_copy()
+    monkeypatch.chdir(tmp_path)
+
+    make_files()
+    SETTINGS.SCRATCH_DIR = tmp_path
+
+    tmpdir, results_dir = calc_setup()
+
+    assert tmpdir.is_dir()
+    assert "-tmp" in str(tmpdir)
+    assert results_dir.is_dir()
+    assert str(SETTINGS.RESULTS_DIR) in str(results_dir)
+    if os.name != "nt":
+        assert Path(SETTINGS.RESULTS_DIR, f"{tmpdir.name}-symlink").is_symlink()
+    else:
+        assert not Path(SETTINGS.RESULTS_DIR, f"{tmpdir.name}-symlink").is_symlink()
+    assert "file1.txt" not in os.listdir(tmpdir)
+    assert "file2.txt" not in os.listdir(tmpdir)
+    assert Path.cwd() == tmpdir
+
+    SETTINGS.RESULTS_DIR = DEFAULT_SETTINGS.RESULTS_DIR
+    SETTINGS.SCRATCH_DIR = DEFAULT_SETTINGS.SCRATCH_DIR
+
+
+def test_calc_setup_v2(tmp_path, monkeypatch):
+    from quacc import SETTINGS
+
+    DEFAULT_SETTINGS = SETTINGS.model_copy()
+    monkeypatch.chdir(tmp_path)
+
+    make_files()
+    SETTINGS.SCRATCH_DIR = None
+
+    tmpdir, results_dir = calc_setup(copy_files=["file1.txt", "tmp_dir"])
+
+    assert tmpdir.is_dir()
+    assert "-tmp" in str(tmpdir)
+    assert results_dir.is_dir()
+    assert str(SETTINGS.RESULTS_DIR) in str(results_dir)
+    assert not Path(SETTINGS.RESULTS_DIR, f"{tmpdir.name}-symlink").is_symlink()
+    assert "file1.txt" in os.listdir(tmpdir)
+    assert "file2.txt" not in os.listdir(tmpdir)
+    assert Path.cwd() == tmpdir
+
+    SETTINGS.RESULTS_DIR = DEFAULT_SETTINGS.RESULTS_DIR
+    SETTINGS.SCRATCH_DIR = DEFAULT_SETTINGS.SCRATCH_DIR
+
+
+def test_calc_cleanup(tmp_path, monkeypatch):
+    from quacc import SETTINGS
+
+    DEFAULT_SETTINGS = SETTINGS.model_copy()
+    monkeypatch.chdir(tmp_path)
+
+    make_files2()
+    SETTINGS.SCRATCH_DIR = tmp_path
+
+    p = Path(Path.cwd(), "quacc-tmp-1234").resolve()
+    assert p.is_dir()
+    calc_cleanup(p, SETTINGS.RESULTS_DIR)
+    assert not p.exists()
+    assert Path.cwd() == SETTINGS.RESULTS_DIR
+
+    SETTINGS.SCRATCH_DIR = DEFAULT_SETTINGS.SCRATCH_DIR
diff --git a/tests/parsl/.quacc.yaml b/tests/parsl/.quacc.yaml
index 6e0e6eba..9d653d09 100644
--- a/tests/parsl/.quacc.yaml
+++ b/tests/parsl/.quacc.yaml
@@ -1,3 +1,4 @@
+CREATE_UNIQUE_WORKDIR: true
 RESULTS_DIR: ./.test_results
 SCRATCH_DIR: ./.test_scratch
 WORKFLOW_ENGINE: parsl