Skip to content

Commit

Permalink
Runtime value checking of public API methods (#707)
Browse files Browse the repository at this point in the history
Add appropriate runtime errors for incorrect types and null values for public API methods and class setters in application, ensemble, experiment, and job. 

 [ committed by @juliaputko ]
 [ reviewed by @amandarichardsonn, @MattToast , @mellis13 ]
  • Loading branch information
juliaputko authored Oct 7, 2024
1 parent dbf7b72 commit 2cbd3be
Show file tree
Hide file tree
Showing 9 changed files with 690 additions and 30 deletions.
10 changes: 10 additions & 0 deletions smartsim/_core/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,18 @@ def unpack(value: _NestedJobSequenceType) -> t.Generator[Job, None, None]:
:param value: Sequence containing elements of type Job or other
sequences that are also of type _NestedJobSequenceType
:return: flattened list of Jobs"""
from smartsim.launchable.job import Job

for item in value:

if isinstance(item, t.Iterable):
# string are iterable of string. Avoid infinite recursion
if isinstance(item, str):
raise TypeError("jobs argument was not of type Job")
yield from unpack(item)
else:
if not isinstance(item, Job):
raise TypeError("jobs argument was not of type Job")
yield item


Expand Down Expand Up @@ -157,10 +164,13 @@ def expand_exe_path(exe: str) -> str:
"""Takes an executable and returns the full path to that executable
:param exe: executable or file
:raises ValueError: if no executable is provided
:raises TypeError: if file is not an executable
:raises FileNotFoundError: if executable cannot be found
"""

if not exe:
raise ValueError("No executable provided")
# which returns none if not found
in_path = which(exe)
if not in_path:
Expand Down
90 changes: 87 additions & 3 deletions smartsim/builders/ensemble.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

from __future__ import annotations

import collections
import copy
import itertools
import os
Expand All @@ -38,6 +39,7 @@
from smartsim.entity.application import Application
from smartsim.entity.files import EntityFiles
from smartsim.launchable.job import Job
from smartsim.settings.launch_settings import LaunchSettings

if t.TYPE_CHECKING:
from smartsim.settings.launch_settings import LaunchSettings
Expand Down Expand Up @@ -137,7 +139,7 @@ def __init__(
copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {}
)
"""The parameters and values to be used when configuring entities"""
self._files = copy.deepcopy(files) if files else None
self._files = copy.deepcopy(files) if files else EntityFiles()
"""The files to be copied, symlinked, and/or configured prior to execution"""
self._file_parameters = (
copy.deepcopy(file_parameters) if file_parameters else {}
Expand All @@ -163,7 +165,11 @@ def exe(self, value: str | os.PathLike[str]) -> None:
"""Set the executable.
:param value: the executable
:raises TypeError: if the exe argument is not str or PathLike str
"""
if not isinstance(value, (str, os.PathLike)):
raise TypeError("exe argument was not of type str or PathLike str")

self._exe = os.fspath(value)

@property
Expand All @@ -179,7 +185,15 @@ def exe_args(self, value: t.Sequence[str]) -> None:
"""Set the executable arguments.
:param value: the executable arguments
:raises TypeError: if exe_args is not sequence of str
"""

if not (
isinstance(value, collections.abc.Sequence)
and (all(isinstance(x, str) for x in value))
):
raise TypeError("exe_args argument was not of type sequence of str")

self._exe_args = list(value)

@property
Expand All @@ -197,11 +211,36 @@ def exe_arg_parameters(
"""Set the executable argument parameters.
:param value: the executable argument parameters
:raises TypeError: if exe_arg_parameters is not mapping
of str and sequences of sequences of strings
"""

if not (
isinstance(value, collections.abc.Mapping)
and (
all(
isinstance(key, str)
and isinstance(val, collections.abc.Sequence)
and all(
isinstance(subval, collections.abc.Sequence) for subval in val
)
and all(
isinstance(item, str)
for item in itertools.chain.from_iterable(val)
)
for key, val in value.items()
)
)
):
raise TypeError(
"exe_arg_parameters argument was not of type "
"mapping of str and sequences of sequences of strings"
)

self._exe_arg_parameters = copy.deepcopy(value)

@property
def files(self) -> t.Union[EntityFiles, None]:
def files(self) -> EntityFiles:
"""Return attached EntityFiles object.
:return: the EntityFiles object of files to be copied, symlinked,
Expand All @@ -210,12 +249,16 @@ def files(self) -> t.Union[EntityFiles, None]:
return self._files

@files.setter
def files(self, value: t.Optional[EntityFiles]) -> None:
def files(self, value: EntityFiles) -> None:
"""Set the EntityFiles object.
:param value: the EntityFiles object of files to be copied, symlinked,
and/or configured prior to execution
:raises TypeError: if files is not of type EntityFiles
"""

if not isinstance(value, EntityFiles):
raise TypeError("files argument was not of type EntityFiles")
self._files = copy.deepcopy(value)

@property
Expand All @@ -231,7 +274,26 @@ def file_parameters(self, value: t.Mapping[str, t.Sequence[str]]) -> None:
"""Set the file parameters.
:param value: the file parameters
:raises TypeError: if file_parameters is not a mapping of str and
sequence of str
"""

if not (
isinstance(value, t.Mapping)
and (
all(
isinstance(key, str)
and isinstance(val, collections.abc.Sequence)
and all(isinstance(subval, str) for subval in val)
for key, val in value.items()
)
)
):
raise TypeError(
"file_parameters argument was not of type mapping of str "
"and sequence of str"
)

self._file_parameters = dict(value)

@property
Expand All @@ -249,7 +311,15 @@ def permutation_strategy(
"""Set the permutation strategy
:param value: the permutation strategy
:raises TypeError: if permutation_strategy is not str or
PermutationStrategyType
"""

if not (callable(value) or isinstance(value, str)):
raise TypeError(
"permutation_strategy argument was not of "
"type str or PermutationStrategyType"
)
self._permutation_strategy = value

@property
Expand All @@ -265,7 +335,11 @@ def max_permutations(self, value: int) -> None:
"""Set the maximum permutations
:param value: the max permutations
:raises TypeError: max_permutations argument was not of type int
"""
if not isinstance(value, int):
raise TypeError("max_permutations argument was not of type int")

self._max_permutations = value

@property
Expand All @@ -281,7 +355,13 @@ def replicas(self, value: int) -> None:
"""Set the number of replicas.
:return: the number of replicas
:raises TypeError: replicas argument was not of type int
"""
if not isinstance(value, int):
raise TypeError("replicas argument was not of type int")
if value <= 0:
raise ValueError("Number of replicas must be a positive integer")

self._replicas = value

def _create_applications(self) -> tuple[Application, ...]:
Expand Down Expand Up @@ -342,7 +422,11 @@ def build_jobs(self, settings: LaunchSettings) -> tuple[Job, ...]:
:param settings: LaunchSettings to apply to each Job
:return: Sequence of Jobs with the provided LaunchSettings
:raises TypeError: if the ids argument is not type LaunchSettings
:raises ValueError: if the LaunchSettings provided are empty
"""
if not isinstance(settings, LaunchSettings):
raise TypeError("ids argument was not of type LaunchSettings")
apps = self._create_applications()
if not apps:
raise ValueError("There are no members as part of this ensemble")
Expand Down
45 changes: 40 additions & 5 deletions smartsim/entity/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def __init__(
"""The executable to run"""
self._exe_args = self._build_exe_args(exe_args) or []
"""The executable arguments"""
self._files = copy.deepcopy(files) if files else None
self._files = copy.deepcopy(files) if files else EntityFiles()
"""Files to be copied, symlinked, and/or configured prior to execution"""
self._file_parameters = (
copy.deepcopy(file_parameters) if file_parameters else {}
Expand All @@ -112,8 +112,16 @@ def exe(self, value: str) -> None:
"""Set the executable.
:param value: the executable
:raises TypeError: exe argument is not int
"""
self._exe = copy.deepcopy(value)
if not isinstance(value, str):
raise TypeError("exe argument was not of type str")

if value == "":
raise ValueError("exe cannot be an empty str")

self._exe = value

@property
def exe_args(self) -> t.MutableSequence[str]:
Expand Down Expand Up @@ -149,12 +157,18 @@ def files(self) -> t.Union[EntityFiles, None]:
return self._files

@files.setter
def files(self, value: t.Optional[EntityFiles]) -> None:
def files(self, value: EntityFiles) -> None:
"""Set the EntityFiles object.
:param value: the EntityFiles object of files to be copied, symlinked,
and/or configured prior to execution
:raises TypeError: files argument was not of type int
"""

if not isinstance(value, EntityFiles):
raise TypeError("files argument was not of type EntityFiles")

self._files = copy.deepcopy(value)

@property
Expand All @@ -170,7 +184,18 @@ def file_parameters(self, value: t.Mapping[str, str]) -> None:
"""Set the file parameters.
:param value: the file parameters
:raises TypeError: file_parameters argument is not a mapping of str and str
"""
if not (
isinstance(value, t.Mapping)
and all(
isinstance(key, str) and isinstance(val, str)
for key, val in value.items()
)
):
raise TypeError(
"file_parameters argument was not of type mapping of str and str"
)
self._file_parameters = copy.deepcopy(value)

@property
Expand All @@ -186,7 +211,15 @@ def incoming_entities(self, value: t.List[SmartSimEntity]) -> None:
"""Set the incoming entities.
:param value: incoming entities
:raises TypeError: incoming_entities argument is not a list of SmartSimEntity
"""
if not isinstance(value, list) or not all(
isinstance(x, SmartSimEntity) for x in value
):
raise TypeError(
"incoming_entities argument was not of type list of SmartSimEntity"
)

self._incoming_entities = copy.copy(value)

@property
Expand All @@ -202,7 +235,11 @@ def key_prefixing_enabled(self, value: bool) -> None:
"""Set whether key prefixing is enabled for the application.
:param value: key prefixing enabled
:raises TypeError: key prefixings enabled argument was not of type bool
"""
if not isinstance(value, bool):
raise TypeError("key_prefixing_enabled argument was not of type bool")

self.key_prefixing_enabled = copy.deepcopy(value)

def as_executable_sequence(self) -> t.Sequence[str]:
Expand Down Expand Up @@ -264,8 +301,6 @@ def attached_files_table(self) -> str:
:return: String version of table
"""
if not self.files:
return "No file attached to this application."
return str(self.files)

@staticmethod
Expand Down
Loading

0 comments on commit 2cbd3be

Please sign in to comment.