Skip to content

Commit

Permalink
tests passing, matt comments addressed, styling
Browse files Browse the repository at this point in the history
  • Loading branch information
amandarichardsonn committed Aug 7, 2024
1 parent f90a285 commit 907a1d0
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 75 deletions.
4 changes: 0 additions & 4 deletions smartsim/_core/entrypoints/file_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,11 @@ def copy(parsed_args: argparse.Namespace) -> None:
FileExistsError will be raised
"""
if os.path.isdir(parsed_args.source):
print("here")
print(parsed_args.source)
print(parsed_args.dest)
shutil.copytree(
parsed_args.source,
parsed_args.dest,
dirs_exist_ok=parsed_args.dirs_exist_ok,
)
print(os.listdir(parsed_args.dest))
else:
shutil.copy(parsed_args.source, parsed_args.dest)

Expand Down
32 changes: 20 additions & 12 deletions smartsim/_core/generation/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class Generator:
files into the Job directory.
"""

def __init__(self, root: str | os.PathLike[str]) -> None:
def __init__(self, root: os.PathLike[str]) -> None:
"""Initialize a generator object
TODO The Generator class is responsible for creating Job directories.
Expand All @@ -62,18 +62,19 @@ def __init__(self, root: str | os.PathLike[str]) -> None:
self.root = root
"""The root path under which to generate files"""

def log_file(self, log_path: pathlib.Path) -> str:
def log_file(self, log_path: os.PathLike[str]) -> os.PathLike[str]:
"""Returns the location of the file
summarizing the parameters used for the generation
of the entity.
:param log_path: Path to log directory
:returns: Path to file with parameter settings
"""
return join(log_path, "smartsim_params.txt")
return pathlib.Path(log_path) / "smartsim_params.txt"


def generate_job(self, job: Job, job_path: str, log_path: str):
def generate_job(
self, job: Job, job_path: os.PathLike[str], log_path: os.PathLike[str]
) -> None:
"""Write and configure input files for a Job.
To have files or directories present in the created Job
Expand All @@ -85,7 +86,7 @@ def generate_job(self, job: Job, job_path: str, log_path: str):
specified with a tag within the input file itself.
The default tag is surronding an input value with semicolons.
e.g. ``THERMO=;90;``
:param job: The job instance to write and configure files for.
:param job_path: The path to the \"run\" directory for the job instance.
:param log_path: The path to the \"log\" directory for the job instance.
Expand All @@ -99,8 +100,7 @@ def generate_job(self, job: Job, job_path: str, log_path: str):
# Perform file system operations on attached files
self._build_operations(job, job_path)


def _build_operations(self, job: Job, job_path: pathlib.Path) -> None:
def _build_operations(self, job: Job, job_path: os.PathLike[str]) -> None:
"""This method orchestrates file system ops for the attached SmartSim entity.
It processes three types of file system ops: to_copy, to_symlink, and to_configure.
For each type, it calls the corresponding private methods that open a subprocess
Expand All @@ -115,7 +115,7 @@ def _build_operations(self, job: Job, job_path: pathlib.Path) -> None:
self._write_tagged_files(app.files, app.params, job_path)

@staticmethod
def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None:
def _copy_files(files: t.Union[EntityFiles, None], dest: os.PathLike[str]) -> None:
"""Perform copy file sys operations on a list of files.
:param app: The Application attached to the Job
Expand All @@ -126,14 +126,16 @@ def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None:
return
for src in files.copy:
if os.path.isdir(src):
base_source_name = os.path.basename(src)
new_dst_path = os.path.join(dest, base_source_name)
subprocess.run(
args=[
sys.executable,
"-m",
"smartsim._core.entrypoints.file_operations",
"copy",
src,
dest,
new_dst_path,
"--dirs_exist_ok",
]
)
Expand All @@ -150,7 +152,9 @@ def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None:
)

@staticmethod
def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None:
def _symlink_files(
files: t.Union[EntityFiles, None], dest: os.PathLike[str]
) -> None:
"""Perform symlink file sys operations on a list of files.
:param app: The Application attached to the Job
Expand Down Expand Up @@ -178,7 +182,11 @@ def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> Non
)

@staticmethod
def _write_tagged_files(files: t.Union[EntityFiles, None], params: t.Mapping[str, str], dest: pathlib.Path) -> None:
def _write_tagged_files(
files: t.Union[EntityFiles, None],
params: t.Mapping[str, str],
dest: os.PathLike[str],
) -> None:
"""Read, configure and write the tagged input files for
a Job instance. This function specifically deals with the tagged
files attached to an entity.
Expand Down
5 changes: 2 additions & 3 deletions smartsim/_core/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,18 @@
_T = t.TypeVar("_T")
_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).
:raises ValueError: If the name is an empty string.
"""
if os.path.sep in name:
raise ValueError("Invalid input: String contains the path separator.")
if name == "":
raise ValueError("Invalid input: Name cannot be an empty string.")


def unpack_fs_identifier(fs_id: str, token: str) -> t.Tuple[str, str]:
"""Unpack the unformatted feature store identifier
Expand Down
28 changes: 17 additions & 11 deletions smartsim/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,7 @@ def start(self, *jobs: Job) -> tuple[LaunchedJobID, ...]:
run_id = datetime.datetime.now().replace(microsecond=0).isoformat()
root = pathlib.Path(self.exp_path, run_id)
"""Create the run id for Experiment.start"""
return self._dispatch(
Generator(root), dispatch.DEFAULT_DISPATCHER, *jobs
)
return self._dispatch(Generator(root), dispatch.DEFAULT_DISPATCHER, *jobs)

def _dispatch(
self,
Expand Down Expand Up @@ -245,7 +243,9 @@ def execute_dispatch(generator: Generator, job: Job, idx: int) -> LaunchedJobID:
)

@_contextualize
def _generate(self, generator: Generator, job: Job, job_index: int) -> pathlib.Path:
def _generate(
self, generator: Generator, job: Job, job_index: int
) -> os.PathLike[str]:
"""Generate the directory and file structure for a ``Job``
``Experiment._generate`` calls the appropriate Generator
Expand Down Expand Up @@ -276,19 +276,23 @@ def _generate(self, generator: Generator, job: Job, job_index: int) -> pathlib.P
logger.error(e)
raise

def _generate_job_root(self, job: Job, job_index: int, root: str) -> pathlib.Path:
def _generate_job_root(
self, job: Job, job_index: int, root: os.PathLike[str]
) -> pathlib.Path:
"""Generates the root directory for a specific job instance.
:param job: The Job instance for which the root directory is generated.
:param job_index: The index of the Job instance (used for naming).
:returns: The path to the root directory for the Job instance.
"""
job_type = f"{job.__class__.__name__.lower()}s"
job_path = root / f"{job_type}/{job.name}-{job_index}"
job_path = pathlib.Path(root) / f"{job_type}/{job.name}-{job_index}"
job_path.mkdir(exist_ok=True, parents=True)
return job_path
return pathlib.Path(job_path)

def _generate_job_path(self, job: Job, job_index: int, root: str) -> pathlib.Path:
def _generate_job_path(
self, job: Job, job_index: int, root: os.PathLike[str]
) -> os.PathLike[str]:
"""Generates the path for the \"run\" directory within the root directory
of a specific job instance.
Expand All @@ -298,9 +302,11 @@ def _generate_job_path(self, job: Job, job_index: int, root: str) -> pathlib.Pat
"""
path = self._generate_job_root(job, job_index, root) / "run"
path.mkdir(exist_ok=False, parents=True)
return path
return pathlib.Path(path)

def _generate_log_path(self, job: Job, job_index: int, root: str) -> pathlib.Path:
def _generate_log_path(
self, job: Job, job_index: int, root: os.PathLike[str]
) -> os.PathLike[str]:
"""
Generates the path for the \"log\" directory within the root directory of a specific job instance.
Expand All @@ -310,7 +316,7 @@ def _generate_log_path(self, job: Job, job_index: int, root: str) -> pathlib.Pat
"""
path = self._generate_job_root(job, job_index, root) / "log"
path.mkdir(exist_ok=False, parents=True)
return path
return pathlib.Path(path)

def preview(
self,
Expand Down
14 changes: 7 additions & 7 deletions smartsim/launchable/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@

from __future__ import annotations

import typing as t
import os
import typing as t
from copy import deepcopy

from smartsim._core.commands.launchCommands import LaunchCommands
from smartsim._core.utils.helpers import check_name
from smartsim.launchable.basejob import BaseJob
from smartsim.settings import LaunchSettings
from smartsim._core.utils.helpers import check_name

if t.TYPE_CHECKING:
from smartsim.entity.entity import SmartSimEntity
Expand All @@ -52,24 +52,24 @@ def __init__(
self,
entity: SmartSimEntity,
launch_settings: LaunchSettings,
name: str | None = "job",
name: str | None = None,
):
super().__init__()
self._entity = deepcopy(entity)
self._launch_settings = deepcopy(launch_settings)
check_name(name)
self._name = name if name else entity.name
check_name(self._name)

@property
def name(self) -> str:
"""Retrieves the name of the Job."""
return self._name

@name.setter
def name(self, name: str) -> None:
def name(self, name: str | None) -> None:
"""Sets the name of the Job."""
check_name(name)
self._entity = name
self._name = name if name else self._entity.name
check_name(self._name)

@property
def entity(self) -> SmartSimEntity:
Expand Down
9 changes: 4 additions & 5 deletions smartsim/launchable/jobGroup.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
import typing as t
from copy import deepcopy

from .._core.utils.helpers import check_name
from .basejob import BaseJob
from .baseJobGroup import BaseJobGroup

from .._core.utils.helpers import check_name

if t.TYPE_CHECKING:
from typing_extensions import Self

Expand All @@ -25,19 +24,19 @@ def __init__(
) -> None:
super().__init__()
self._jobs = deepcopy(jobs)
check_name(name)
self._name = name
check_name(self._name)

@property
def name(self) -> str:
"""Retrieves the name of the JobGroup."""
return self._name

@name.setter
def name(self, name: str) -> None:
"""Sets the name of the JobGroup."""
check_name(name)
self._entity = name
self._name = name

@property
def jobs(self) -> t.List[BaseJob]:
Expand Down
12 changes: 9 additions & 3 deletions tests/temp_tests/test_jobGroup.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ def get_launch_steps(self):

def test_invalid_job_name(wlmutils):
job_1 = Job(app_1, wlmutils.get_test_launcher())
job_2 = Job(app_2,wlmutils.get_test_launcher())
with pytest.raises(ValueError):
_ = JobGroup([job_1, job_2], name="")
job_2 = Job(app_2, wlmutils.get_test_launcher())
with pytest.raises(ValueError):
_ = JobGroup([job_1, job_2], name="name/not/allowed")

Expand All @@ -59,6 +57,14 @@ def test_create_JobGroup():
assert len(job_group) == 1


def test_name_setter(wlmutils):
job_1 = Job(app_1, wlmutils.get_test_launcher())
job_2 = Job(app_2, wlmutils.get_test_launcher())
job_group = JobGroup([job_1, job_2])
job_group.name = "new_name"
assert job_group.name == "new_name"


def test_getitem_JobGroup(wlmutils):
job_1 = Job(app_1, wlmutils.get_test_launcher())
job_2 = Job(app_2, wlmutils.get_test_launcher())
Expand Down
20 changes: 16 additions & 4 deletions tests/temp_tests/test_launchable.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,19 @@ def test_launchable_init():
launchable = Launchable()
assert isinstance(launchable, Launchable)


def test_invalid_job_name(wlmutils):
entity = Application(
"test_name",
run_settings=LaunchSettings(wlmutils.get_test_launcher()),
run_settings="RunSettings",
exe="echo",
exe_args=["spam", "eggs"],
)
) # Mock RunSettings
settings = LaunchSettings(wlmutils.get_test_launcher())
with pytest.raises(ValueError):
_ = Job(entity, settings, name="")
with pytest.raises(ValueError):
_ = Job(entity, settings, name="path/to/name")


def test_job_init():
entity = Application(
"test_name",
Expand All @@ -77,6 +77,18 @@ def test_job_init():
assert "eggs" in job.entity.exe_args


def test_name_setter():
entity = Application(
"test_name",
run_settings=LaunchSettings("slurm"),
exe="echo",
exe_args=["spam", "eggs"],
)
job = Job(entity, LaunchSettings("slurm"))
job.name = "new_name"
assert job.name == "new_name"


def test_job_init_deepcopy():
entity = Application(
"test_name",
Expand Down
Loading

0 comments on commit 907a1d0

Please sign in to comment.