Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't call a subprocess with VASP custodian #1850

Merged
merged 12 commits into from
Mar 12, 2024
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()
Loading