Skip to content

refactor: refine shell and scheduler arguments #805

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions hpcflow/sdk/data/config_schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,9 @@ rules:
- path: [schedulers, { type: map_value }, defaults]
condition:
value.allowed_keys: # TODO: split these up based on scheduler type
- shebang_args
- options
- shebang_executable
- directives # TODO: only actually allowed in `QueuedScheduler` sub-classes
- options # deprecated, replaced by directives
- submit_cmd
- show_cmd
- del_cmd
Expand Down Expand Up @@ -211,6 +212,7 @@ rules:
condition:
value.allowed_keys:
- executable
- executable_args
- os_args
- WSL_executable
- WSL_distribution
Expand Down
7 changes: 3 additions & 4 deletions hpcflow/sdk/submission/jobscript.py
Original file line number Diff line number Diff line change
Expand Up @@ -1440,7 +1440,7 @@ def compose_jobscript(
scheduler_name: str | None = None,
scheduler_args: dict[str, Any] | None = None,
) -> str:
"""Prepare the jobscript file string."""
"""Prepare the jobscript file contents as a string."""
scheduler_name = scheduler_name or self.scheduler_name
assert scheduler_name
assert os_name
Expand All @@ -1465,15 +1465,14 @@ def compose_jobscript(
}

shebang = shell.JS_SHEBANG.format(
shebang_executable=" ".join(shell.shebang_executable),
shebang_args=scheduler.shebang_args,
shebang=" ".join(scheduler.shebang_executable or shell.shebang_executable)
)
header = shell.JS_HEADER.format(**header_args)

if isinstance(scheduler, QueuedScheduler):
header = shell.JS_SCHEDULER_HEADER.format(
shebang=shebang,
scheduler_options=scheduler.format_options(
scheduler_options=scheduler.format_directives(
resources=self.resources,
num_elements=self.blocks[0].num_elements, # only used for array jobs
is_array=self.is_array,
Expand Down
49 changes: 27 additions & 22 deletions hpcflow/sdk/submission/schedulers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import sys
import time
from typing import Generic, TypeVar, TYPE_CHECKING
import warnings
from typing_extensions import override
from hpcflow.sdk.typing import hydrate
from hpcflow.sdk.core.app_aware import AppAware
Expand Down Expand Up @@ -39,12 +40,9 @@ class Scheduler(ABC, Generic[JSRefType], AppAware):

Parameters
----------
shell_args: str
Arguments to pass to the shell. Pre-quoted.
shebang_args: str
Arguments to set on the shebang line. Pre-quoted.
options: dict
Options to the scheduler.
shebang_executable: list[str]
If specified, this will be used in the jobscript's shebang line instead of the
shell's `executable` and `executable_args` attributes.
"""

# This would be in the docstring except it renders really wrongly!
Expand All @@ -53,20 +51,8 @@ class Scheduler(ABC, Generic[JSRefType], AppAware):
# T
# The type of a jobscript reference.

#: Default value for arguments to the shell.
DEFAULT_SHELL_ARGS: ClassVar[str] = ""
#: Default value for arguments on the shebang line.
DEFAULT_SHEBANG_ARGS: ClassVar[str] = ""

def __init__(
self,
shell_args: str | None = None,
shebang_args: str | None = None,
options: dict | None = None,
):
self.shebang_args = shebang_args or self.DEFAULT_SHEBANG_ARGS
self.shell_args = shell_args or self.DEFAULT_SHELL_ARGS
self.options = options or {}
def __init__(self, shebang_executable: list[str] | None = None):
self.shebang_executable = shebang_executable

@property
def unique_properties(self) -> tuple[str, ...]:
Expand Down Expand Up @@ -168,6 +154,13 @@ class QueuedScheduler(Scheduler[str]):

Parameters
----------
directives: dict
Scheduler directives. Each item is written verbatim in the jobscript as a
scheduler directive, and is not processed in any way. If a value is `None`, the
key is considered a flag-like directive. If a value is a list, multiple directives
will be printed to the jobscript with the same key, but different values.
options: dict
Deprecated. Please use `directives` instead.
submit_cmd: str
The submission command, if overridden from default.
show_cmd: str
Expand Down Expand Up @@ -203,6 +196,8 @@ class QueuedScheduler(Scheduler[str]):

def __init__(
self,
directives: dict | None = None,
options: dict | None = None,
submit_cmd: str | None = None,
show_cmd: Sequence[str] | None = None,
del_cmd: str | None = None,
Expand All @@ -215,6 +210,16 @@ def __init__(
) -> None:
super().__init__(*args, **kwargs)

if options:
warnings.warn(
f"{self.__class__.__name__!r}: Please use `directives` instead of "
f"`options`, which will be removed in a future release.",
DeprecationWarning,
stacklevel=2,
)
directives = options

self.directives = directives or {}
self.submit_cmd: str = submit_cmd or self.DEFAULT_SUBMIT_CMD
self.show_cmd = show_cmd or self.DEFAULT_SHOW_CMD
self.del_cmd = del_cmd or self.DEFAULT_DEL_CMD
Expand Down Expand Up @@ -250,7 +255,7 @@ def wait_for_jobscripts(self, js_refs: list[str]) -> None:
time.sleep(2)

@abstractmethod
def format_options(
def format_directives(
self,
resources: ElementResources,
num_elements: int,
Expand All @@ -259,7 +264,7 @@ def format_options(
js_idx: int,
) -> str:
"""
Render options in a way that the scheduler can handle.
Render directives in a way that the scheduler can handle.
"""

def get_std_out_err_filename(
Expand Down
28 changes: 0 additions & 28 deletions hpcflow/sdk/submission/schedulers/direct.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,6 @@ class DirectScheduler(Scheduler[DirectRef]):
The correct subclass (:py:class:`DirectPosix` or :py:class:`DirectWindows`) should
be used to create actual instances.

Keyword Args
------------
shell_args: str
Arguments to pass to the shell. Pre-quoted.
shebang_args: str
Arguments to set on the shebang line. Pre-quoted.
options: dict
Options to the jobscript command.
"""

@classmethod
Expand Down Expand Up @@ -221,36 +213,16 @@ class DirectPosix(DirectScheduler):
"""
A direct scheduler for POSIX systems.

Keyword Args
------------
shell_args: str
Arguments to pass to the shell. Pre-quoted.
shebang_args: str
Arguments to set on the shebang line. Pre-quoted.
options: dict
Options to the jobscript command.
"""

#: Default shell.
DEFAULT_SHELL_EXECUTABLE: ClassVar[str] = "/bin/bash"


@hydrate
class DirectWindows(DirectScheduler):
"""
A direct scheduler for Windows.

Keyword Args
------------
shell_args: str
Arguments to pass to the shell. Pre-quoted.
options: dict
Options to the jobscript command.
"""

#: Default shell.
DEFAULT_SHELL_EXECUTABLE: ClassVar[str] = "powershell.exe"

@override
def get_submit_command(
self, shell: Shell, js_path: str, deps: dict[Any, tuple[Any, ...]]
Expand Down
19 changes: 8 additions & 11 deletions hpcflow/sdk/submission/schedulers/sge.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,11 @@ class SGEPosix(QueuedScheduler):
------------
cwd_switch: str
Override of default switch to use to set the current working directory.
shell_args: str
Arguments to pass to the shell. Pre-quoted.
shebang_args: str
Arguments to set on the shebang line. Pre-quoted.
options: dict
Options to the jobscript command.
directives: dict
Scheduler directives. Each item is written verbatim in the jobscript as a
scheduler directive, and is not processed in any way. If a value is `None`, the
key is considered a flag-like directive. If a value is a list, multiple directives
will be printed to the jobscript with the same key, but different values.

Notes
-----
Expand All @@ -55,8 +54,6 @@ class SGEPosix(QueuedScheduler):

"""

#: Default args for shebang line.
DEFAULT_SHEBANG_ARGS: ClassVar[str] = ""
#: Default submission command.
DEFAULT_SUBMIT_CMD: ClassVar[str] = "qsub"
#: Default command to show the queue state.
Expand Down Expand Up @@ -203,7 +200,7 @@ def __format_std_stream_file_option_lines(
yield f"{self.js_cmd} -e {base}"

@override
def format_options(
def format_directives(
self,
resources: ElementResources,
num_elements: int,
Expand All @@ -212,7 +209,7 @@ def format_options(
js_idx: int,
) -> str:
"""
Format the options to the jobscript command.
Format the directives to the jobscript command.
"""
opts: list[str] = []
opts.append(self.format_switch(self.cwd_switch))
Expand All @@ -226,7 +223,7 @@ def format_options(
)
)

for opt_k, opt_v in self.options.items():
for opt_k, opt_v in self.directives.items():
if opt_v is None:
opts.append(f"{self.js_cmd} {opt_k}")
elif isinstance(opt_v, list):
Expand Down
21 changes: 8 additions & 13 deletions hpcflow/sdk/submission/schedulers/slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,11 @@ class SlurmPosix(QueuedScheduler):

Keyword Args
------------
shell_args: str
Arguments to pass to the shell. Pre-quoted.
shebang_args: str
Arguments to set on the shebang line. Pre-quoted.
options: dict
Options to the jobscript command.
directives: dict
Scheduler directives. Each item is written verbatim in the jobscript as a
scheduler directive, and is not processed in any way. If a value is `None`, the
key is considered a flag-like directive. If a value is a list, multiple directives
will be printed to the jobscript with the same key, but different values.

Notes
-----
Expand All @@ -59,10 +58,6 @@ class SlurmPosix(QueuedScheduler):

"""

#: Default shell.
DEFAULT_SHELL_EXECUTABLE: ClassVar[str] = "/bin/bash"
#: Default args for shebang line.
DEFAULT_SHEBANG_ARGS: ClassVar[str] = ""
#: Default submission command.
DEFAULT_SUBMIT_CMD: ClassVar[str] = "sbatch"
#: Default command to show the queue state.
Expand Down Expand Up @@ -368,7 +363,7 @@ def __format_std_stream_file_option_lines(
yield f"{self.js_cmd} --error {base}.err"

@override
def format_options(
def format_directives(
self,
resources: ElementResources,
num_elements: int,
Expand All @@ -377,7 +372,7 @@ def format_options(
js_idx: int,
) -> str:
"""
Format the options to the scheduler.
Format the directives to the scheduler.
"""
opts: list[str] = []
opts.extend(self.__format_core_request_lines(resources))
Expand All @@ -391,7 +386,7 @@ def format_options(
)
)

for opt_k, opt_v in self.options.items():
for opt_k, opt_v in self.directives.items():
if isinstance(opt_v, list):
for i in opt_v:
opts.append(f"{self.js_cmd} {opt_k} {i}")
Expand Down
19 changes: 14 additions & 5 deletions hpcflow/sdk/submission/shells/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,30 +79,39 @@ class Shell(ABC):
#: Template for the jobscript footer.
JS_FOOTER: ClassVar[str]

__slots__ = ("_executable", "os_args")
__slots__ = ("_executable", "executable_args", "os_args")

def __init__(
self, executable: str | None = None, os_args: dict[str, str] | None = None
self,
executable: str | None = None,
executable_args: list[str] | None = None,
os_args: dict[str, str] | None = None,
):
#: Which executable implements the shell.
self._executable = executable or self.DEFAULT_EXE
#: Arguments to provide to the shell executable (e.g. `--login`).
self.executable_args = executable_args or []
#: Arguments to pass to the shell.
self.os_args = os_args or {}

def __eq__(self, other: Any) -> bool:
if not isinstance(other, self.__class__):
return False
return self._executable == other._executable and self.os_args == other.os_args
return (
self._executable == other._executable
and self.executable_args == other.executable_args
and self.os_args == other.os_args
)

def __hash__(self):
return get_hash((self._executable, self.os_args))
return get_hash((self._executable, tuple(self.executable_args), self.os_args))

@property
def executable(self) -> list[str]:
"""
The executable to use plus any mandatory arguments.
"""
return [self._executable]
return [self._executable, *self.executable_args]

@property
def shebang_executable(self) -> list[str]:
Expand Down
6 changes: 5 additions & 1 deletion hpcflow/sdk/submission/shells/bash.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Bash(Shell):
#: Indent for environment setup.
JS_ENV_SETUP_INDENT: ClassVar[str] = 2 * JS_INDENT
#: Template for the jobscript shebang line.
JS_SHEBANG: ClassVar[str] = """#!{shebang_executable} {shebang_args}"""
JS_SHEBANG: ClassVar[str] = """#!{shebang}"""
#: Template for the jobscript functions file.
JS_FUNCS: ClassVar[str] = dedent(
"""\
Expand Down Expand Up @@ -429,6 +429,10 @@ def executable(self) -> list[str]:

@property
def shebang_executable(self) -> list[str]:
"""
The executable to use in a shebang line, overridden here to exclude the WSL
command.
"""
return super().executable

def _get_OS_info_POSIX(self) -> Mapping[str, str]:
Expand Down
Loading
Loading