Skip to content

Commit

Permalink
Allow for thread-safe execution (#1780)
Browse files Browse the repository at this point in the history
## Summary of Changes

This PR adds a new quacc setting, `CHDIR`, that will allow the user to
specify whether any `os.chdir` calls are made, thereby allowing for
multiple jobs to be run on a single Python process.

Closes #1641.

### Checklist

- [X] I have read the ["Guidelines"
section](https://quantum-accelerators.github.io/quacc/dev/contributing.html#guidelines)
of the contributing guide. Don't lie! 😉
- [X] My PR is on a custom branch and is _not_ named `main`.
- [X] I have used `black`, `isort`, and `ruff` as described in the
[style
guide](https://quantum-accelerators.github.io/quacc/dev/contributing.html#style).
- [X] I have added relevant, comprehensive [unit
tests](https://quantum-accelerators.github.io/quacc/dev/contributing.html#unit-tests).

### Notes

- Your PR will likely not be merged without proper and thorough tests.
- If you are an external contributor, you will see a comment from
[@buildbot-princeton](https://github.com/buildbot-princeton). This is
solely for the maintainers.
- When your code is ready for review, ping one of the [active
maintainers](https://quantum-accelerators.github.io/quacc/about/contributors.html#active-maintainers).

---------

Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
  • Loading branch information
Andrew-S-Rosen and deepsource-autofix[bot] authored Feb 28, 2024
1 parent 5426e78 commit 47b96f1
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 5 deletions.
6 changes: 4 additions & 2 deletions src/quacc/runners/prep.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ def calc_setup(
# for all threads in the current process. However, elsewhere in the code,
# we use absolute paths to avoid issues. We keep this here for now because some
# old ASE calculators do not support the `directory` keyword argument.
os.chdir(tmpdir)
if SETTINGS.CHDIR:
os.chdir(tmpdir)

return tmpdir, job_results_dir

Expand Down Expand Up @@ -126,7 +127,8 @@ def calc_cleanup(atoms: Atoms, tmpdir: Path | str, job_results_dir: Path | str)
# for all threads in the current process. However, elsewhere in the code,
# we use absolute paths to avoid issues. We keep this here for now because some
# old ASE calculators do not support the `directory` keyword argument.
os.chdir(job_results_dir)
if SETTINGS.CHDIR:
os.chdir(job_results_dir)

# Gzip files in tmpdir
if SETTINGS.GZIP_FILES:
Expand Down
14 changes: 14 additions & 0 deletions src/quacc/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,20 @@ class QuaccSettings(BaseSettings):
"""
),
)
CHDIR: bool = Field(
True,
description=(
"""
Whether quacc will make `os.chdir` calls to change the working directory
to be the location where the calculation is run. By default, we leave this
as `True` because not all ASE calculators properly support a `directory`
parameter. In most cases, this is fine, but it breaks thread safety.
If you need to run multiple, parallel calculations in a single Python process,
such as in a multithreaded job execution mode, then this setting needs
to be `False`. Note that not all calculators properly support this, however.
"""
),
)
GZIP_FILES: bool = Field(
True, description="Whether generated files should be gzip'd."
)
Expand Down
15 changes: 15 additions & 0 deletions tests/core/recipes/lj_recipes/test_lj_recipes.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,18 @@ def test_freq_job(tmp_path, monkeypatch):
assert len(output["results"]["vib_freqs"]) == 3 * len(atoms) - 6
assert len(output["parameters_thermo"]["vib_freqs"]) == 3 * len(atoms) - 6
assert output["parameters_thermo"]["n_imag"] == 0


def test_freq_job_threads(tmp_path, monkeypatch):
monkeypatch.chdir(tmp_path)

SETTINGS.CHDIR = False

atoms = molecule("H2O")

output = freq_job(relax_job(atoms)["atoms"])
assert output["natoms"] == len(atoms)
assert len(output["results"]["vib_freqs_raw"]) == 3 * len(atoms)
assert output["parameters_thermo"]["n_imag"] == 0

SETTINGS.CHDIR = DEFAULT_SETTINGS.CHDIR = True
25 changes: 22 additions & 3 deletions tests/parsl/test_emt_recipes.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,19 @@

from ase.build import bulk

from quacc import SETTINGS
from quacc.recipes.emt.core import relax_job # skipcq: PYL-C0412
from quacc.recipes.emt.slabs import bulk_to_slabs_flow # skipcq: PYL-C0412

DEFAULT_SETTINGS = SETTINGS.model_copy()

def test_parsl_functools(tmp_path, monkeypatch):

@pytest.mark.parametrize("chdir", [True, False])
def test_parsl_functools(tmp_path, monkeypatch, chdir):
monkeypatch.chdir(tmp_path)

SETTINGS.CHDIR = chdir

atoms = bulk("Cu")
result = bulk_to_slabs_flow(
atoms, job_params={"relax_job": {"opt_params": {"fmax": 0.1}}}, run_static=False
Expand All @@ -18,27 +25,39 @@ def test_parsl_functools(tmp_path, monkeypatch):
assert "atoms" in result[-1]
assert result[-1]["fmax"] == 0.1

SETTINGS.CHDIR = DEFAULT_SETTINGS.CHDIR


def test_phonon_flow(tmp_path, monkeypatch):
@pytest.mark.parametrize("chdir", [True, False])
def test_phonon_flow(tmp_path, monkeypatch, chdir):
pytest.importorskip("phonopy")
from quacc.recipes.emt.phonons import phonon_flow

SETTINGS.CHDIR = chdir

monkeypatch.chdir(tmp_path)
atoms = bulk("Cu")
output = phonon_flow(atoms)
assert output.result()["results"]["thermal_properties"]["temperatures"].shape == (
101,
)

SETTINGS.CHDIR = DEFAULT_SETTINGS.CHDIR


def test_phonon_flow_multistep(tmp_path, monkeypatch):
@pytest.mark.parametrize("chdir", [True, False])
def test_phonon_flow_multistep(tmp_path, monkeypatch, chdir):
pytest.importorskip("phonopy")
from quacc.recipes.emt.phonons import phonon_flow

SETTINGS.CHDIR = chdir

monkeypatch.chdir(tmp_path)
atoms = bulk("Cu")
relaxed = relax_job(atoms)
output = phonon_flow(relaxed["atoms"])
assert output.result()["results"]["thermal_properties"]["temperatures"].shape == (
101,
)

SETTINGS.CHDIR = DEFAULT_SETTINGS.CHDIR

0 comments on commit 47b96f1

Please sign in to comment.