diff --git a/.github/workflows/pytests.yml b/.github/workflows/pytests.yml index 56de2f68..3aaf61e6 100644 --- a/.github/workflows/pytests.yml +++ b/.github/workflows/pytests.yml @@ -77,6 +77,7 @@ jobs: fi source $mklvars intel64 + # pip constraint needs to be an absolute filename export PIP_CONSTRAINT=$PWD/$PIP_CONSTRAINT git clone https://github.com/phonopy/phonopy @@ -201,7 +202,15 @@ jobs: echo "which pw.x" which pw.x ls -l /usr/bin/pw.x - export PYTEST_WFL_ASE_ESPRESSO_COMMAND=pw.x + espresso_command=pw.x + + mkdir -p ${HOME}/.config/ase/ + echo "[espresso]" >> ${HOME}/.config/ase/config.ini + echo "command = ${espresso_command}" >> ${HOME}/.config/ase/config.ini + echo "pseudo_dir = ${HOME}/dummy" >> ${HOME}/.config/ase/config.ini + + echo 'post-espresso $HOME/.config/ase/config.ini' + cat $HOME/.config/ase/config.ini - name: Lint with flake8 run: | diff --git a/tests/calculators/test_qe.py b/tests/calculators/test_qe.py index 22dd9492..40cf3b3b 100644 --- a/tests/calculators/test_qe.py +++ b/tests/calculators/test_qe.py @@ -5,12 +5,14 @@ from shutil import which, copy as shutil_copy from pathlib import Path import pytest +from packaging.version import Version import ase.io import numpy as np import requests from ase import Atoms from ase.build import bulk +from ase.calculators.espresso import EspressoProfile from pytest import approx, fixture, raises, skip # from wfl.calculators.espresso import evaluate_autopara_wrappable, qe_kpoints_and_kwargs @@ -19,29 +21,36 @@ from wfl.configset import ConfigSet, OutputSpec from wfl.autoparallelize import AutoparaInfo +ase_version = pytest.mark.skipif(Version(ase.__version__) < Version("3.23"), + reason="Quantum espresso tests are only supported for ASE v3.23, " + f"please update from {ase.__version__}.") + +from ase.config import cfg as ase_cfg +from ase.calculators.espresso import EspressoProfile + +# do all tests using user's default config file +# pseudo_dir will be overridden whenever calculator is constructed to ensure that +# pytest-specific PPs are used +espresso_avail = pytest.mark.skipif(not ("espresso" in ase_cfg.parser and os.environ.get('OMP_NUM_THREADS') == "1"), + reason='No "espresso" ASE configuration or ' + f'"OMP_NUM_THREADS={os.environ.get("OMP_NUM_THREADS")}" is not set to 1.') + @fixture(scope="session") -def qe_cmd_and_pseudo(tmp_path_factory): +def qe_pseudo(tmp_path_factory): """Quantum Espresso fixture - - checks if pw.x exists (skip otherwise) - - downloads a pseudo-potential for Si + - copies a pseudo-potential for Si implementation based on: https://stackoverflow.com/questions/63417661/pytest-downloading-a-test-file-once-and-using-it-for-multiple-tests Returns ------- - cmd: str - command for pw.x pspot_file: str - Si pseudo potential file + Si pseudo potential file name """ - cmd = os.environ.get("PYTEST_WFL_ASE_ESPRESSO_COMMAND") - if cmd is None: - skip("no PYTEST_WFL_ASE_ESPRESSO_COMMAND to specify executable") - # originally downloaded from here, but broken due to need for account/license click # url = "https://www.quantum-espresso.org/upf_files/Si.pbe-n-kjpaw_psl.1.0.0.UPF" # replaced with this @@ -53,27 +62,29 @@ def qe_cmd_and_pseudo(tmp_path_factory): # write to a temporary file pspot_file = tmp_path_factory.getbasetemp() / "Si.UPF" - shutil_copy(Path(__file__).parent / ".." / "assets" / "QE" / "Si.pz-vbc.UPF", pspot_file) + shutil_copy(Path(__file__).parent.parent / "assets" / "QE" / "Si.pz-vbc.UPF", pspot_file) - return cmd, pspot_file + return pspot_file -def test_qe_kpoints(tmp_path, qe_cmd_and_pseudo): - qe_cmd, pspot = qe_cmd_and_pseudo +@ase_version +@espresso_avail +def test_qe_kpoints(tmp_path, qe_pseudo): + + pspot = qe_pseudo kw = dict( - pseudopotentials=dict(Si=os.path.basename(pspot)), + pseudopotentials=dict(Si=pspot.name), + pseudo_dir=pspot.parent, input_data={"SYSTEM": {"ecutwfc": 40, "input_dft": "LDA",}}, kpts=(2, 3, 4), conv_thr=0.0001, - calculator_exec=qe_cmd, - pseudo_dir=os.path.dirname(pspot), workdir=tmp_path - ) + ) # PBC = TTT atoms = Atoms("H", cell=[1, 1, 1], pbc=True) - properties = ["energy", "stress"] + properties = ["energy", "stress"] calc = wfl.calculators.espresso.Espresso(**kw) calc.atoms = atoms.copy() calc.setup_calc_params(properties) @@ -83,7 +94,7 @@ def test_qe_kpoints(tmp_path, qe_cmd_and_pseudo): # PBC = FFF atoms = Atoms("H", cell=[1, 1, 1], pbc=False) - properties = ["energy", "stress", "forces"] + properties = ["energy", "stress", "forces"] ## removing stress here to duplicate what calculators.generic would do properties.remove("stress") ## @@ -104,7 +115,7 @@ def test_qe_kpoints(tmp_path, qe_cmd_and_pseudo): # PBC mixed -- kpts atoms = Atoms("H", cell=[1, 1, 1], pbc=[True, False, False]) - properties = ["energy", "stress", "forces"] + properties = ["energy", "stress", "forces"] kw["koffset"] = True calc = wfl.calculators.espresso.Espresso(**kw) calc.atoms = atoms.copy() @@ -125,8 +136,8 @@ def test_qe_kpoints(tmp_path, qe_cmd_and_pseudo): # koffset in mixed PBC atoms = Atoms("H", cell=[1, 1, 1], pbc=[True, False, False]) - properties = ["energy", "forces"] - kw["koffset"] = False + properties = ["energy", "forces"] + kw["koffset"] = False calc = wfl.calculators.espresso.Espresso(**kw) calc.atoms = atoms.copy() calc.setup_calc_params(properties) @@ -136,7 +147,7 @@ def test_qe_kpoints(tmp_path, qe_cmd_and_pseudo): # PBC mixed -- kspacing atoms = Atoms("H", cell=[1, 1, 1], pbc=[True, False, False]) - properties = ["energy", "stress", "forces"] + properties = ["energy", "stress", "forces"] kw["kspacing"] = 0.1 kw["koffset"] = (0, 1, 0) calc = wfl.calculators.espresso.Espresso(**kw) @@ -155,9 +166,11 @@ def test_qe_kpoints(tmp_path, qe_cmd_and_pseudo): assert calc.parameters["koffset"] == (0, 0, 0) -def test_qe_calculation(tmp_path, qe_cmd_and_pseudo): - # command and pspot - qe_cmd, pspot = qe_cmd_and_pseudo +@ase_version +@espresso_avail +def test_qe_calculation(tmp_path, qe_pseudo): + + pspot = qe_pseudo # atoms at = bulk("Si") @@ -165,14 +178,13 @@ def test_qe_calculation(tmp_path, qe_cmd_and_pseudo): at0 = Atoms("Si", cell=[6.0, 6.0, 6.0], positions=[[3.0, 3.0, 3.0]], pbc=False) kw = dict( - pseudopotentials=dict(Si=os.path.basename(pspot)), + pseudopotentials=dict(Si=pspot.name), + pseudo_dir=pspot.parent, input_data={"SYSTEM": {"ecutwfc": 40, "input_dft": "LDA",}}, kpts=(2, 2, 2), conv_thr=0.0001, - calculator_exec=qe_cmd, - pseudo_dir=os.path.dirname(pspot), workdir=tmp_path - ) + ) calc = (wfl.calculators.espresso.Espresso, [], kw) @@ -181,8 +193,8 @@ def test_qe_calculation(tmp_path, qe_cmd_and_pseudo): results = generic.calculate( inputs=[at0, at], - outputs=c_out, - calculator=calc, + outputs=c_out, + calculator=calc, output_prefix='QE_', ) @@ -215,22 +227,24 @@ def test_qe_calculation(tmp_path, qe_cmd_and_pseudo): assert si2.arrays["QE_forces"][0] == approx(-1 * si2.arrays["QE_forces"][1]) -def test_wfl_Espresso_calc(tmp_path, qe_cmd_and_pseudo): - # command and pspot - qe_cmd, pspot = qe_cmd_and_pseudo +@ase_version +@espresso_avail +def test_wfl_Espresso_calc(tmp_path, qe_pseudo): + + pspot = qe_pseudo atoms = Atoms("Si", cell=(2, 2, 2), pbc=[True] * 3) kw = dict( - pseudopotentials=dict(Si=os.path.basename(pspot)), + pseudopotentials=dict(Si=pspot.name), + pseudo_dir=pspot.parent, input_data={"SYSTEM": {"ecutwfc": 40, "input_dft": "LDA",}}, kpts=(2, 2, 2), - conv_thr=0.0001, - calculator_exec=qe_cmd, - pseudo_dir=os.path.dirname(pspot) - ) + conv_thr=0.0001 + ) calc = wfl.calculators.espresso.Espresso( workdir=tmp_path, + keep_files=True, **kw) atoms.calc = calc @@ -239,20 +253,21 @@ def test_wfl_Espresso_calc(tmp_path, qe_cmd_and_pseudo): atoms.get_stress() -def test_wfl_Espresso_calc_via_generic(tmp_path, qe_cmd_and_pseudo): +@ase_version +@espresso_avail +def test_wfl_Espresso_calc_via_generic(tmp_path, qe_pseudo): - qe_cmd, pspot = qe_cmd_and_pseudo + pspot = qe_pseudo atoms = Atoms("Si", cell=(2, 2, 2), pbc=[True] * 3) kw = dict( - pseudopotentials=dict(Si=os.path.basename(pspot)), + pseudopotentials=dict(Si=pspot.name), + pseudo_dir=pspot.parent, input_data={"SYSTEM": {"ecutwfc": 40, "input_dft": "LDA",}}, kpts=(2, 2, 2), conv_thr=0.0001, - calculator_exec=qe_cmd, - pseudo_dir=os.path.dirname(pspot), workdir=tmp_path - ) + ) calc = (wfl.calculators.espresso.Espresso, [], kw) @@ -265,8 +280,8 @@ def test_wfl_Espresso_calc_via_generic(tmp_path, qe_cmd_and_pseudo): ci = generic.calculate( inputs=ci, - outputs=co, - calculator=calc, + outputs=co, + calculator=calc, output_prefix='qe_', autopara_info=autoparainfo ) @@ -274,5 +289,3 @@ def test_wfl_Espresso_calc_via_generic(tmp_path, qe_cmd_and_pseudo): ats = list(ci) assert not any("qe_calculation_failed" in at.info for at in ats[:-1]) assert "qe_calculation_failed" in list(ci)[-1].info - - diff --git a/tests/local_scripts/complete_pytest.tin b/tests/local_scripts/complete_pytest.tin index 7bd2b0da..2467ee14 100755 --- a/tests/local_scripts/complete_pytest.tin +++ b/tests/local_scripts/complete_pytest.tin @@ -32,7 +32,6 @@ export ASE_VASP_COMMAND_GAMMA=vasp.gamma.serial export PYTEST_VASP_POTCAR_DIR=$VASP_PATH/pot/rev_54/PBE # QE module load dft/pwscf -export PYTEST_WFL_ASE_ESPRESSO_COMMAND="env MPIRUN_EXTRA_ARGS='-np 1' pw.x" # no ORCA export OPENBLAS_NUM_THREADS=1 @@ -58,6 +57,9 @@ if [ -d $pytest_dir ]; then echo "Refusing to run after failing to delete $pytest_dir" 1>&2 exit 1 fi + +mkdir -p $pytest_dir + pytest -v -s --basetemp $pytest_dir ${runremote} --runslow --runperf -rxXs "$@" >> complete_pytest.tin.out 2>&1 l=`egrep '^=.*(passed|failed|skipped|xfailed)' complete_pytest.tin.out` @@ -65,7 +67,7 @@ l=`egrep '^=.*(passed|failed|skipped|xfailed)' complete_pytest.tin.out` echo "summary line $l" lp=$( echo $l | sed -E -e 's/ in .*//' -e 's/\s*,\s*/\n/g' ) -declare -A expected_n=( ["passed"]="176" ["skipped"]="21" ["warnings"]=823 ["xfailed"]=2 ["xpassed"]=1 ) +declare -A expected_n=( ["passed"]="175" ["skipped"]="21" ["warnings"]=823 ["xfailed"]=2 ["xpassed"]=1 ) IFS=$'\n' for out in $lp; do out_n=$(echo $out | sed -e 's/^=* //' -e 's/ .*//' -e 's/,//') diff --git a/tests/test_minimahopping.py b/tests/test_minimahopping.py index 770b9ec1..65cc9a46 100644 --- a/tests/test_minimahopping.py +++ b/tests/test_minimahopping.py @@ -31,8 +31,8 @@ def test_return_md_traj(cu_slab, tmp_path): atoms_opt = minimahopping.minimahopping(inputs, outputs, calc, fmax=1, totalsteps=5, save_tmpdir=True, return_all_traj=True, rng=np.random.default_rng(1), workdir=tmp_path) - assert any(["minima" in at.info["config_type"] for at in atoms_opt]) - assert any(["traj" in at.info["config_type"] for at in atoms_opt]) + assert any(["minhop_min" in at.info["config_type"] for at in atoms_opt]) + assert any(["minhop_traj" in at.info["config_type"] for at in atoms_opt]) def test_mult_files(cu_slab, tmp_path): @@ -82,8 +82,8 @@ def test_relax(cu_slab, tmp_path): assert 1 <= len(list(ats)) <= totalsteps atoms_opt = list(atoms_opt) - assert all(['minima' in at.info['config_type'] for at in atoms_opt]) + assert all(['minhop_min' in at.info['config_type'] for at in atoms_opt]) for at in atoms_opt: - force_norms = np.linalg.norm(at.get_forces(), axis=1) + force_norms = np.linalg.norm(at.arrays["last_op__minhop_forces"], axis=1) assert all(force_norms <= fmax) diff --git a/tests/test_remote_run.py b/tests/test_remote_run.py index 01f924bb..6a1e9318 100644 --- a/tests/test_remote_run.py +++ b/tests/test_remote_run.py @@ -190,9 +190,6 @@ def do_generic_calc_qe(tmp_path, sys_name, monkeypatch, remoteinfo_env): 'resources': {'max_time': '1h', 'num_nodes': 1}, 'num_inputs_per_queued_job': -36, 'check_interval': 10} - qe_cmd = os.environ.get("PYTEST_WFL_ASE_ESPRESSO_COMMAND") - if qe_cmd is None: - pytest.skip("no PYTEST_WFL_ASE_ESPRESSO_COMMAND to specify executable") pspot = tmp_path / "Si.UPF" shutil.copy(Path(__file__).parent / "assets" / "QE" / "Si.pz-vbc.UPF", pspot) @@ -208,7 +205,6 @@ def do_generic_calc_qe(tmp_path, sys_name, monkeypatch, remoteinfo_env): input_data={"SYSTEM": {"ecutwfc": 40, "input_dft": "LDA",}}, kpts=(2, 2, 2), conv_thr=0.0001, - calculator_exec=qe_cmd, pseudo_dir=str(pspot.parent) ) diff --git a/wfl/calculators/aims.py b/wfl/calculators/aims.py index 2088fb96..be40a915 100644 --- a/wfl/calculators/aims.py +++ b/wfl/calculators/aims.py @@ -2,8 +2,6 @@ FHI-Aims Calculator """ -import shlex - from copy import deepcopy import numpy as np diff --git a/wfl/calculators/espresso.py b/wfl/calculators/espresso.py index 79f72bea..c3300361 100644 --- a/wfl/calculators/espresso.py +++ b/wfl/calculators/espresso.py @@ -10,15 +10,10 @@ from ase.calculators.calculator import all_changes from ase.calculators.espresso import Espresso as ASE_Espresso -try: - from ase.calculators.espresso import EspressoProfile -except ImportError: - EspressoProfile = None from ase.io.espresso import kspacing_to_grid from .wfl_fileio_calculator import WFLFileIOCalculator from wfl.utils.save_calc_results import save_calc_results -from .utils import parse_genericfileio_profile_argv # NOMAD compatible, see https://nomad-lab.eu/prod/rae/gui/uploads _default_keep_files = ["*.pwo"] @@ -45,10 +40,6 @@ class Espresso(WFLFileIOCalculator, ASE_Espresso): scratchdir: str / Path, default None temporary directory to execute calculations in and delete or copy back results (set by "keep_files") if needed. For example, directory on a local disk with fast file I/O. - calculator_exec: str - command for QE, without any prefix or redirection set. - for example: "mpirun -n 4 /path/to/pw.x" - mutually exclusive with "command" with "profile" **kwargs: arguments for ase.calculators.espresso.Espresso """ @@ -59,58 +50,10 @@ class Espresso(WFLFileIOCalculator, ASE_Espresso): wfl_generic_default_autopara_info = {"num_inputs_per_python_subprocess": 1} def __init__(self, keep_files="default", rundir_prefix="run_QE_", - workdir=None, scratchdir=None, - calculator_exec=None, **kwargs): + workdir=None, scratchdir=None, **kwargs): kwargs_command = deepcopy(kwargs) - # check for various Espresso versions - # NOTE: should we be doing this much massaging of inputs, or should we make the user keep up - # with their ASE Espresso version? - if EspressoProfile is not None: - # new version, command and ASE_ESPRESSO_COMMAND deprecated - if "command" in kwargs_command: - raise ValueError("Espresso calculator defines EspressoProfile, but deprecated 'command' arg was passed") - - if calculator_exec is not None: - # check for conflicts, wrong format - if "profile" in kwargs_command: - raise ValueError("Cannot specify both calculator_exec and profile") - if " -in " in calculator_exec: - raise ValueError("calculator_exec should not include espresso command line arguments such as ' -in PREFIX.pwi'") - - # newer syntax, but pass binary without a keyword (which changed from "argv" to "exc" - # to "binary" over time), assuming it's first argument - argv = shlex.split(calculator_exec) - try: - kwargs_command["profile"] = EspressoProfile(argv=argv) - except TypeError: - binary, parallel_info = parse_genericfileio_profile_argv(argv) - # argument names keep changing (e.g. pseudo_path -> pseudo_dir), just pass first two as positional - # and hope order doesn't change - if "pseudo_dir" not in kwargs_command: - raise ValueError(f"calculator_exec also requires pseudo_dir to create EspressoProfile") - kwargs_command["profile"] = EspressoProfile(binary, kwargs_command.pop("pseudo_dir"), - parallel_info=parallel_info) - elif "profile" not in kwargs_command: - raise ValueError("EspressoProfile is defined but neither calculator_exec nor profile was specified") - - # better be defined by now - assert "profile" in kwargs_command - else: - # old (pre EspressoProfile) version - if "profile" in kwargs_command: - raise ValueError("EspressoProfile is not defined (old version) but profile was passed") - - if calculator_exec is not None: - if "command" in kwargs_command: - raise ValueError("Cannot specify both command and calc_exec") - - kwargs_command["command"] = f"{calculator_exec} -in PREFIX.pwi > PREFIX.pwo" - - # command or env var must be set - assert "command" in kwargs_command or "ASE_ESPRESSO_CALCULATOR" in os.environ - # WFLFileIOCalculator is a mixin, will call remaining superclass constructors for us super().__init__(keep_files=keep_files, rundir_prefix=rundir_prefix, workdir=workdir, scratchdir=scratchdir, **kwargs_command) diff --git a/wfl/calculators/generic.py b/wfl/calculators/generic.py index 37e9a86b..50c4b350 100644 --- a/wfl/calculators/generic.py +++ b/wfl/calculators/generic.py @@ -51,7 +51,7 @@ def _run_autopara_wrappable(atoms, calculator, properties=None, output_prefix='_ except Exception as exc: # if calculator constructor failed, it may still be fine if every atoms object has # enough info to construct its own calculator, but we won't know until later - calculator_failure_message = f"(exc)\n{traceback.format_exc()}" + calculator_failure_message = f"({exc})\n{traceback.format_exc()}" calculator_default = None if output_prefix == '_auto_': diff --git a/wfl/generate/minimahopping.py b/wfl/generate/minimahopping.py index 5851be1f..481806ea 100644 --- a/wfl/generate/minimahopping.py +++ b/wfl/generate/minimahopping.py @@ -10,26 +10,28 @@ import ase.io from wfl.autoparallelize import autoparallelize, autoparallelize_docstring +from wfl.utils.save_calc_results import at_copy_save_calc_results from wfl.utils.misc import atoms_to_list from .utils import save_config_type from wfl.utils.parallel import construct_calculator_picklesafe -def _get_MD_trajectory(rundir, update_config_type): +def _get_MD_trajectory(rundir, update_config_type, prefix): md_traj = [] mdtrajfiles = sorted([file for file in Path(rundir).glob("md*.traj")]) for mdtraj in mdtrajfiles: for at in ase.io.read(f"{mdtraj}", ":"): - save_config_type(at, update_config_type, 'minimahop_traj') - md_traj.append(at) + new_config = at_copy_save_calc_results(at, prefix=prefix) + save_config_type(new_config, update_config_type, 'minhop_traj') + md_traj.append(new_config) return md_traj # perform MinimaHopping on one ASE.atoms object def _atom_opt_hopping(atom, calculator, Ediff0, T0, minima_threshold, mdmin, - fmax, timestep, totalsteps, skip_failures, update_config_type, + fmax, timestep, totalsteps, skip_failures, update_config_type, results_prefix, workdir=None, **opt_kwargs): save_tmpdir = opt_kwargs.pop("save_tmpdir", False) return_all_traj = opt_kwargs.pop("return_all_traj", False) @@ -61,11 +63,12 @@ def _atom_opt_hopping(atom, calculator, Ediff0, T0, minima_threshold, mdmin, else: traj = [] if return_all_traj: - traj += _get_MD_trajectory(rundir, update_config_type) + traj += _get_MD_trajectory(rundir, update_config_type, prefix=results_prefix) for hop_traj in Trajectory('minima.traj'): - save_config_type(hop_traj, update_config_type, 'minimahop_min') - traj.append(hop_traj) + new_config = at_copy_save_calc_results(hop_traj, prefix=results_prefix) + save_config_type(new_config, update_config_type, 'minhop_min') + traj.append(new_config) if not save_tmpdir: os.chdir(workdir) shutil.rmtree(rundir) @@ -77,7 +80,7 @@ def _atom_opt_hopping(atom, calculator, Ediff0, T0, minima_threshold, mdmin, def _run_autopara_wrappable(atoms, calculator, Ediff0=1, T0=1000, minima_threshold=0.5, mdmin=2, fmax=1, timestep=1, totalsteps=10, skip_failures=True, update_config_type="append", - workdir=None, rng=None, _autopara_per_item_info=None, + results_prefix='last_op__minhop_', workdir=None, rng=None, _autopara_per_item_info=None, **opt_kwargs): """runs a structure optimization @@ -131,7 +134,7 @@ def _run_autopara_wrappable(atoms, calculator, Ediff0=1, T0=1000, minima_thresho traj = _atom_opt_hopping(atom=at, calculator=calculator, Ediff0=Ediff0, T0=T0, minima_threshold=minima_threshold, mdmin=mdmin, fmax=fmax, timestep=timestep, totalsteps=totalsteps, - skip_failures=skip_failures, update_config_type=update_config_type, + skip_failures=skip_failures, update_config_type=update_config_type, results_prefix=results_prefix, workdir=workdir, **opt_kwargs) all_trajs.append(traj)