Skip to content

[Resources] Add cpus in resource specification #1622

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.

Sign up for GitHub

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

Merged
WoosukKwon merged 54 commits into from
Jan 28, 2023
Merged

[Resources] Add cpus in resource specification #1622

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
d5b5511
Add cpu option
WoosukKwon Jan 24, 2023
3e0fa4a
yapf
WoosukKwon Jan 24, 2023
22fe74b
Add cpu in CLI
WoosukKwon Jan 24, 2023
386b1dc
Fix GCP and Do not raise errors
WoosukKwon Jan 24, 2023
23a0ea5
yapf
WoosukKwon Jan 24, 2023
28b0c5a
Fix TPU VM
WoosukKwon Jan 24, 2023
0d8cf90
Print error msg for cpu mismatches
WoosukKwon Jan 24, 2023
a825cf7
Fix error when default instance is not found
WoosukKwon Jan 24, 2023
d65634c
Merge branch 'master' into woosuk-cpu
WoosukKwon Jan 24, 2023
4959f8f
Fix a comment
WoosukKwon Jan 24, 2023
5d57fd5
Minor
WoosukKwon Jan 24, 2023
9067308
Add _impl
WoosukKwon Jan 24, 2023
797a632
Define _DEFAULT_NUM_VCPUS as constant
WoosukKwon Jan 24, 2023
b4e0b00
yapf
WoosukKwon Jan 24, 2023
b95c56b
Ban specifying CPU in sky exec
WoosukKwon Jan 24, 2023
9324e52
yapf
WoosukKwon Jan 24, 2023
12f4bbe
Fix type error
WoosukKwon Jan 25, 2023
09ad032
xlarge -> large
WoosukKwon Jan 25, 2023
83a7a9d
Fix AWS
WoosukKwon Jan 25, 2023
c400dea
Fix comments on default instance family
WoosukKwon Jan 25, 2023
75cf94a
Fix sky exec
WoosukKwon Jan 25, 2023
afd80fe
minor
WoosukKwon Jan 25, 2023
89ba768
Rename
WoosukKwon Jan 25, 2023
e4ae323
fuzzy_candidate_list -> []
WoosukKwon Jan 25, 2023
6103c2e
Fix version up comment
WoosukKwon Jan 25, 2023
1961191
Minor
WoosukKwon Jan 25, 2023
f856b66
Fix help msg
WoosukKwon Jan 25, 2023
40ac63b
Add docstring for cpu
WoosukKwon Jan 25, 2023
2ac7471
Document get_default_instance_type
WoosukKwon Jan 25, 2023
62f78a2
prevent sky launch -c --cpu and sky exec --cpu
WoosukKwon Jan 25, 2023
b141f75
Minor fix
WoosukKwon Jan 25, 2023
bc7cd0a
cpu -> cpus
WoosukKwon Jan 25, 2023
a6f4355
yapf
WoosukKwon Jan 25, 2023
8033bbf
vcpus -> vCPUs
WoosukKwon Jan 25, 2023
add2811
n2 -> n2-standard
WoosukKwon Jan 25, 2023
aa926af
Fix docstring
WoosukKwon Jan 25, 2023
0368415
Add comment on assertion
WoosukKwon Jan 25, 2023
54faa77
Fix docstring
WoosukKwon Jan 25, 2023
8f9dabc
Fix docstring
WoosukKwon Jan 25, 2023
97dc60e
Add assert in less_demanding_than
WoosukKwon Jan 25, 2023
4599f36
roll back
WoosukKwon Jan 25, 2023
01021c9
Merge branch 'master' into woosuk-cpu
WoosukKwon Jan 27, 2023
4949000
Remove is_same_resources
WoosukKwon Jan 27, 2023
fec1b7a
Add TODO
WoosukKwon Jan 27, 2023
13607fd
Check cpus format again in _filter_with_cpus
WoosukKwon Jan 27, 2023
2059309
yapf
WoosukKwon Jan 27, 2023
6a2b554
Add tests
WoosukKwon Jan 27, 2023
99a66e7
Fix tests
WoosukKwon Jan 27, 2023
ad4827b
Fix
WoosukKwon Jan 27, 2023
2c08184
Fix
WoosukKwon Jan 27, 2023
014f63c
Fix tests
WoosukKwon Jan 28, 2023
1f8a031
Merge branch 'master' into woosuk-cpu
WoosukKwon Jan 28, 2023
62aa150
Fix example_app
WoosukKwon Jan 28, 2023
4ea39d7
Add cpus in yaml spec
WoosukKwon Jan 28, 2023
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
68 changes: 50 additions & 18 deletions sky/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ def _interactive_node_cli_command(cli_func):
default=None,
type=str,
help='Instance type to use.')
cpu = click.option('--cpu',
default=None,
type=str,
help='Number of vCPUs to use. '
'(e.g., ``--cpu=4`` or ``--cpu=4+``).')
gpus = click.option('--gpus',
default=None,
type=str,
Expand Down Expand Up @@ -268,6 +273,7 @@ def _interactive_node_cli_command(cli_func):
region_option,
zone_option,
instance_type_option,
cpu,
*([gpus] if cli_func.__name__ == 'gpunode' else []),
*([tpus] if cli_func.__name__ == 'tpunode' else []),
spot_option,
Expand Down Expand Up @@ -392,6 +398,12 @@ def _parse_env_var(env_var: str) -> Tuple[str, str]:
'"resources.instance_type" config. Passing "none" resets the '
'config.'),
),
click.option(
'--cpu',
default=None,
type=str,
required=False,
help=('The number of vCPUs to use. Either <number> or <number>+.')),
]


Expand Down Expand Up @@ -556,6 +568,7 @@ def _parse_override_params(cloud: Optional[str] = None,
region: Optional[str] = None,
zone: Optional[str] = None,
gpus: Optional[str] = None,
cpu: Optional[str] = None,
instance_type: Optional[str] = None,
use_spot: Optional[bool] = None,
image_id: Optional[str] = None,
Expand All @@ -582,6 +595,11 @@ def _parse_override_params(cloud: Optional[str] = None,
override_params['accelerators'] = None
else:
override_params['accelerators'] = gpus
if cpu is not None:
if cpu.lower() == 'none':
override_params['cpu'] = None
else:
override_params['cpu'] = cpu
if instance_type is not None:
if instance_type.lower() == 'none':
override_params['instance_type'] = None
Expand Down Expand Up @@ -908,6 +926,7 @@ def _make_task_from_entrypoint_with_overrides(
region: Optional[str] = None,
zone: Optional[str] = None,
gpus: Optional[str] = None,
cpu: Optional[str] = None,
instance_type: Optional[str] = None,
num_nodes: Optional[int] = None,
use_spot: Optional[bool] = None,
Expand Down Expand Up @@ -949,6 +968,7 @@ def _make_task_from_entrypoint_with_overrides(
region=region,
zone=zone,
gpus=gpus,
cpu=cpu,
instance_type=instance_type,
use_spot=use_spot,
image_id=image_id,
Expand Down Expand Up @@ -1154,6 +1174,7 @@ def launch(
region: Optional[str],
zone: Optional[str],
gpus: Optional[str],
cpu: Optional[str],
instance_type: Optional[str],
num_nodes: Optional[int],
use_spot: Optional[bool],
Expand Down Expand Up @@ -1198,6 +1219,7 @@ def launch(
region=region,
zone=zone,
gpus=gpus,
cpu=cpu,
instance_type=instance_type,
num_nodes=num_nodes,
use_spot=use_spot,
Expand Down Expand Up @@ -1259,6 +1281,7 @@ def exec(
zone: Optional[str],
workdir: Optional[str],
gpus: Optional[str],
cpu: Optional[str],
instance_type: Optional[str],
num_nodes: Optional[int],
use_spot: Optional[bool],
Expand Down Expand Up @@ -1343,6 +1366,7 @@ def exec(
region=region,
zone=zone,
gpus=gpus,
cpu=cpu,
instance_type=instance_type,
use_spot=use_spot,
image_id=image_id,
Expand Down Expand Up @@ -2414,11 +2438,11 @@ def _down_or_stop(name: str):
# pylint: disable=redefined-outer-name
def gpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
cloud: Optional[str], region: Optional[str], zone: Optional[str],
instance_type: Optional[str], gpus: Optional[str],
use_spot: Optional[bool], screen: Optional[bool],
tmux: Optional[bool], disk_size: Optional[int],
idle_minutes_to_autostop: Optional[int], down: bool,
retry_until_up: bool):
instance_type: Optional[str], cpu: Optional[str],
gpus: Optional[str], use_spot: Optional[bool],
screen: Optional[bool], tmux: Optional[bool],
disk_size: Optional[int], idle_minutes_to_autostop: Optional[int],
down: bool, retry_until_up: bool):
"""Launch or attach to an interactive GPU node.

Examples:
Expand Down Expand Up @@ -2457,7 +2481,8 @@ def gpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],

user_requested_resources = not (cloud is None and region is None and
zone is None and instance_type is None and
gpus is None and use_spot is None)
cpu is None and gpus is None and
use_spot is None)
default_resources = _INTERACTIVE_NODE_DEFAULT_RESOURCES['gpunode']
cloud_provider = clouds.CLOUD_REGISTRY.from_str(cloud)
if gpus is None and instance_type is None:
Expand All @@ -2470,6 +2495,7 @@ def gpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
region=region,
zone=zone,
instance_type=instance_type,
cpu=cpu,
accelerators=gpus,
use_spot=use_spot,
disk_size=disk_size)
Expand All @@ -2493,10 +2519,11 @@ def gpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
# pylint: disable=redefined-outer-name
def cpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
cloud: Optional[str], region: Optional[str], zone: Optional[str],
instance_type: Optional[str], use_spot: Optional[bool],
screen: Optional[bool], tmux: Optional[bool],
disk_size: Optional[int], idle_minutes_to_autostop: Optional[int],
down: bool, retry_until_up: bool):
instance_type: Optional[str], cpu: Optional[str],
use_spot: Optional[bool], screen: Optional[bool],
tmux: Optional[bool], disk_size: Optional[int],
idle_minutes_to_autostop: Optional[int], down: bool,
retry_until_up: bool):
"""Launch or attach to an interactive CPU node.

Examples:
Expand Down Expand Up @@ -2534,7 +2561,7 @@ def cpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],

user_requested_resources = not (cloud is None and region is None and
zone is None and instance_type is None and
use_spot is None)
cpu is None and use_spot is None)
default_resources = _INTERACTIVE_NODE_DEFAULT_RESOURCES['cpunode']
cloud_provider = clouds.CLOUD_REGISTRY.from_str(cloud)
if instance_type is None:
Expand All @@ -2545,6 +2572,7 @@ def cpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
region=region,
zone=zone,
instance_type=instance_type,
cpu=cpu,
use_spot=use_spot,
disk_size=disk_size)

Expand All @@ -2567,11 +2595,12 @@ def cpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
# pylint: disable=redefined-outer-name
def tpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
region: Optional[str], zone: Optional[str],
instance_type: Optional[str], tpus: Optional[str],
use_spot: Optional[bool], tpu_vm: Optional[bool],
screen: Optional[bool], tmux: Optional[bool],
disk_size: Optional[int], idle_minutes_to_autostop: Optional[int],
down: bool, retry_until_up: bool):
instance_type: Optional[str], cpu: Optional[str],
tpus: Optional[str], use_spot: Optional[bool],
tpu_vm: Optional[bool], screen: Optional[bool],
tmux: Optional[bool], disk_size: Optional[int],
idle_minutes_to_autostop: Optional[int], down: bool,
retry_until_up: bool):
"""Launch or attach to an interactive TPU node.

Examples:
Expand Down Expand Up @@ -2608,8 +2637,8 @@ def tpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
name = _default_interactive_node_name('tpunode')

user_requested_resources = not (region is None and zone is None and
instance_type is None and tpus is None and
use_spot is None)
instance_type is None and cpu is None and
tpus is None and use_spot is None)
default_resources = _INTERACTIVE_NODE_DEFAULT_RESOURCES['tpunode']
accelerator_args = default_resources.accelerator_args
if tpu_vm:
Expand All @@ -2625,6 +2654,7 @@ def tpunode(cluster: str, yes: bool, port_forward: Optional[List[int]],
region=region,
zone=zone,
instance_type=instance_type,
cpu=cpu,
accelerators=tpus,
accelerator_args=accelerator_args,
use_spot=use_spot,
Expand Down Expand Up @@ -3009,6 +3039,7 @@ def spot_launch(
region: Optional[str],
zone: Optional[str],
gpus: Optional[str],
cpu: Optional[str],
instance_type: Optional[str],
num_nodes: Optional[int],
use_spot: Optional[bool],
Expand Down Expand Up @@ -3047,6 +3078,7 @@ def spot_launch(
region=region,
zone=zone,
gpus=gpus,
cpu=cpu,
instance_type=instance_type,
num_nodes=num_nodes,
use_spot=use_spot,
Expand Down
19 changes: 12 additions & 7 deletions sky/clouds/aws.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,9 @@ def is_same_cloud(self, other: clouds.Cloud):
return isinstance(other, AWS)

@classmethod
def get_default_instance_type(cls) -> str:
# General-purpose instance with 8 vCPUs and 32 GB RAM.
# Intel Ice Lake 8375C
return 'm6i.2xlarge'
def get_default_instance_type(cls,
cpu: Optional[str] = None) -> Optional[str]:
return service_catalog.get_default_instance_type(cpu=cpu, clouds='aws')

# TODO: factor the following three methods, as they are the same logic
# between Azure and AWS.
Expand Down Expand Up @@ -350,16 +349,21 @@ def _make(instance_list):
# Setting this to None as AWS doesn't separately bill /
# attach the accelerators. Billed as part of the VM type.
accelerators=None,
cpu=None,
)
resource_list.append(r)
return resource_list

# Currently, handle a filter on accelerators only.
accelerators = resources.accelerators
if accelerators is None:
# No requirements to filter, so just return a default VM type.
return (_make([AWS.get_default_instance_type()]),
fuzzy_candidate_list)
# Return a default instance type with the given number of vCPUs.
default_instance_type = AWS.get_default_instance_type(
Copy link
Copy Markdown
Member

@concretevitamin concretevitamin Jan 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For discussion/a thought: This is now dynamically picking a VM type based on cpu, which means it's no longer a "default" constant. More like a service_catalog.get_instance_type_for_cpu() a la L371.

Should we remove this interface (Cloud.get_default_instance_type()) then? Wdyt? (I can see arguments for both choices.)

Copy link
Copy Markdown
Collaborator Author

@WoosukKwon WoosukKwon Jan 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it cannot be get_instance_type_for_cpu() because it limits the search space to the default instance family and uses 8 as the default number of the vCPUs. I think get_default_instance_type is fine; one can easily understand what it means.

Copy link
Copy Markdown
Member

@concretevitamin concretevitamin Jan 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming part sounds good. Sorry, I did not spell out in my comment but was thinking removing one interface from Cloud will make it easier to add new clouds. Also, it's a shallow method (1-line dispatch). How about removing it and just calling service_catalog.get_default_instance_type() directly here?

Copy link
Copy Markdown
Collaborator Author

@WoosukKwon WoosukKwon Jan 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be still useful to explicitly show what the default type is?

Copy link
Copy Markdown
Member

@concretevitamin concretevitamin Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind clarifying what you meant?

Currently, the Cloud interface includes a shallow method, which does not show the default type:

    @classmethod
    def get_default_instance_type(cls,
                                  cpu: Optional[str] = None) -> Optional[str]:
        return service_catalog.get_default_instance_type(cpu=cpu, clouds='aws')

Copy link
Copy Markdown
Collaborator Author

@WoosukKwon WoosukKwon Jan 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean one can easily find which instance type is default for AWS by calling AWS.get_default_instance_type, which is good.

Copy link
Copy Markdown
Member

@concretevitamin concretevitamin Jan 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

cpu=resources.cpu)
if default_instance_type is None:
return ([], fuzzy_candidate_list)
else:
return (_make([default_instance_type]), fuzzy_candidate_list)

assert len(accelerators) == 1, resources
acc, acc_count = list(accelerators.items())[0]
Expand All @@ -368,6 +372,7 @@ def _make(instance_list):
acc,
acc_count,
use_spot=resources.use_spot,
cpu=resources.cpu,
region=resources.region,
zone=resources.zone,
clouds='aws')
Expand Down
23 changes: 15 additions & 8 deletions sky/clouds/azure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ def is_same_cloud(self, other):
return isinstance(other, Azure)

@classmethod
def get_default_instance_type(cls) -> str:
# General-purpose instance with 8 vCPUs and 32 GB RAM.
# Intel Ice Lake 8370C
return 'Standard_D8_v5'
def get_default_instance_type(cls,
cpu: Optional[str] = None) -> Optional[str]:
return service_catalog.get_default_instance_type(cpu=cpu,
clouds='azure')

def _get_image_config(self, gen_version, instance_type):
# az vm image list \
Expand Down Expand Up @@ -265,23 +265,30 @@ def _make(instance_list):
instance_type=instance_type,
# Setting this to None as Azure doesn't separately bill /
# attach the accelerators. Billed as part of the VM type.
accelerators=None)
accelerators=None,
cpu=None,
)
resource_list.append(r)
return resource_list

# Currently, handle a filter on accelerators only.
accelerators = resources.accelerators
if accelerators is None:
# No requirements to filter, so just return a default VM type.
return (_make([Azure.get_default_instance_type()]),
fuzzy_candidate_list)
# Return a default instance type with the given number of vCPUs.
default_instance_type = Azure.get_default_instance_type(
cpu=resources.cpu)
if default_instance_type is None:
return ([], fuzzy_candidate_list)
else:
return (_make([default_instance_type]), fuzzy_candidate_list)

assert len(accelerators) == 1, resources
acc, acc_count = list(accelerators.items())[0]
(instance_list, fuzzy_candidate_list
) = service_catalog.get_instance_type_for_accelerator(
acc,
acc_count,
cpu=resources.cpu,
use_spot=resources.use_spot,
region=resources.region,
zone=resources.zone,
Expand Down
3 changes: 2 additions & 1 deletion sky/clouds/cloud.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ def get_accelerators_from_instance_type(
raise NotImplementedError

@classmethod
def get_default_instance_type(cls) -> str:
def get_default_instance_type(cls,
cpu: Optional[str] = None) -> Optional[str]:
raise NotImplementedError

@classmethod
Expand Down
Loading