Skip to content

Commit

Permalink
Fixing command line interface for the run command.
Browse files Browse the repository at this point in the history 
This commit fixes bugs associated with the MLCube's `run` command:
- It now uses correct option decorators from the `cli` (class Options) instead of deprecated decorators from the `__main__.py` file.
- It now uses the `-P` option to correctly parse MLCube parameters that users provide on a command line.

The latter particularly fixes the mlcommons#347 issue.
  • Loading branch information
@sergey-serebryakov
sergey-serebryakov committed Dec 11, 2023
1 parent 755d388 commit 7ceb50f
Showing 1 changed file with 34 additions and 73 deletions.
107 changes: 34 additions & 73 deletions mlcube/mlcube/__main__.py
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
"""This requires the MLCube 2.0 that's located somewhere in one of dev branches."""
import logging
import os
from pathlib import Path
import shutil
import sys
import typing as t
from pathlib import Path

import click
import coloredlogs
Expand Down Expand Up @@ -41,9 +41,7 @@ def parser_process(value: str, state: click.parser.ParsingState):

super(MultiValueOption, self).add_to_parser(parser, ctx)
for opt_name in self.opts:
our_parser: t.Optional[click.parser.Option] = parser._long_opt.get(
opt_name
) or parser._short_opt.get(opt_name)
our_parser: t.Optional[click.parser.Option] = parser._long_opt.get(opt_name) or parser._short_opt.get(opt_name)
if our_parser:
self._eat_all_parser = our_parser
self._previous_parser_process = our_parser.process
Expand Down Expand Up @@ -238,9 +236,7 @@ def configure(mlcube: t.Optional[str], platform: str, p: t.Tuple[str]) -> None:
p: Additional MLCube configuration parameters (these parameters are those parameters that normally start with
`-P` prefix). Here, due to original implementation, we need to `unparse` by adding `-P` prefix.
"""
logger.debug(
"mlcube::configure, mlcube=%s, platform=%s, p=%s", mlcube, platform, str(p)
)
logger.debug("mlcube::configure, mlcube=%s, platform=%s, p=%s", mlcube, platform, str(p))
if mlcube is None:
mlcube = os.getcwd()
logger.info(
Expand Down Expand Up @@ -280,16 +276,17 @@ def configure(mlcube: t.Optional[str], platform: str, p: t.Tuple[str]) -> None:
"max_content_width": _TERMINAL_WIDTH,
},
)
@mlcube_option
@platform_option
@task_option
@workspace_option
@Options.mlcube
@Options.platform
@Options.task
@Options.workspace
@network_option
@security_option
@gpus_option
@memory_option
@cpu_option
@mount_option
@Options.parameter
@Options.help
@click.pass_context
def run(
Expand All @@ -304,6 +301,7 @@ def run(
memory: str,
cpu: str,
mount: str,
p: t.Tuple[str],
) -> None:
"""Run MLCube task(s).
Expand All @@ -321,11 +319,12 @@ def run(
cpu: CPU options defined during MLCube container execution.
mount: Mount (global) options defined for all input parameters in all tasks to be executed. They override any
mount options defined for individual parameters.
p: Additional MLCube configuration parameters (these parameters are those parameters that normally start with
`-P` prefix). Here, due to original implementation, we need to `unparse` by adding `-P` prefix.
"""
logger.info(
"run input_arg mlcube=%s, platform=%s, task=%s, workspace=%s, network=%s, security=%s, gpus=%s, "
"memory=%s, mount=%s"
"cpu=%s",
"memory=%s, mount=%s, cpu=%s, p=%s",
mlcube,
platform,
task,
Expand All @@ -336,9 +335,10 @@ def run(
memory,
cpu,
mount,
str(p),
)
runner_cls, mlcube_config = parse_cli_args(
unparsed_args=ctx.args,
unparsed_args=ctx.args + ["-P" + param for param in p],
parsed_args={
"mlcube": mlcube,
"platform": platform,
Expand All @@ -352,17 +352,11 @@ def run(
},
resolve=True,
)
mlcube_tasks: t.List[str] = list(
(mlcube_config.get("tasks", None) or {}).keys()
) # Tasks in this MLCube.
tasks: t.List[str] = CliParser.parse_list_arg(
task, default=None
) # Requested tasks.
mlcube_tasks: t.List[str] = list((mlcube_config.get("tasks", None) or {}).keys()) # Tasks in this MLCube.
tasks: t.List[str] = CliParser.parse_list_arg(task, default=None) # Requested tasks.

if len(tasks) == 0:
logger.warning(
"Missing required task name (--task=COMMA_SEPARATED_LIST_OF_TASK_NAMES)."
)
logger.warning("Missing required task name (--task=COMMA_SEPARATED_LIST_OF_TASK_NAMES).")
if len(mlcube_tasks) != 1:
logger.error(
"Task name could not be automatically resolved (supported tasks = %s).",
Expand All @@ -379,8 +373,7 @@ def run(
unknown_tasks: t.List[str] = [name for name in tasks if name not in mlcube_tasks]
if len(unknown_tasks) > 0:
logger.error(
"Unknown tasks have been requested: supported tasks = %s, requested tasks = %s, "
"unknown tasks = %s.",
"Unknown tasks have been requested: supported tasks = %s, requested tasks = %s, " "unknown tasks = %s.",
str(mlcube_tasks),
str(tasks),
str(unknown_tasks),
Expand Down Expand Up @@ -419,9 +412,7 @@ def describe(mlcube: t.Optional[str]) -> None:
"""
if mlcube is None:
mlcube = os.getcwd()
_, mlcube_config = parse_cli_args(
unparsed_args=[], parsed_args={"mlcube": mlcube}, resolve=True
)
_, mlcube_config = parse_cli_args(unparsed_args=[], parsed_args={"mlcube": mlcube}, resolve=True)
print("MLCube")
print(f" path = {mlcube_config.runtime.root}")
print(f" name = {mlcube_config.name}:{mlcube_config.get('version', 'latest')}")
Expand All @@ -433,25 +424,17 @@ def describe(mlcube: t.Optional[str]) -> None:
for task_name, task_def in mlcube_config.tasks.items():
description = f"name = {task_name}"
if len(task_def.parameters.inputs) > 0:
description = (
f"{description}, inputs = {list(task_def.parameters.inputs.keys())}"
)
description = f"{description}, inputs = {list(task_def.parameters.inputs.keys())}"
if len(task_def.parameters.outputs) > 0:
description = (
f"{description}, outputs = {list(task_def.parameters.outputs.keys())}"
)
description = f"{description}, outputs = {list(task_def.parameters.outputs.keys())}"
print(f" {description}")
print()
print("Run this MLCube:")
print(" Configure MLCube:")
print(
f" mlcube configure --mlcube={mlcube_config.runtime.root} --platform=docker"
)
print(f" mlcube configure --mlcube={mlcube_config.runtime.root} --platform=docker")
print(" Run MLCube tasks:")
for task_name in mlcube_config.tasks.keys():
print(
f" mlcube run --mlcube={mlcube_config.runtime.root} --task={task_name} --platform=docker"
)
print(f" mlcube run --mlcube={mlcube_config.runtime.root} --task={task_name} --platform=docker")
print()


Expand Down Expand Up @@ -543,19 +526,11 @@ def config(
ctx: click.core.Context,
list_all: bool, # mlcube config --list
get: t.Optional[str], # mlcube config --get KEY
create_platform: t.Optional[
t.Tuple
], # mlcube config --create-platform RUNNER PLATFORM
create_platform: t.Optional[t.Tuple], # mlcube config --create-platform RUNNER PLATFORM
remove_platform: t.Optional[str], # mlcube config --remove-platform NAME
rename_platform: t.Optional[
t.Tuple
], # mlcube config --rename-platform OLD_NAME NEW_NAME
copy_platform: t.Optional[
t.Tuple
], # mlcube config --copy-platform EXISTING_PLATFORM NEW_PLATFORM
rename_runner: t.Optional[
t.Tuple
], # mlcube config --rename-runner OLD_NAME NEW_NAME
rename_platform: t.Optional[t.Tuple], # mlcube config --rename-platform OLD_NAME NEW_NAME
copy_platform: t.Optional[t.Tuple], # mlcube config --copy-platform EXISTING_PLATFORM NEW_PLATFORM
rename_runner: t.Optional[t.Tuple], # mlcube config --rename-runner OLD_NAME NEW_NAME
remove_runner: t.Optional[str], # mlcube config --remove-runner NAME
) -> None:
"""Display or change MLCube system settings.
Expand All @@ -570,49 +545,35 @@ def config(
print(f"System settings file path = {SystemSettings.system_settings_file()}")
settings = SystemSettings()

def _check_tuple(
_tuple: t.Tuple, _name: str, _expected_size: int, _expected_value: str
) -> None:
def _check_tuple(_tuple: t.Tuple, _name: str, _expected_size: int, _expected_value: str) -> None:
if len(_tuple) != _expected_size:
raise IllegalParameterValueError(
f"--{_name}", " ".join(_tuple), f'"{_expected_value}"'
)
raise IllegalParameterValueError(f"--{_name}", " ".join(_tuple), f'"{_expected_value}"')

try:
if list_all:
print(OmegaConf.to_yaml(settings.settings))
elif get:
print(OmegaConf.to_yaml(OmegaConf.select(settings.settings, get)))
elif create_platform:
_check_tuple(
create_platform, "create_platform", 2, "RUNNER_NAME PLATFORM_NAME"
)
_check_tuple(create_platform, "create_platform", 2, "RUNNER_NAME PLATFORM_NAME")
settings.create_platform(create_platform)
elif remove_platform:
settings.remove_platform(remove_platform)
elif rename_platform:
_check_tuple(rename_platform, "rename_platform", 2, "OLD_NAME NEW_NAME")
settings.copy_platform(rename_platform, delete_source=True)
elif copy_platform:
_check_tuple(
copy_platform, "copy_platform", 2, "EXISTING_PLATFORM NEW_PLATFORM"
)
_check_tuple(copy_platform, "copy_platform", 2, "EXISTING_PLATFORM NEW_PLATFORM")
settings.copy_platform(rename_platform, delete_source=False)
elif rename_runner:
_check_tuple(rename_runner, "rename_runner", 2, "OLD_NAME NEW_NAME")
update_platforms: bool = (
"--update-platforms" in ctx.args or "--update_platforms" in ctx.args
)
update_platforms: bool = "--update-platforms" in ctx.args or "--update_platforms" in ctx.args
settings.rename_runner(rename_runner, update_platforms=update_platforms)
elif remove_runner:
remove_platforms: bool = (
"--remove-platforms" in ctx.args or "--remove_platforms" in ctx.args
)
remove_platforms: bool = "--remove-platforms" in ctx.args or "--remove_platforms" in ctx.args
settings.remove_runner(remove_runner, remove_platforms=remove_platforms)
except MLCubeError as e:
logger.error(
"Command failed, command = '%s' error = '%s'", " ".join(sys.argv), str(e)
)
logger.error("Command failed, command = '%s' error = '%s'", " ".join(sys.argv), str(e))


@cli.command(
Expand Down

0 comments on commit 7ceb50f

Please sign in to comment.