Skip to content

Commit

Permalink
Remove init_default function (CrayLabs#545)
Browse files Browse the repository at this point in the history
This PR removes the helper function `init_default` and instead
implements traditional type narrowing.

[ reviewed by @MattToast ]
[ committed by @amandarichardsonn ]
  • Loading branch information
amandarichardsonn authored and ankona committed May 6, 2024
1 parent 0d39f31 commit a81c0ef
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 36 deletions.
4 changes: 4 additions & 0 deletions doc/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ To be released at some future point in time
Description

- Fix race condition for telemetry monitor
- Remove helper function ``init_default``
- Fix telemetry monitor logging errrors for task history
- Change default path for entities
- Drop Python 3.8 support
Expand All @@ -44,6 +45,8 @@ Description
Detailed Notes

- Ensure the telemetry monitor is started prior to launching entities (SmartSim-PR549_)
- Remove helper function ``init_default`` and replace with traditional type
narrowing. (SmartSim-PR545_)
- Ensure the telemetry monitor does not track a task_id
for a managed task. (SmartSim-PR557_)
- The default path for an entity is now the path to the experiment / the
Expand Down Expand Up @@ -110,6 +113,7 @@ Detailed Notes
the previously registered signal handler. (SmartSim-PR535_)

.. _SmartSim-PR549: https://github.com/CrayLabs/SmartSim/pull/549
.. _SmartSim-PR545: https://github.com/CrayLabs/SmartSim/pull/545
.. _SmartSim-PR557: https://github.com/CrayLabs/SmartSim/pull/557
.. _SmartSim-PR533: https://github.com/CrayLabs/SmartSim/pull/533
.. _SmartSim-PR544: https://github.com/CrayLabs/SmartSim/pull/544
Expand Down
2 changes: 1 addition & 1 deletion smartsim/_core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

from .helpers import colorize, delete_elements, init_default, installed_redisai_backends
from .helpers import colorize, delete_elements, installed_redisai_backends
from .redis import check_cluster_status, create_cluster, db_is_active
12 changes: 0 additions & 12 deletions smartsim/_core/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,18 +116,6 @@ def get_base_36_repr(positive_int: int) -> str:
return "".join(reversed(result))


def init_default(
default: t.Any,
init_value: t.Any,
expected_type: t.Union[t.Type[t.Any], t.Tuple[t.Type[t.Any], ...], None] = None,
) -> t.Any:
if init_value is None:
return default
if expected_type is not None and not isinstance(init_value, expected_type):
raise TypeError(f"Argument was of type {type(init_value)}, not {expected_type}")
return init_value


def expand_exe_path(exe: str) -> str:
"""Takes an executable and returns the full path to that executable
Expand Down
4 changes: 0 additions & 4 deletions smartsim/entity/dbobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
from pathlib import Path

from .._core._install.builder import Device
from .._core.utils import init_default
from ..error import SSUnsupportedError

__all__ = ["DBObject", "DBModel", "DBScript"]
Expand Down Expand Up @@ -76,9 +75,6 @@ def _check_tensor_args(
inputs: t.Union[str, t.Optional[t.List[str]]],
outputs: t.Union[str, t.Optional[t.List[str]]],
) -> t.Tuple[t.List[str], t.List[str]]:
inputs = init_default([], inputs, (list, str))
outputs = init_default([], outputs, (list, str))

if isinstance(inputs, str):
inputs = [inputs]
if isinstance(outputs, str):
Expand Down
9 changes: 4 additions & 5 deletions smartsim/entity/ensemble.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
from tabulate import tabulate

from .._core._install.builder import Device
from .._core.utils.helpers import init_default
from ..error import (
EntityExistsError,
SmartSimError,
Expand Down Expand Up @@ -98,11 +97,11 @@ def __init__(
:return: ``Ensemble`` instance
:rtype: ``Ensemble``
"""
self.params = init_default({}, params, dict)
self.params_as_args = init_default({}, params_as_args, (list, str))
self.params = params or {}
self.params_as_args = params_as_args or []
self._key_prefixing_enabled = True
self.batch_settings = init_default({}, batch_settings, BatchSettings)
self.run_settings = init_default({}, run_settings, RunSettings)
self.batch_settings = batch_settings
self.run_settings = run_settings

super().__init__(name, str(path), perm_strat=perm_strat, **kwargs)

Expand Down
8 changes: 4 additions & 4 deletions smartsim/entity/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from os import path as osp

from .._core._install.builder import Device
from .._core.utils.helpers import cat_arg_and_value, init_default
from .._core.utils.helpers import cat_arg_and_value
from ..error import EntityExistsError, SSUnsupportedError
from ..log import get_logger
from ..settings.base import BatchSettings, RunSettings
Expand Down Expand Up @@ -164,9 +164,9 @@ def attach_generator_files(
:param to_configure: input files with tagged parameters, defaults to []
:type to_configure: list, optional
"""
to_copy = init_default([], to_copy, (list, str))
to_symlink = init_default([], to_symlink, (list, str))
to_configure = init_default([], to_configure, (list, str))
to_copy = to_copy or []
to_symlink = to_symlink or []
to_configure = to_configure or []

# Check that no file collides with the parameter file written
# by Generator. We check the basename, even though it is more
Expand Down
19 changes: 11 additions & 8 deletions smartsim/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
from smartsim.status import SmartSimStatus

from ._core import Controller, Generator, Manifest
from ._core.utils import init_default
from .database import Orchestrator
from .entity import (
Ensemble,
Expand Down Expand Up @@ -160,7 +159,10 @@ def __init__(
if not osp.isdir(osp.abspath(exp_path)):
raise NotADirectoryError("Experiment path provided does not exist")
exp_path = osp.abspath(exp_path)
self.exp_path: str = init_default(osp.join(getcwd(), name), exp_path, str)
else:
exp_path = osp.join(getcwd(), name)

self.exp_path = exp_path

self._launcher = launcher.lower()

Expand Down Expand Up @@ -497,8 +499,9 @@ def create_ensemble(
"""
if name is None:
raise AttributeError("Entity has no name. Please set name attribute.")
path = path or osp.join(self.exp_path, name)
entity_path: str = osp.abspath(path)
check_path = path or osp.join(self.exp_path, name)
entity_path: str = osp.abspath(check_path)

try:
new_ensemble = Ensemble(
name=name,
Expand Down Expand Up @@ -613,8 +616,8 @@ def create_model(
"""
if name is None:
raise AttributeError("Entity has no name. Please set name attribute.")
path = path or osp.join(self.exp_path, name)
entity_path: str = osp.abspath(path)
check_path = path or osp.join(self.exp_path, name)
entity_path: str = osp.abspath(check_path)
if params is None:
params = {}

Expand Down Expand Up @@ -827,8 +830,8 @@ def create_database(
"""

self.append_to_db_identifier_list(db_identifier)
path = path or osp.join(self.exp_path, db_identifier)
entity_path: str = osp.abspath(path)
check_path = path or osp.join(self.exp_path, db_identifier)
entity_path: str = osp.abspath(check_path)
return Orchestrator(
port=port,
path=entity_path,
Expand Down
3 changes: 1 addition & 2 deletions smartsim/wlm/slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
from .._core.launcher.slurm.slurmCommands import salloc, scancel, scontrol, sinfo
from .._core.launcher.slurm.slurmParser import parse_salloc, parse_salloc_error
from .._core.launcher.util.launcherUtil import ComputeNode, Partition
from .._core.utils.helpers import init_default
from ..error import (
AllocationError,
LauncherError,
Expand Down Expand Up @@ -84,7 +83,7 @@ def get_allocation(
"Attempted slurm function without access to slurm(salloc) at the call site"
)

options = init_default({}, options, dict)
options = options or {}

salloc_args = _get_alloc_cmd(nodes, time, account, options=options)
debug_msg = " ".join(salloc_args[1:])
Expand Down

0 comments on commit a81c0ef

Please sign in to comment.