Skip to content

Commit

Permalink
more value checking, move tests to launcher specific tests
Browse files Browse the repository at this point in the history
  • Loading branch information
juliaputko committed Oct 17, 2024
1 parent 67ba3a5 commit bb04488
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 41 deletions.
3 changes: 3 additions & 0 deletions smartsim/settings/arguments/batch/lsf.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ def set_walltime(self, walltime: str) -> None:
:param walltime: Time in hh:mm format, e.g. "10:00" for 10 hours,
if time is supplied in hh:mm:ss format, seconds
will be ignored and walltime will be set as ``hh:mm``
:raises TypeError: if not type str
"""
# For compatibility with other launchers, as explained in docstring
if not isinstance(walltime, str):
raise TypeError("walltime argument was not of type str")
if walltime:
if len(walltime.split(":")) > 2:
walltime = ":".join(walltime.split(":")[:2])
Expand Down
7 changes: 5 additions & 2 deletions smartsim/settings/arguments/batch/pbs.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ def set_walltime(self, walltime: str) -> None:
:param walltime: wall time
:raises ValueError: if walltime format is invalid
:raises TypeError: if not type str
"""
if not isinstance(walltime, str):
raise TypeError("walltime argument was not of type str")
pattern = r"^\d{2}:\d{2}:\d{2}$"
if walltime and re.match(pattern, walltime):
self.set("walltime", walltime)
Expand All @@ -110,7 +113,7 @@ def set_queue(self, queue: str) -> None:
"""
if not isinstance(queue, str):
raise TypeError("queue argument was not of type str")
self.set("q", str(queue))
self.set("q", queue)

def set_ncpus(self, num_cpus: int) -> None:
"""Set the number of cpus obtained in each node.
Expand All @@ -134,7 +137,7 @@ def set_account(self, account: str) -> None:
"""
if not isinstance(account, str):
raise TypeError("account argument was not of type str")
self.set("A", str(account))
self.set("A", account)

def format_batch_args(self) -> t.List[str]:
"""Get the formatted batch arguments for a preview
Expand Down
11 changes: 10 additions & 1 deletion smartsim/settings/arguments/batch/slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ def set_walltime(self, walltime: str) -> None:
:param walltime: wall time
:raises ValueError: if walltime format is invalid
:raises TypeError: if not str
"""
if not isinstance(walltime, str):
raise TypeError("walltime argument was not of type str")
pattern = r"^\d{2}:\d{2}:\d{2}$"
if walltime and re.match(pattern, walltime):
self.set("time", str(walltime))
Expand Down Expand Up @@ -94,8 +97,11 @@ def set_partition(self, partition: str) -> None:
This sets ``--partition``.
:param partition: partition name
:raises TypeError: if not a str
"""
self.set("partition", str(partition))
if not isinstance(partition, str):
raise TypeError("partition argument was not of type str")
self.set("partition", partition)

def set_queue(self, queue: str) -> None:
"""alias for set_partition
Expand All @@ -115,7 +121,10 @@ def set_cpus_per_task(self, cpus_per_task: int) -> None:
This sets ``--cpus-per-task``
:param num_cpus: number of cpus to use per task
:raises TypeError: if not int
"""
if not isinstance(cpus_per_task, int):
raise TypeError("cpus_per_task argument was not of type int")
self.set("cpus-per-task", str(cpus_per_task))

def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None:
Expand Down
38 changes: 0 additions & 38 deletions tests/temp_tests/test_settings/test_batchSettings.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,41 +159,3 @@ def test_batch_arguments_type_set_queue(scheduler):
bs = BatchSettings(batch_scheduler=scheduler, env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="queue argument was not of type str"):
bs.batch_args.set_queue(27)


@pytest.mark.parametrize(
"scheduler",
[
pytest.param("slurm", id="slurm scheduler"),
pytest.param("lsf", id="bsub scheduler"),
pytest.param("pbs", id="qsub scheduler"),
],
)
def test_batch_arguments_type_set_hostlist(scheduler):
bs = BatchSettings(batch_scheduler=scheduler, env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="host_list argument must be a list of strings"):
bs.batch_args.set_hostlist([25, 37])


def test_batch_arguments_type_set_ncpus():
bs = BatchSettings(batch_scheduler="pbs", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="num_cpus argument was not of type int"):
bs.batch_args.set_ncpus("invalid")


def test_batch_arguments_type_set_smts():
bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="smts argument was not of type int"):
bs.batch_args.set_smts("invalid")


def test_batch_arguments_type_set_project():
bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="project argument was not of type str"):
bs.batch_args.set_project(27)


def test_batch_arguments_type_set_tasks():
bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="tasks argument was not of type int"):
bs.batch_args.set_tasks("invalid")
30 changes: 30 additions & 0 deletions tests/temp_tests/test_settings/test_lsfScheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,33 @@ def test_create_bsub():
lsfScheduler.batch_args.set_queue("default")
args = lsfScheduler.format_batch_args()
assert args == ["-core_isolation", "-nnodes", "1", "-W", "10:10", "-q", "default"]


def test_batch_arguments_type_set_hostlist(scheduler):
bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="host_list argument must be a list of strings"):
bs.batch_args.set_hostlist([25, 37])


def test_batch_arguments_type_set_smts():
bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="smts argument was not of type int"):
bs.batch_args.set_smts("invalid")


def test_batch_arguments_type_set_project():
bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="project argument was not of type str"):
bs.batch_args.set_project(27)


def test_batch_arguments_type_set_tasks():
bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="tasks argument was not of type int"):
bs.batch_args.set_tasks("invalid")


def test_batch_arguments_type_set_walltime():
bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="walltime argument was not of type str"):
bs.batch_args.set_walltime(27)
18 changes: 18 additions & 0 deletions tests/temp_tests/test_settings/test_pbsScheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,21 @@ def test_format_pbs_batch_args():
"-A",
"myproject",
]


def test_batch_arguments_type_set_hostlist(scheduler):
bs = BatchSettings(batch_scheduler="pbs", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="host_list argument must be a list of strings"):
bs.batch_args.set_hostlist([25, 37])


def test_batch_arguments_type_set_ncpus():
bs = BatchSettings(batch_scheduler="pbs", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="num_cpus argument was not of type int"):
bs.batch_args.set_ncpus("invalid")


def test_batch_arguments_type_set_walltime():
bs = BatchSettings(batch_scheduler="pbs", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="walltime argument was not of type str"):
bs.batch_args.set_walltime(27)
24 changes: 24 additions & 0 deletions tests/temp_tests/test_settings/test_slurmScheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,27 @@ def test_sbatch_manual():
formatted = slurmScheduler.format_batch_args()
result = ["--nodes=5", "--account=A3531", "--time=10:00:00"]
assert formatted == result


def test_batch_arguments_type_set_walltime():
bs = BatchSettings(batch_scheduler="slurm", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="walltime argument was not of type str"):
bs.batch_args.set_walltime(27)


def test_batch_arguments_type_set_cpus_per_task():
bs = BatchSettings(batch_scheduler="slurm", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="cpus_per_task argument was not of type int"):
bs.batch_args.set_cpus_per_task("invalid")


def test_batch_arguments_type_set_partition():
bs = BatchSettings(batch_scheduler="slurm", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="partition argument was not of type str"):
bs.batch_args.set_partition(27)


def test_batch_arguments_type_set_hostlist(scheduler):
bs = BatchSettings(batch_scheduler="slurm", env_vars={"ENV": "VAR"})
with pytest.raises(TypeError, match="host_list argument must be a list of strings"):
bs.batch_args.set_hostlist([25, 37])

0 comments on commit bb04488

Please sign in to comment.