Skip to content

Commit

Permalink
Don't call a subprocess with VASP custodian (#1850)
Browse files Browse the repository at this point in the history
## Summary of Changes

Call Custodian directly rather than via a subprocess for VASP.

FYI: @zulissimeta, I had to revert this PR due to issues with concurrent
calculations in certain workflow engines. Once we solve the upstream
issue in Custodian, I can re-merge this PR.

Requires:
- #1821
- Custodian > 2024.2.15

### 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 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).
  • Loading branch information
Andrew-S-Rosen authored Mar 12, 2024
1 parent ce61d8a commit 62bed57
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 7 deletions.
45 changes: 38 additions & 7 deletions src/quacc/calculators/vasp/vasp.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

from __future__ import annotations

import inspect
import os
import sys
import subprocess
from pathlib import Path
from typing import TYPE_CHECKING

Expand All @@ -13,7 +12,6 @@
from ase.calculators.vasp import setups as ase_setups
from ase.constraints import FixAtoms

from quacc.calculators.vasp import vasp_custodian
from quacc.calculators.vasp.io import load_vasp_yaml_calc
from quacc.calculators.vasp.params import (
get_param_swaps,
Expand All @@ -23,6 +21,7 @@
set_auto_dipole,
set_pmg_kpts,
)
from quacc.calculators.vasp.vasp_custodian import run_custodian
from quacc.schemas.prep import set_magmoms
from quacc.utils.dicts import sort_dict

Expand Down Expand Up @@ -181,10 +180,6 @@ def _manage_environment(self) -> str:
"VASP_VDW setting was not provided, yet you requested a vdW functional."
)

if self.use_custodian:
run_vasp_custodian_file = Path(inspect.getfile(vasp_custodian)).resolve()
return f"{sys.executable} {run_vasp_custodian_file}"

# Return vanilla ASE command
vasp_cmd = (
SETTINGS.VASP_GAMMA_CMD
Expand Down Expand Up @@ -290,3 +285,39 @@ def _cleanup_params(self) -> None:
self.user_calc_params = sort_dict(
normalize_params(remove_unused_flags(self.user_calc_params))
)

def _run(
self,
command: list[str] | None = None,
out: Path | str | None = None,
directory: Path | str | None = None,
) -> int:
"""
Override the Vasp calculator's run method to use Custodian if necessary.
Parameters
----------
command
The command to run the VASP calculation. If None, will use the
self.command attribute.
out
The stdout file path.
directory
The directory to run the calculation in. If None, will use the
self.directory attribute.
Returns
-------
int
The return code.
"""
if command is None:
command = self.command
if directory is None:
directory = self.directory

if self.use_custodian:
run_custodian(directory=directory)
return 0
else:
return subprocess.call(command, shell=True, stdout=out, cwd=directory)
2 changes: 2 additions & 0 deletions src/quacc/calculators/vasp/vasp_custodian.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def run_custodian(
vasp_custodian_handlers: list[str] | None = None,
vasp_custodian_validators: list[str] | None = None,
scratch_dir: str | None = None,
directory: str | None = "./",
vasp_job_kwargs: VaspJobKwargs | None = None,
custodian_kwargs: CustodianKwargs | None = None,
) -> list[list[dict]]:
Expand Down Expand Up @@ -202,6 +203,7 @@ def run_custodian(
validators=validators,
max_errors=vasp_custodian_max_errors,
scratch_dir=scratch_dir,
directory=directory,
**custodian_kwargs,
)

Expand Down
5 changes: 5 additions & 0 deletions tests/core/calculators/vasp/test_vasp.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,3 +903,8 @@ def test_run(monkeypatch, tmp_path):
atoms = bulk("Cu")
calc = Vasp(atoms, xc="PBE", use_custodian=False)
assert calc._run() > 0

atoms = bulk("Cu")
calc = Vasp(atoms, xc="PBE", use_custodian=True)
with pytest.raises(FileNotFoundError):
calc._run()

0 comments on commit 62bed57

Please sign in to comment.