From c2bea2a9e72470d1a8ff75cad017f90b993f9bba Mon Sep 17 00:00:00 2001 From: ElliottKasoar <45317199+ElliottKasoar@users.noreply.github.com> Date: Tue, 6 Aug 2024 20:31:21 +0100 Subject: [PATCH] Fix sequences of atoms (#245) * Raise error if sequence passed to single struct calcs * Test for struct inputs * Set default read_kwargs for CLI * Add tests for CLI read kwargs * Add NaCl trajetory for tests * Update janus_core/calculations/eos.py * Clarify reason for limiting ASE index --------- Co-authored-by: Alin Marin Elena --- janus_core/calculations/eos.py | 11 ++++- janus_core/calculations/geom_opt.py | 8 ++++ janus_core/calculations/md.py | 8 ++++ janus_core/calculations/phonons.py | 9 +++++ janus_core/cli/descriptors.py | 7 ++-- janus_core/cli/eos.py | 11 +++-- janus_core/cli/geomopt.py | 11 +++-- janus_core/cli/md.py | 11 +++-- janus_core/cli/phonons.py | 11 +++-- janus_core/cli/singlepoint.py | 7 ++-- janus_core/cli/types.py | 18 ++++++++- janus_core/cli/utils.py | 20 +++++++++ tests/data/NaCl-traj.xyz | 20 +++++++++ tests/test_eos.py | 18 +++++++++ tests/test_eos_cli.py | 54 +++++++++++++++++++++++++ tests/test_geom_opt.py | 22 ++++++++++ tests/test_geomopt_cli.py | 55 +++++++++++++++++++++++++ tests/test_md.py | 18 +++++++++ tests/test_md_cli.py | 63 +++++++++++++++++++++++++++++ tests/test_phonons.py | 19 +++++++++ tests/test_phonons_cli.py | 56 +++++++++++++++++++++++++ 21 files changed, 436 insertions(+), 21 deletions(-) create mode 100644 tests/data/NaCl-traj.xyz diff --git a/janus_core/calculations/eos.py b/janus_core/calculations/eos.py index f720ab27..6bf4823a 100644 --- a/janus_core/calculations/eos.py +++ b/janus_core/calculations/eos.py @@ -1,5 +1,6 @@ """Equation of State.""" +from collections.abc import Sequence from copy import copy from typing import Any, Optional @@ -22,7 +23,7 @@ class EoS(FileNameMixin): Parameters ---------- struct : Atoms - Structure. + Structure to calculate equation of state for. struct_name : Optional[str] Name of structure. Default is None. min_volume : float @@ -147,6 +148,14 @@ def __init__( # pylint: disable=too-many-arguments,too-many-locals self.write_kwargs = write_kwargs self.log_kwargs = log_kwargs + if not isinstance(struct, Atoms): + if isinstance(struct, Sequence) and isinstance(struct[0], Atoms): + raise NotImplementedError( + "The equation of state can only be calculated for one Atoms " + "object at a time currently" + ) + raise ValueError("`struct` must be an ASE Atoms object") + log_kwargs.setdefault("name", __name__) self.logger = config_logger(**log_kwargs) self.tracker = config_tracker(self.logger, **tracker_kwargs) diff --git a/janus_core/calculations/geom_opt.py b/janus_core/calculations/geom_opt.py index afce15c7..7ef05705 100644 --- a/janus_core/calculations/geom_opt.py +++ b/janus_core/calculations/geom_opt.py @@ -1,5 +1,6 @@ """Geometry optimization.""" +from collections.abc import Sequence from typing import Any, Callable, Optional, Union import warnings @@ -160,6 +161,13 @@ def __init__( # pylint: disable=too-many-arguments self.write_kwargs = write_kwargs self.traj_kwargs = traj_kwargs + if not isinstance(struct, Atoms): + if isinstance(struct, Sequence) and isinstance(struct[0], Atoms): + raise NotImplementedError( + "Only one Atoms object at a time can currently be optimized" + ) + raise ValueError("`struct` must be an ASE Atoms object") + FileNameMixin.__init__(self, self.struct, None, None) self.write_kwargs.setdefault( diff --git a/janus_core/calculations/md.py b/janus_core/calculations/md.py index 3e039ece..d209461e 100644 --- a/janus_core/calculations/md.py +++ b/janus_core/calculations/md.py @@ -1,6 +1,7 @@ # pylint: disable=too-many-lines """Run molecular dynamics simulations.""" +from collections.abc import Sequence import datetime from functools import partial from itertools import combinations_with_replacement @@ -319,6 +320,13 @@ def __init__( # pylint: disable=too-many-arguments,too-many-locals,too-many-sta self.ensemble = ensemble self.seed = seed + if not isinstance(struct, Atoms): + if isinstance(struct, Sequence) and isinstance(struct[0], Atoms): + raise NotImplementedError( + "MD can only be run for one Atoms object at a time currently" + ) + raise ValueError("`struct` must be an ASE Atoms object") + FileNameMixin.__init__(self, struct, struct_name, file_prefix, ensemble) self.write_kwargs.setdefault( diff --git a/janus_core/calculations/phonons.py b/janus_core/calculations/phonons.py index f575fb9f..a20fbae7 100644 --- a/janus_core/calculations/phonons.py +++ b/janus_core/calculations/phonons.py @@ -1,5 +1,6 @@ """Phonon calculations.""" +from collections.abc import Sequence from typing import Any, Optional from ase import Atoms @@ -134,6 +135,14 @@ def __init__( # pylint: disable=too-many-arguments,disable=too-many-locals tracker_kwargs : Optional[dict[str, Any]] Keyword arguments to pass to `config_tracker`. Default is {}. """ + if not isinstance(struct, Atoms): + if isinstance(struct, Sequence) and isinstance(struct[0], Atoms): + raise NotImplementedError( + "Phonons can only be calculated for one Atoms object at a time " + "currently" + ) + raise ValueError("`struct` must be an ASE Atoms object") + FileNameMixin.__init__(self, struct, struct_name, file_prefix) [minimize_kwargs, log_kwargs, tracker_kwargs] = none_to_dict( diff --git a/janus_core/cli/descriptors.py b/janus_core/cli/descriptors.py index 02514c34..7f015993 100644 --- a/janus_core/cli/descriptors.py +++ b/janus_core/cli/descriptors.py @@ -14,7 +14,7 @@ Device, LogPath, ModelPath, - ReadKwargs, + ReadKwargsAll, StructPath, Summary, WriteKwargs, @@ -63,7 +63,7 @@ def descriptors( ), ), ] = None, - read_kwargs: ReadKwargs = None, + read_kwargs: ReadKwargsAll = None, calc_kwargs: CalcKwargs = None, write_kwargs: WriteKwargs = None, log: LogPath = "descriptors.log", @@ -95,7 +95,8 @@ def descriptors( Path to save structure with calculated results. Default is inferred from name of the structure file. read_kwargs : Optional[dict[str, Any]] - Keyword arguments to pass to ase.io.read. Default is {}. + Keyword arguments to pass to ase.io.read. By default, + read_kwargs["index"] is ":". calc_kwargs : Optional[dict[str, Any]] Keyword arguments to pass to the selected calculator. Default is {}. write_kwargs : Optional[dict[str, Any]] diff --git a/janus_core/cli/eos.py b/janus_core/cli/eos.py index eb4c632a..2e6932a7 100644 --- a/janus_core/cli/eos.py +++ b/janus_core/cli/eos.py @@ -15,7 +15,7 @@ LogPath, MinimizeKwargs, ModelPath, - ReadKwargs, + ReadKwargsFirst, StructPath, Summary, WriteKwargs, @@ -25,6 +25,7 @@ end_summary, parse_typer_dicts, save_struct_calc, + set_read_kwargs_index, start_summary, yaml_converter_callback, ) @@ -70,7 +71,7 @@ def eos( arch: Architecture = "mace_mp", device: Device = "cpu", model_path: ModelPath = None, - read_kwargs: ReadKwargs = None, + read_kwargs: ReadKwargsFirst = None, calc_kwargs: CalcKwargs = None, file_prefix: Annotated[ Optional[Path], @@ -128,7 +129,8 @@ def eos( model_path : Optional[str] Path to MLIP model. Default is `None`. read_kwargs : Optional[dict[str, Any]] - Keyword arguments to pass to ase.io.read. Default is {}. + Keyword arguments to pass to ase.io.read. By default, + read_kwargs["index"] is 0. calc_kwargs : Optional[dict[str, Any]] Keyword arguments to pass to the selected calculator. Default is {}. file_prefix : Optional[PathLike] @@ -151,6 +153,9 @@ def eos( if not eos_type in get_args(EoSNames): raise ValueError(f"Fit type must be one of: {get_args(EoSNames)}") + # Read only first structure by default and ensure only one image is read + set_read_kwargs_index(read_kwargs) + # Set up single point calculator s_point = SinglePoint( struct_path=struct, diff --git a/janus_core/cli/geomopt.py b/janus_core/cli/geomopt.py index 410387d5..0c2abc02 100644 --- a/janus_core/cli/geomopt.py +++ b/janus_core/cli/geomopt.py @@ -15,7 +15,7 @@ LogPath, MinimizeKwargs, ModelPath, - ReadKwargs, + ReadKwargsFirst, StructPath, Summary, WriteKwargs, @@ -25,6 +25,7 @@ end_summary, parse_typer_dicts, save_struct_calc, + set_read_kwargs_index, start_summary, yaml_converter_callback, ) @@ -143,7 +144,7 @@ def geomopt( str, Option(help="Path if saving optimization frames. [default: None]"), ] = None, - read_kwargs: ReadKwargs = None, + read_kwargs: ReadKwargsFirst = None, calc_kwargs: CalcKwargs = None, minimize_kwargs: MinimizeKwargs = None, write_kwargs: WriteKwargs = None, @@ -192,7 +193,8 @@ def geomopt( traj : Optional[str] Path if saving optimization frames. Default is None. read_kwargs : Optional[dict[str, Any]] - Keyword arguments to pass to ase.io.read. Default is {}. + Keyword arguments to pass to ase.io.read. By default, + read_kwargs["index"] is 0. calc_kwargs : Optional[dict[str, Any]] Keyword arguments to pass to the selected calculator. Default is {}. minimize_kwargs : Optional[dict[str, Any]] @@ -215,6 +217,9 @@ def geomopt( [read_kwargs, calc_kwargs, minimize_kwargs, write_kwargs] ) + # Read only first structure by default and ensure only one image is read + set_read_kwargs_index(read_kwargs) + # Set up single point calculator s_point = SinglePoint( struct_path=struct, diff --git a/janus_core/cli/md.py b/janus_core/cli/md.py index fe385c25..42191051 100644 --- a/janus_core/cli/md.py +++ b/janus_core/cli/md.py @@ -17,7 +17,7 @@ MinimizeKwargs, ModelPath, PostProcessKwargs, - ReadKwargs, + ReadKwargsFirst, StructPath, Summary, WriteKwargs, @@ -27,6 +27,7 @@ end_summary, parse_typer_dicts, save_struct_calc, + set_read_kwargs_index, start_summary, yaml_converter_callback, ) @@ -80,7 +81,7 @@ def md( arch: Architecture = "mace_mp", device: Device = "cpu", model_path: ModelPath = None, - read_kwargs: ReadKwargs = None, + read_kwargs: ReadKwargsFirst = None, calc_kwargs: CalcKwargs = None, equil_steps: Annotated[ int, @@ -232,7 +233,8 @@ def md( model_path : Optional[str] Path to MLIP model. Default is `None`. read_kwargs : Optional[dict[str, Any]] - Keyword arguments to pass to ase.io.read. Default is {}. + Keyword arguments to pass to ase.io.read. By default, + read_kwargs["index"] is 0. calc_kwargs : Optional[dict[str, Any]] Keyword arguments to pass to the selected calculator. Default is {}. equil_steps : int @@ -330,6 +332,9 @@ def md( if not ensemble in get_args(Ensembles): raise ValueError(f"ensemble must be in {get_args(Ensembles)}") + # Read only first structure by default and ensure only one image is read + set_read_kwargs_index(read_kwargs) + # Set up single point calculator s_point = SinglePoint( struct_path=struct, diff --git a/janus_core/cli/phonons.py b/janus_core/cli/phonons.py index ef227ed0..2bbaaa74 100644 --- a/janus_core/cli/phonons.py +++ b/janus_core/cli/phonons.py @@ -15,7 +15,7 @@ LogPath, MinimizeKwargs, ModelPath, - ReadKwargs, + ReadKwargsFirst, StructPath, Summary, ) @@ -24,6 +24,7 @@ end_summary, parse_typer_dicts, save_struct_calc, + set_read_kwargs_index, start_summary, yaml_converter_callback, ) @@ -113,7 +114,7 @@ def phonons( arch: Architecture = "mace_mp", device: Device = "cpu", model_path: ModelPath = None, - read_kwargs: ReadKwargs = None, + read_kwargs: ReadKwargsFirst = None, calc_kwargs: CalcKwargs = None, file_prefix: Annotated[ Optional[Path], @@ -187,7 +188,8 @@ def phonons( model_path : Optional[str] Path to MLIP model. Default is `None`. read_kwargs : Optional[dict[str, Any]] - Keyword arguments to pass to ase.io.read. Default is {}. + Keyword arguments to pass to ase.io.read. By default, + read_kwargs["index"] is 0. calc_kwargs : Optional[dict[str, Any]] Keyword arguments to pass to the selected calculator. Default is {}. file_prefix : Optional[PathLike] @@ -208,6 +210,9 @@ def phonons( [read_kwargs, calc_kwargs, minimize_kwargs] ) + # Read only first structure by default and ensure only one image is read + set_read_kwargs_index(read_kwargs) + # Set up single point calculator s_point = SinglePoint( struct_path=struct, diff --git a/janus_core/cli/singlepoint.py b/janus_core/cli/singlepoint.py index f01e3a62..54c3eff1 100644 --- a/janus_core/cli/singlepoint.py +++ b/janus_core/cli/singlepoint.py @@ -13,7 +13,7 @@ Device, LogPath, ModelPath, - ReadKwargs, + ReadKwargsAll, StructPath, Summary, WriteKwargs, @@ -59,7 +59,7 @@ def singlepoint( ), ), ] = None, - read_kwargs: ReadKwargs = None, + read_kwargs: ReadKwargsAll = None, calc_kwargs: CalcKwargs = None, write_kwargs: WriteKwargs = None, log: LogPath = "singlepoint.log", @@ -87,7 +87,8 @@ def singlepoint( Path to save structure with calculated results. Default is inferred from name of the structure file. read_kwargs : Optional[dict[str, Any]] - Keyword arguments to pass to ase.io.read. Default is {}. + Keyword arguments to pass to ase.io.read. By default, + read_kwargs["index"] is ":". calc_kwargs : Optional[dict[str, Any]] Keyword arguments to pass to the selected calculator. Default is {}. write_kwargs : Optional[dict[str, Any]] diff --git a/janus_core/cli/types.py b/janus_core/cli/types.py index 0381df9e..0b0c4fb6 100644 --- a/janus_core/cli/types.py +++ b/janus_core/cli/types.py @@ -67,14 +67,28 @@ def __str__(self): Device = Annotated[str, Option(help="Device to run calculations on.")] ModelPath = Annotated[str, Option(help="Path to MLIP model. [default: None]")] -ReadKwargs = Annotated[ +ReadKwargsAll = Annotated[ TyperDict, Option( parser=parse_dict_class, help=( """ Keyword arguments to pass to ase.io.read. Must be passed as a dictionary - wrapped in quotes, e.g. "{'key' : value}". [default: "{}"] + wrapped in quotes, e.g. "{'key' : value}". [default: "{'index': ':'}"] + """ + ), + metavar="DICT", + ), +] + +ReadKwargsFirst = Annotated[ + TyperDict, + Option( + parser=parse_dict_class, + help=( + """ + Keyword arguments to pass to ase.io.read. Must be passed as a dictionary + wrapped in quotes, e.g. "{'key' : value}". [default: "{'index': 0}"] """ ), metavar="DICT", diff --git a/janus_core/cli/utils.py b/janus_core/cli/utils.py index 8dae36bf..ae191d19 100644 --- a/janus_core/cli/utils.py +++ b/janus_core/cli/utils.py @@ -17,6 +17,26 @@ from janus_core.helpers.utils import dict_remove_hyphens +def set_read_kwargs_index(read_kwargs: dict[str, Any]) -> None: + """ + Set default read_kwargs["index"] and check its value is an integer. + + To ensure only a single Atoms object is read, slices such as ":" are forbidden. + + Parameters + ---------- + read_kwargs : dict[str, Any] + Keyword arguments to be passed to ase.io.read. If specified, + read_kwargs["index"] must be an integer, and if not, a default value + of 0 is set. + """ + read_kwargs.setdefault("index", 0) + try: + int(read_kwargs["index"]) + except ValueError as e: + raise ValueError("`read_kwargs['index']` must be an integer") from e + + def parse_typer_dicts(typer_dicts: list[TyperDict]) -> list[dict]: """ Convert list of TyperDict objects to list of dictionaries. diff --git a/tests/data/NaCl-traj.xyz b/tests/data/NaCl-traj.xyz new file mode 100644 index 00000000..f5c5f14a --- /dev/null +++ b/tests/data/NaCl-traj.xyz @@ -0,0 +1,20 @@ +8 +Lattice="5.64 0.0 0.0 0.0 5.64 0.0 0.0 0.0 5.64" Properties=species:S:1:pos:R:3 pbc="T T T" +Na 0.00000000 0.00000000 0.00000000 +Cl 2.82000000 0.00000000 0.00000000 +Na 0.00000000 2.82000000 2.82000000 +Cl 2.82000000 2.82000000 2.82000000 +Na 2.82000000 0.00000000 2.82000000 +Cl 0.00000000 0.00000000 2.82000000 +Na 2.82000000 2.82000000 0.00000000 +Cl 0.00000000 2.82000000 0.00000000 +8 +Lattice="5.64 0.0 0.0 0.0 5.64 0.0 0.0 0.0 5.64" Properties=species:S:1:pos:R:3 pbc="T T T" +Na 0.00124690 0.00391954 0.00297262 +Cl 2.82041775 -0.00059359 -0.00108448 +Na 0.00180732 2.82298136 2.82316615 +Cl 2.81860026 2.81853489 2.82005323 +Na 2.82071352 0.00814158 2.81856713 +Cl -0.00211419 -0.00357673 2.81844051 +Na 2.82204155 2.81429554 0.00116447 +Cl -0.00065740 2.81958913 -0.00121527 diff --git a/tests/test_eos.py b/tests/test_eos.py index 32a6386d..1536d0bb 100644 --- a/tests/test_eos.py +++ b/tests/test_eos.py @@ -90,3 +90,21 @@ def test_extra_potentials(arch, device, tmp_path): assert len(lines) == 2 assert len(lines[1].split()) == 3 + + +def test_invalid_struct(): + """Test setting invalid structure.""" + single_point = SinglePoint( + struct_path=DATA_PATH / "benzene-traj.xyz", + architecture="mace_mp", + calc_kwargs={"model": MODEL_PATH}, + ) + + with pytest.raises(NotImplementedError): + EoS( + single_point.struct, + ) + with pytest.raises(ValueError): + EoS( + "structure", + ) diff --git a/tests/test_eos_cli.py b/tests/test_eos_cli.py index b41dbb9c..9a2c2d86 100644 --- a/tests/test_eos_cli.py +++ b/tests/test_eos_cli.py @@ -229,3 +229,57 @@ def test_error_write_geomopt(tmp_path): ) assert result.exit_code == 1 assert isinstance(result.exception, ValueError) + + +@pytest.mark.parametrize("read_kwargs", ["{'index': 1}", "{}"]) +def test_valid_traj_input(read_kwargs, tmp_path): + """Test valid trajectory input structure handled.""" + eos_raw_path = tmp_path / "traj-eos-raw.dat" + eos_fit_path = tmp_path / "traj-eos-fit.dat" + log_path = tmp_path / "test.log" + summary_path = tmp_path / "summary.yml" + + result = runner.invoke( + app, + [ + "eos", + "--struct", + DATA_PATH / "NaCl-traj.xyz", + "--read-kwargs", + read_kwargs, + "--file-prefix", + tmp_path / "traj", + "--log", + log_path, + "--summary", + summary_path, + ], + ) + assert result.exit_code == 0 + assert eos_raw_path.exists() + assert eos_fit_path.exists() + + +def test_invalid_traj_input(tmp_path): + """Test invalid trajectory input structure handled.""" + log_path = tmp_path / "test.log" + summary_path = tmp_path / "summary.yml" + + result = runner.invoke( + app, + [ + "eos", + "--struct", + DATA_PATH / "NaCl-traj.xyz", + "--read-kwargs", + "{'index': ':'}", + "--file-prefix", + tmp_path / "traj", + "--log", + log_path, + "--summary", + summary_path, + ], + ) + assert result.exit_code == 1 + assert isinstance(result.exception, ValueError) diff --git a/tests/test_geom_opt.py b/tests/test_geom_opt.py index a16ea128..116df5a3 100644 --- a/tests/test_geom_opt.py +++ b/tests/test_geom_opt.py @@ -310,3 +310,25 @@ def test_invalid_str_filter(): with pytest.raises(AttributeError): GeomOpt(single_point.struct, fmax=0.001, filter_func="test") + + +def test_invalid_struct(): + """Test setting invalid structure.""" + single_point = SinglePoint( + struct_path=DATA_PATH / "benzene-traj.xyz", + architecture="mace_mp", + calc_kwargs={"model": MODEL_PATH}, + ) + + with pytest.raises(NotImplementedError): + GeomOpt( + single_point.struct, + fmax=0.001, + optimizer="test", + ) + with pytest.raises(ValueError): + GeomOpt( + "structure", + fmax=0.001, + optimizer="test", + ) diff --git a/tests/test_geomopt_cli.py b/tests/test_geomopt_cli.py index eb257e14..bded1ff7 100644 --- a/tests/test_geomopt_cli.py +++ b/tests/test_geomopt_cli.py @@ -2,6 +2,7 @@ from pathlib import Path +from ase import Atoms from ase.io import read import pytest from typer.testing import CliRunner @@ -521,3 +522,57 @@ def test_filter_str_error(tmp_path): ) assert result.exit_code == 1 assert isinstance(result.exception, ValueError) + + +@pytest.mark.parametrize("read_kwargs", ["{'index': 1}", "{}"]) +def test_valid_traj_input(read_kwargs, tmp_path): + """Test valid trajectory input structure handled.""" + results_path = tmp_path / "benezene-traj.extxyz" + log_path = tmp_path / "test.log" + summary_path = tmp_path / "summary.yml" + + result = runner.invoke( + app, + [ + "geomopt", + "--struct", + DATA_PATH / "benzene-traj.xyz", + "--out", + results_path, + "--read-kwargs", + read_kwargs, + "--log", + log_path, + "--summary", + summary_path, + ], + ) + assert result.exit_code == 0 + atoms = read(results_path) + assert isinstance(atoms, Atoms) + + +def test_invalid_traj_input(tmp_path): + """Test invalid trajectory input structure handled.""" + results_path = tmp_path / "benezene-traj.extxyz" + log_path = tmp_path / "test.log" + summary_path = tmp_path / "summary.yml" + + result = runner.invoke( + app, + [ + "geomopt", + "--struct", + DATA_PATH / "benzene-traj.xyz", + "--out", + results_path, + "--read-kwargs", + "{'index': ':'}", + "--log", + log_path, + "--summary", + summary_path, + ], + ) + assert result.exit_code == 1 + assert isinstance(result.exception, ValueError) diff --git a/tests/test_md.py b/tests/test_md.py index 6e7c7dcb..f6fe64a1 100644 --- a/tests/test_md.py +++ b/tests/test_md.py @@ -842,3 +842,21 @@ def test_ensemble_kwargs(tmp_path): expected_mask = [[False, False, False], [False, True, False], [False, False, False]] assert np.array_equal(npt.dyn.mask, expected_mask) + + +def test_invalid_struct(): + """Test setting invalid structure.""" + single_point = SinglePoint( + struct_path=DATA_PATH / "benzene-traj.xyz", + architecture="mace_mp", + calc_kwargs={"model": MODEL_PATH}, + ) + + with pytest.raises(NotImplementedError): + NPT( + single_point.struct, + ) + with pytest.raises(ValueError): + NPT( + "structure", + ) diff --git a/tests/test_md_cli.py b/tests/test_md_cli.py index fb32cfe9..85ea937d 100644 --- a/tests/test_md_cli.py +++ b/tests/test_md_cli.py @@ -557,3 +557,66 @@ def test_write_kwargs(tmp_path): # Check labelled info has been set assert "mace_mp_energy" in final_atoms.info assert "mace_mp_energy" in traj[0].info + + +@pytest.mark.parametrize("read_kwargs", ["{'index': 1}", "{}"]) +def test_valid_traj_input(read_kwargs, tmp_path): + """Test valid trajectory input structure handled.""" + file_prefix = tmp_path / "traj" + final_path = tmp_path / "traj-final.extxyz" + log_path = tmp_path / "test.log" + summary_path = tmp_path / "summary.yml" + + result = runner.invoke( + app, + [ + "md", + "--ensemble", + "nvt", + "--struct", + DATA_PATH / "benzene-traj.xyz", + "--file-prefix", + file_prefix, + "--steps", + 2, + "--read-kwargs", + read_kwargs, + "--log", + log_path, + "--summary", + summary_path, + ], + ) + assert result.exit_code == 0 + atoms = read(final_path) + assert isinstance(atoms, Atoms) + + +def test_invalid_traj_input(tmp_path): + """Test invalid trajectory input structure handled.""" + file_prefix = tmp_path / "traj" + log_path = tmp_path / "test.log" + summary_path = tmp_path / "summary.yml" + + result = runner.invoke( + app, + [ + "md", + "--ensemble", + "nvt", + "--struct", + DATA_PATH / "benzene-traj.xyz", + "--file-prefix", + file_prefix, + "--steps", + 2, + "--read-kwargs", + "{'index': ':'}", + "--log", + log_path, + "--summary", + summary_path, + ], + ) + assert result.exit_code == 1 + assert isinstance(result.exception, ValueError) diff --git a/tests/test_phonons.py b/tests/test_phonons.py index 770e7f41..ffc0c4ba 100644 --- a/tests/test_phonons.py +++ b/tests/test_phonons.py @@ -3,6 +3,7 @@ from pathlib import Path from ase.io import read +import pytest from janus_core.calculations.phonons import Phonons from janus_core.calculations.single_point import SinglePoint @@ -59,3 +60,21 @@ def test_optimize(tmp_path): log_file, includes=["Using filter", "Using optimizer", "Starting geometry optimization"], ) + + +def test_invalid_struct(): + """Test setting invalid structure.""" + single_point = SinglePoint( + struct_path=DATA_PATH / "benzene-traj.xyz", + architecture="mace_mp", + calc_kwargs={"model": MODEL_PATH}, + ) + + with pytest.raises(NotImplementedError): + Phonons( + single_point.struct, + ) + with pytest.raises(ValueError): + Phonons( + "structure", + ) diff --git a/tests/test_phonons_cli.py b/tests/test_phonons_cli.py index ea78a499..974a38e4 100644 --- a/tests/test_phonons_cli.py +++ b/tests/test_phonons_cli.py @@ -13,6 +13,10 @@ runner = CliRunner() +# Many pylint now warnings raised due to similar log/summary flags +# These depend on tmp_path, so not easily refactorisable +# pylint: disable=duplicate-code + def test_help(): """Test calling `janus phonons --help`.""" @@ -343,3 +347,55 @@ def test_minimize_filename(tmp_path): ) assert result.exit_code == 0 assert opt_path.exists() + + +@pytest.mark.parametrize("read_kwargs", ["{'index': 0}", "{}"]) +def test_valid_traj_input(read_kwargs, tmp_path): + """Test valid trajectory input structure handled.""" + phonon_results = tmp_path / "traj-phonopy.yml" + log_path = tmp_path / "test.log" + summary_path = tmp_path / "summary.yml" + + result = runner.invoke( + app, + [ + "phonons", + "--struct", + DATA_PATH / "NaCl-traj.xyz", + "--read-kwargs", + read_kwargs, + "--file-prefix", + tmp_path / "traj", + "--log", + log_path, + "--summary", + summary_path, + ], + ) + assert result.exit_code == 0 + assert phonon_results.exists() + + +def test_invalid_traj_input(tmp_path): + """Test invalid trajectory input structure handled.""" + log_path = tmp_path / "test.log" + summary_path = tmp_path / "summary.yml" + + result = runner.invoke( + app, + [ + "phonons", + "--struct", + DATA_PATH / "NaCl-traj.xyz", + "--read-kwargs", + "{'index': ':'}", + "--file-prefix", + tmp_path / "traj", + "--log", + log_path, + "--summary", + summary_path, + ], + ) + assert result.exit_code == 1 + assert isinstance(result.exception, ValueError)