Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests for ShellLauncher & additional shell launch context #671

Merged
Merged
Show file tree
Hide file tree
Changes from 92 commits
Commits
Show all changes
96 commits
Select commit Hold shift + click to select a range
42f7ff1
Builders are generic, have a format method
MattToast Jun 17, 2024
f76ee4f
Impl slurm
MattToast Jun 18, 2024
3027721
Impl local
MattToast Jun 18, 2024
6266b75
Impl jsrun
MattToast Jun 18, 2024
bbb5152
Impl mpi{run,exec}, orterun
MattToast Jun 18, 2024
b96a160
Impl aprun
MattToast Jun 18, 2024
dc1cf0b
Impl dragon
MattToast Jun 20, 2024
e555342
Type errors supressed for now
MattToast Jun 20, 2024
9161125
Add a dispatcher class to send built settings to a launcher
MattToast Jun 22, 2024
d765718
Isort/Black
MattToast Jun 24, 2024
01393c7
Env: dict -> Mapping
MattToast Jun 26, 2024
e504ccf
Organize dispatch file, call out work to do
MattToast Jun 26, 2024
910b2d9
Wire up dispatch dragon builder to dragon launcher
MattToast Jun 26, 2024
f4ebada
Import sort
MattToast Jul 2, 2024
9d2901f
Remove old TODOs
MattToast Jul 2, 2024
c413b8f
textwrap.dedent fix
MattToast Jul 3, 2024
35e686c
Add doc strs
MattToast Jul 4, 2024
9b2c2b7
Make dispatching settings more concise
MattToast Jul 4, 2024
a1a7638
Merge remote-tracking branch 'upstream/smartsim-refactor' into builde…
MattToast Jul 9, 2024
219d481
Address reviewer feedback
MattToast Jul 9, 2024
44ba1eb
Merge remote-tracking branch 'upstream/smartsim-refactor' into builde…
MattToast Jul 10, 2024
9797442
Remove stale FIXME comment
MattToast Jul 16, 2024
2a667f2
Path injection and Generator class refactor
amandarichardsonn Jul 9, 2024
4e9f5b2
Dispatcher takes a format function with launcher like
MattToast Jul 18, 2024
0b9ec1a
Re-wrire up default dispatcher
MattToast Jul 18, 2024
6d5a3c4
Add tests for new dispatch API, make old tests pass
MattToast Jul 19, 2024
6b7943a
Upper pin typing extensions (thanks TF)
MattToast Jul 20, 2024
82ee19f
Make 3.9 compatable, lint, better names
MattToast Jul 20, 2024
36bcfbb
Address reviewer comments
MattToast Jul 17, 2024
0a1ebba
Add tests for `Experiment.start_jobs`
MattToast Jul 21, 2024
b729e91
Rename `Builder` -> `Arguments`
MattToast Jul 22, 2024
5955278
Merge branch 'builders-fmt' of https://github.com/MattToast/SmartSim …
amandarichardsonn Jul 23, 2024
26cb61d
test_generator passing
amandarichardsonn Jul 23, 2024
20d8a8e
settings tests passing and mypy errors resolved
amandarichardsonn Jul 23, 2024
9ac3762
doc string updates
amandarichardsonn Jul 24, 2024
24e42ef
experiment tests passing
amandarichardsonn Jul 24, 2024
fdf4b63
pushing one small change to pull
amandarichardsonn Jul 24, 2024
11d3c5a
Merge branch 'smartsim-refactor' of https://github.com/CrayLabs/Smart…
amandarichardsonn Jul 24, 2024
1378114
coverage for gen tests, config, copy and symlink
amandarichardsonn Jul 26, 2024
e4036aa
Tagged Files Heirarchy added
amandarichardsonn Jul 26, 2024
717e951
make style formatting and additional changes I forget
amandarichardsonn Jul 29, 2024
ba351b3
Merge branch 'smartsim-refactor' of https://github.com/CrayLabs/Smart…
amandarichardsonn Jul 29, 2024
d778862
path injection merge conflicts addressed
amandarichardsonn Jul 29, 2024
91f3af8
all tests passing and make style
amandarichardsonn Jul 29, 2024
c1ec227
mypy errors partially corrected
amandarichardsonn Jul 29, 2024
9370cb0
passing tests
amandarichardsonn Jul 30, 2024
023d51b
mark test
amandarichardsonn Jul 30, 2024
0df2aad
update popen arguments
amandarichardsonn Jul 30, 2024
9a97620
fixes issues
amandarichardsonn Jul 30, 2024
8c86007
tests passing
amandarichardsonn Jul 30, 2024
24a3671
Merge branch 'path-injection' of https://github.com/amandarichardsonn…
amandarichardsonn Jul 31, 2024
e991b54
mypy errors and failing tests
amandarichardsonn Jul 31, 2024
b7e95ea
Merge branch 'path-injection' of https://github.com/amandarichardsonn…
amandarichardsonn Jul 31, 2024
2170243
notes after meeting with chris
amandarichardsonn Aug 1, 2024
67f0307
Merge branch 'smartsim-refactor' of https://github.com/CrayLabs/Smart…
amandarichardsonn Aug 1, 2024
871d542
addressing half of matts comments
amandarichardsonn Aug 1, 2024
c8246c7
remove edits to the legacy gen test
amandarichardsonn Aug 1, 2024
0baddf2
adding new line to legacy gen test
amandarichardsonn Aug 1, 2024
0874767
subprocess added
amandarichardsonn Aug 1, 2024
12cedef
additional matt comments
amandarichardsonn Aug 2, 2024
5e731a1
addressing matts comments
amandarichardsonn Aug 2, 2024
80e2229
generator tests passing, doc strings updated
amandarichardsonn Aug 2, 2024
1f1fcdc
finished addressing matt comments
amandarichardsonn Aug 2, 2024
202f629
attempt to fix ML runtimes workflow
amandarichardsonn Aug 2, 2024
f9c9d56
type error in ML workflow
amandarichardsonn Aug 2, 2024
df26bd2
Merge branch 'path-injection' of https://github.com/amandarichardsonn…
amandarichardsonn Aug 4, 2024
43c82ee
tests not passing
amandarichardsonn Aug 5, 2024
f90a285
addressing all of matts comments besides 1
amandarichardsonn Aug 6, 2024
907a1d0
tests passing, matt comments addressed, styling
amandarichardsonn Aug 7, 2024
69c9f2d
styling
amandarichardsonn Aug 7, 2024
d90a761
Merge branch 'path-injection' into shell-context
amandarichardsonn Aug 7, 2024
98cb30d
push changes
amandarichardsonn Aug 12, 2024
dbe9ca7
pushing to switch branches - failing
amandarichardsonn Aug 13, 2024
9b152e0
pushing changes
amandarichardsonn Aug 13, 2024
082f917
pushing to switch branches, not passing
amandarichardsonn Aug 13, 2024
4d7fb6d
Merge remote-tracking branch 'upstream/smartsim-refactor' into shell-…
amandarichardsonn Aug 14, 2024
335758e
update to test suite, does not pass, pushing to save
amandarichardsonn Aug 15, 2024
5b28902
failing because a change in types
amandarichardsonn Aug 19, 2024
f370b83
slurm tests passing but work to be done on other settings
amandarichardsonn Aug 19, 2024
18b1455
all tests pass
amandarichardsonn Aug 20, 2024
2648258
all tests passing, mypy green, styling done
amandarichardsonn Aug 20, 2024
0c55223
usused import deletions
amandarichardsonn Aug 20, 2024
e7ec7d7
Merge remote-tracking branch 'upstream/smartsim-refactor' into shell-…
amandarichardsonn Aug 20, 2024
2de6943
marked test
amandarichardsonn Aug 20, 2024
d834ced
make style
amandarichardsonn Aug 20, 2024
f4822d7
skip when slurm not avail
amandarichardsonn Aug 20, 2024
ae913a1
address all of matts comments
amandarichardsonn Aug 23, 2024
d118565
Merge remote-tracking branch 'upstream/smartsim-refactor' into safety
amandarichardsonn Aug 23, 2024
098b828
Merge branch 'smartsim-refactor' into shell-context
amandarichardsonn Aug 23, 2024
20844d0
mypy updates, resolving | issues
amandarichardsonn Aug 27, 2024
e905874
styling
amandarichardsonn Aug 27, 2024
c114569
working on invalid char in file path when loading artifact
amandarichardsonn Aug 27, 2024
1a555aa
addressing matts comments + styling
amandarichardsonn Aug 27, 2024
4a44e33
updating shell tests
amandarichardsonn Aug 27, 2024
262084e
final comments, and fix duplicate num
amandarichardsonn Aug 27, 2024
aa00b09
remove the zip
amandarichardsonn Aug 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/run_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,19 @@ jobs:
echo "SMARTSIM_LOG_LEVEL=debug" >> $GITHUB_ENV
py.test -s --import-mode=importlib -o log_cli=true --cov=$(smart site) --cov-report=xml --cov-config=./tests/test_configs/cov/local_cov.cfg --ignore=tests/full_wlm/ -m ${{ matrix.subset }} ./tests

- name: try zip the test result folder
if: failure()
run: |
zip -r testResult.zip tests/test_output

# Upload artifacts on failure, ignoring binary files
- name: Upload Artifact
if: failure()
uses: actions/upload-artifact@v3
with:
name: test_artifact
path: |
tests/test_output
testResult.zip
!**/*.so
!**/*.pb
!**/*.pt
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ module = [
# FIXME: DO NOT MERGE THIS INTO DEVELOP BRANCH UNLESS THESE ARE PASSING OR
# REMOVED!!
"smartsim._core._cli.*",
"smartsim._core.commands.*",
"smartsim._core.control.controller",
"smartsim._core.control.manifest",
"smartsim._core.entrypoints.dragon_client",
Expand Down
52 changes: 35 additions & 17 deletions smartsim/_core/commands/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,41 +26,60 @@

import typing as t
from collections.abc import MutableSequence
from copy import deepcopy

from ...settings.launchCommand import LauncherType
from typing_extensions import Self


class Command(MutableSequence[str]):
"""Basic container for command information"""

def __init__(self, launcher: LauncherType, command: t.List[str]) -> None:
def __init__(self, command: t.List[str]) -> None:
"""Command constructor"""
self._launcher = launcher
self._command = command

@property
def launcher(self) -> LauncherType:
"""Get the launcher type.
Return a reference to the LauncherType.
"""
return self._launcher

@property
def command(self) -> t.List[str]:
"""Get the command list.
Return a reference to the command list.
"""
return self._command

def __getitem__(self, idx: int) -> str:
@t.overload
def __getitem__(self, idx: int) -> str: ...
@t.overload
def __getitem__(self, idx: slice) -> Self: ...
def __getitem__(self, idx: t.Union[int, slice]) -> t.Union[str, Self]:
"""Get the command at the specified index."""
return self._command[idx]
cmd = self._command[idx]
if isinstance(cmd, str):
return cmd
return type(self)(cmd)

def __setitem__(self, idx: int, value: str) -> None:
@t.overload
def __setitem__(self, idx: int, value: str) -> None: ...
@t.overload
def __setitem__(self, idx: slice, value: t.Iterable[str]) -> None: ...
def __setitem__(
self, idx: t.Union[int, slice], value: t.Union[str, t.Iterable[str]]
) -> None:
"""Set the command at the specified index."""
self._command[idx] = value
if isinstance(idx, int):
if not isinstance(value, str):
raise ValueError(
"Value must be of type `str` when assigning to an index"
)
self._command[idx] = deepcopy(value)
elif isinstance(idx, slice):
if not isinstance(value, list) or not all(
isinstance(item, str) for item in value
):
raise ValueError(
"Value must be a list of strings when assigning to a slice"
)
self._command[idx] = (deepcopy(val) for val in value)
amandarichardsonn marked this conversation as resolved.
Show resolved Hide resolved

def __delitem__(self, idx: int) -> None:
def __delitem__(self, idx: t.Union[int, slice]) -> None:
"""Delete the command at the specified index."""
del self._command[idx]

Expand All @@ -73,6 +92,5 @@ def insert(self, idx: int, value: str) -> None:
self._command.insert(idx, value)

def __str__(self) -> str: # pragma: no cover
string = f"\nLauncher: {self.launcher.value}\n"
string += f"Command: {' '.join(str(cmd) for cmd in self.command)}"
string = f"\nCommand: {' '.join(str(cmd) for cmd in self.command)}"
return string
41 changes: 36 additions & 5 deletions smartsim/_core/commands/commandList.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import typing as t
from collections.abc import MutableSequence
from copy import deepcopy

from .command import Command

Expand All @@ -46,15 +47,45 @@ def commands(self) -> t.List[Command]:
"""
return self._commands

def __getitem__(self, idx: int) -> Command:
@t.overload
def __getitem__(self, idx: int) -> Command: ...
@t.overload
def __getitem__(self, idx: slice) -> t.List[Command]: ...
def __getitem__(
self, idx: t.Union[slice, int]
) -> t.Union[Command, t.List[Command]]:
"""Get the Command at the specified index."""
return self._commands[idx]

def __setitem__(self, idx: int, value: Command) -> None:
"""Set the Command at the specified index."""
self._commands[idx] = value
@t.overload
def __setitem__(self, idx: int, value: Command) -> None: ...
@t.overload
def __setitem__(self, idx: slice, value: t.Iterable[Command]) -> None: ...
def __setitem__(
self, idx: t.Union[int, slice], value: t.Union[Command, t.Iterable[Command]]
) -> None:
"""Set the Commands at the specified index."""
if isinstance(idx, int):
if not isinstance(value, Command):
raise ValueError(
"Value must be of type `Command` when assigning to an index"
)
self._commands[idx] = deepcopy(value)
elif isinstance(idx, slice):
if not isinstance(value, list):
raise ValueError(
"Value must be a list of Commands when assigning to a slice"
)
for sublist in value:
if not isinstance(sublist.command, list) or not all(
isinstance(item, str) for item in sublist.command
):
raise ValueError(
"Value sublists must be a list of Commands when assigning to a slice"
)
self._commands[idx] = (deepcopy(val) for val in value)
amandarichardsonn marked this conversation as resolved.
Show resolved Hide resolved

def __delitem__(self, idx: int) -> None:
def __delitem__(self, idx: t.Union[int, slice]) -> None:
"""Delete the Command at the specified index."""
del self._commands[idx]

Expand Down
28 changes: 21 additions & 7 deletions smartsim/_core/dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import dataclasses
import os
import pathlib
import typing as t

from typing_extensions import Self, TypeAlias, TypeVarTuple, Unpack
Expand All @@ -42,10 +43,11 @@
from smartsim.experiment import Experiment
from smartsim.settings.arguments import LaunchArguments


_Ts = TypeVarTuple("_Ts")


_WorkingDirectory: TypeAlias = t.Union[str, os.PathLike[str]]
_WorkingDirectory: TypeAlias = pathlib.Path
"""A working directory represented as a string or PathLike object"""

_DispatchableT = t.TypeVar("_DispatchableT", bound="LaunchArguments")
Expand All @@ -62,15 +64,25 @@
a job
"""
_FormatterType: TypeAlias = t.Callable[
[_DispatchableT, "ExecutableProtocol", _WorkingDirectory, _EnvironMappingType],
[
_DispatchableT,
"ExecutableProtocol",
_WorkingDirectory,
_EnvironMappingType,
pathlib.Path,
pathlib.Path,
],
_LaunchableT,
]
"""A callable that is capable of formatting the components of a job into a type
capable of being launched by a launcher.
"""
_LaunchConfigType: TypeAlias = (
"_LauncherAdapter[ExecutableProtocol, _WorkingDirectory, _EnvironMappingType]"
)
_LaunchConfigType: TypeAlias = """_LauncherAdapter[
ExecutableProtocol,
_WorkingDirectory,
_EnvironMappingType,
pathlib.Path,
pathlib.Path]"""

"""A launcher adapater that has configured a launcher to launch the components
of a job with some pre-determined launch settings
Expand Down Expand Up @@ -260,10 +272,12 @@ def create_adapter_from_launcher(

def format_(
exe: ExecutableProtocol,
path: str | os.PathLike[str],
path: pathlib.Path,
env: _EnvironMappingType,
out: pathlib.Path,
err: pathlib.Path,
) -> _LaunchableT:
return self.formatter(arguments, exe, path, env)
return self.formatter(arguments, exe, path, env, out, err)

return _LauncherAdapter(launcher, format_)

Expand Down
20 changes: 18 additions & 2 deletions smartsim/_core/generation/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,16 @@ def _log_file(log_path: pathlib.Path) -> pathlib.Path:
"""
return pathlib.Path(log_path) / "smartsim_params.txt"

def generate_job(self, job: Job, job_index: int) -> pathlib.Path:
def _output_files(
self, log_path: pathlib.Path, job_name: str
) -> t.Tuple[pathlib.Path, pathlib.Path]:
out_file_path = log_path / (job_name + ".out")
err_file_path = log_path / (job_name + ".err")
amandarichardsonn marked this conversation as resolved.
Show resolved Hide resolved
return out_file_path, err_file_path
amandarichardsonn marked this conversation as resolved.
Show resolved Hide resolved

def generate_job(
self, job: Job, job_index: int
) -> t.Tuple[pathlib.Path, pathlib.Path, pathlib.Path]:
"""Write and configure input files for a Job.

To have files or directories present in the created Job
Expand Down Expand Up @@ -136,10 +145,17 @@ def generate_job(self, job: Job, job_index: int) -> pathlib.Path:
dt_string = datetime.now().strftime("%d/%m/%Y %H:%M:%S")
log_file.write(f"Generation start date and time: {dt_string}\n")

# Create output files
out_file, err_file = self._output_files(log_path, job.entity.name)
# Open and write to .out file
open(out_file, mode="w", encoding="utf-8")
# Open and write to .err file
open(err_file, mode="w", encoding="utf-8")
amandarichardsonn marked this conversation as resolved.
Show resolved Hide resolved

# Perform file system operations on attached files
self._build_operations(job, job_path)

return job_path
return job_path, out_file, err_file

@classmethod
def _build_operations(cls, job: Job, job_path: pathlib.Path) -> None:
Expand Down
7 changes: 5 additions & 2 deletions smartsim/_core/launcher/dragon/dragonLauncher.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from __future__ import annotations

import os
import pathlib
import typing as t

from smartsim._core.schemas.dragonRequests import DragonRunPolicy
Expand Down Expand Up @@ -371,6 +372,8 @@ def _as_run_request_args_and_policy(
exe: ExecutableProtocol,
path: str | os.PathLike[str],
env: t.Mapping[str, str | None],
stdout_path: pathlib.Path,
stderr_path: pathlib.Path,
) -> tuple[DragonRunRequestView, DragonRunPolicy]:
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
# FIXME: This type is 100% unacceptable, but I don't want to spend too much
Expand All @@ -392,8 +395,8 @@ def _as_run_request_args_and_policy(
env=env,
# TODO: Not sure how this info is injected
name=None,
output_file=None,
error_file=None,
output_file=stdout_path,
error_file=stderr_path,
**run_args,
),
policy,
Expand Down
Loading
Loading