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

Refactor of Generation class and inject path into Launch process #650

Merged
Merged
Show file tree
Hide file tree
Changes from 70 commits
Commits
Show all changes
71 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
9a97620
fixes issues
amandarichardsonn Jul 30, 2024
8c86007
tests passing
amandarichardsonn Jul 30, 2024
e991b54
mypy errors and failing tests
amandarichardsonn Jul 31, 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
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
0294d71
failing
amandarichardsonn Aug 13, 2024
4c101e1
does not pass, adding matts comments
amandarichardsonn Aug 13, 2024
375ad36
pushing after meeting with matt
amandarichardsonn Aug 13, 2024
bbee824
Merge remote-tracking branch 'upstream/smartsim-refactor' into path-i…
amandarichardsonn Aug 13, 2024
ffaa225
passing all tests, review ready
amandarichardsonn Aug 13, 2024
ff2eaae
styling and all tests passing
amandarichardsonn Aug 13, 2024
7c3c546
tests passing ready for merge
amandarichardsonn Aug 13, 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
2 changes: 1 addition & 1 deletion smartsim/_core/entrypoints/file_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def copy(parsed_args: argparse.Namespace) -> None:
dirs_exist_ok=parsed_args.dirs_exist_ok,
)
else:
shutil.copyfile(parsed_args.source, parsed_args.dest)
shutil.copy(parsed_args.source, parsed_args.dest)


def symlink(parsed_args: argparse.Namespace) -> None:
Expand Down
481 changes: 237 additions & 244 deletions smartsim/_core/generation/generator.py

Large diffs are not rendered by default.

158 changes: 0 additions & 158 deletions smartsim/_core/generation/modelwriter.py

This file was deleted.

4 changes: 2 additions & 2 deletions smartsim/_core/launcher/dragon/dragonLauncher.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ def _assert_schema_type(obj: object, typ: t.Type[_SchemaT], /) -> _SchemaT:
def _as_run_request_args_and_policy(
run_req_args: DragonLaunchArguments,
exe: ExecutableProtocol,
path: str | os.PathLike[str],
env: t.Mapping[str, str | None],
) -> tuple[DragonRunRequestView, DragonRunPolicy]:
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -384,8 +385,7 @@ def _as_run_request_args_and_policy(
# this will need to be injected by the user or by us to have
# the command execute next to any generated files. A similar
# problem exists for the other settings.
# TODO: Find a way to inject this path
path=os.getcwd(),
path=path,
env=env,
# TODO: Not sure how this info is injected
name=None,
Expand Down
12 changes: 12 additions & 0 deletions smartsim/_core/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@
_TSignalHandlerFn = t.Callable[[int, t.Optional["FrameType"]], object]


def check_name(name: str) -> None:
"""
Checks if the input name is valid.

:param name: The name to be checked.

:raises ValueError: If the name contains the path separator (os.path.sep).
"""
if os.path.sep in name:
raise ValueError("Invalid input: String contains the path separator.")


def unpack_fs_identifier(fs_id: str, token: str) -> t.Tuple[str, str]:
"""Unpack the unformatted feature store identifier
and format for env variable suffix using the token
Expand Down
2 changes: 1 addition & 1 deletion smartsim/entity/dbnode.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def __init__(
fs_identifier: str = "",
) -> None:
"""Initialize a feature store node within an feature store."""
super().__init__(name, path, run_settings)
super().__init__(name, run_settings)
self.exe = [exe] if run_settings.container else [expand_exe_path(exe)]
self.exe_args = exe_args or []
self.ports = ports
Expand Down
9 changes: 1 addition & 8 deletions smartsim/entity/ensemble.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ def __init__(
exe: str | os.PathLike[str],
exe_args: t.Sequence[str] | None = None,
exe_arg_parameters: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None = None,
path: str | os.PathLike[str] | None = None,
files: EntityFiles | None = None,
file_parameters: t.Mapping[str, t.Sequence[str]] | None = None,
permutation_strategy: str | strategies.PermutationStrategyType = "all_perm",
Expand All @@ -66,11 +65,6 @@ def __init__(
self.exe_arg_parameters = (
copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {}
)
self.path = os.fspath(path) if path is not None else os.getcwd()
# ^^^^^^^^^^^
# TODO: Copied from the original implementation, but I'm not sure that
# I like this default. Shouldn't it be something under an
# experiment directory? If so, how it injected??
self.files = copy.deepcopy(files) if files else EntityFiles()
self.file_parameters = dict(file_parameters) if file_parameters else {}
self.permutation_strategy = permutation_strategy
Expand All @@ -97,7 +91,6 @@ def _create_applications(self) -> tuple[Application, ...]:
# ^^^^^^^^^^^^^^^^^^^^^^^
# FIXME: remove this constructor arg! It should not exist!!
exe_args=self.exe_args,
path=os.path.join(self.path, self.name),
amandarichardsonn marked this conversation as resolved.
Show resolved Hide resolved
files=self.files,
params=permutation.params,
params_as_args=permutation.exe_args, # type: ignore[arg-type]
Expand All @@ -111,4 +104,4 @@ def as_jobs(self, settings: LaunchSettings) -> tuple[Job, ...]:
apps = self._create_applications()
if not apps:
raise ValueError("There are no members as part of this ensemble")
return tuple(Job(app, settings) for app in apps)
return tuple(Job(app, settings, app.name) for app in apps)
4 changes: 1 addition & 3 deletions smartsim/entity/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,17 @@ def _on_disable(self) -> None:


class SmartSimEntity:
def __init__(self, name: str, path: str, run_settings: "RunSettings") -> None:
def __init__(self, name: str, run_settings: "RunSettings") -> None:
"""Initialize a SmartSim entity.

Each entity must have a name, path, and
run_settings. All entities within SmartSim
share these attributes.

:param name: Name of the entity
:param path: path to output, error, and configuration files
"""
self.name = name
self.run_settings = run_settings
self.path = path

@property
def type(self) -> str:
Expand Down
5 changes: 1 addition & 4 deletions smartsim/entity/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ def __init__(
run_settings: "RunSettings",
params: t.Optional[t.Dict[str, str]] = None,
exe_args: t.Optional[t.List[str]] = None,
path: t.Optional[str] = getcwd(),
params_as_args: t.Optional[t.List[str]] = None,
batch_settings: t.Optional["BatchSettings"] = None,
files: t.Optional[EntityFiles] = None,
Expand All @@ -76,7 +75,6 @@ def __init__(
:param exe_args: executable arguments
:param params: application parameters for writing into configuration files or
to be passed as command line arguments to executable.
:param path: path to output, error, and configuration files
:param run_settings: launcher settings specified in the experiment
:param params_as_args: list of parameters which have to be
interpreted as command line arguments to
Expand All @@ -85,7 +83,7 @@ def __init__(
application as a batch job
:param files: Files to have available to the application
"""
super().__init__(name, str(path), run_settings)
super().__init__(name, run_settings)
self.exe = [expand_exe_path(exe)]
# self.exe = [exe] if run_settings.container else [expand_exe_path(exe)]
self.exe_args = exe_args or []
Expand Down Expand Up @@ -228,7 +226,6 @@ def attach_generator_files(
"`smartsim_params.txt` is a file automatically "
+ "generated by SmartSim and cannot be ovewritten."
)

self.files = EntityFiles(to_configure, to_copy, to_symlink)

@property
Expand Down
Loading
Loading