-
Notifications
You must be signed in to change notification settings - Fork 640
[GCP] Remap series-specific disk types #5457
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
[GCP] Remap series-specific disk types #5457
Conversation
Signed-off-by: JiangJiaWei1103 <[email protected]>
@SeungjinYang PTAL, thanks a lot! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I assume you might not be done with the PR given it is in draft state, but this looks good. I was a bit worried the logic might be a sprawl, but the extra logic added ends up being compensated by the logic sprawl removed in check_disk_tier
.
Signed-off-by: JiangJiaWei1103 <[email protected]>
Signed-off-by: JiangJiaWei1103 <[email protected]>
After applying all the suggested changes to the code, we re-ran the experiments and obtained the same results:
Btw, I currently skip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @JiangJiaWei1103! Based on how the code is written I'm sure a2
type works with ULTRA works if n1
type works, which you tested.
Tracking issue
Closes #4705.
Why are the changes needed?
GCP instances have series-specific disk type supports which are summarized as follows:
To make SkyPilot smart at figuring out the best disk type for each specific machine series, we consider remapping disk types for those unsupported, other than raising an error.
What changes were proposed in this pull request?
HIGH
disk tier (which maps topd-ssd
) toULTRA
asN2
(with less than 64 vCPUs),N1
,A2
, andG2
don't supportpd-extreme
MEDIUM
disk tier (which maps topd-balanced
) toLOW
asG2
doesn't supportpd-standard
Screenshots
sky launch -t n2-standard-32 --disk-tier ultra --dryrun
sky launch -t n2-standard-128 --disk-tier ultra --dryrun
sky launch -t n1-standard-32 --disk-tier ultra --dryrun
sky launch -t g2-standard-4 --disk-tier ultra --dryrun
sky launch -t g2-standard-4 --disk-tier low --dryrun
Because we refactor the propagation logic, experiments on
A3
series are rerun (see #5351):sky launch -t a3-highgpu-8g --disk-tier ultra --dryrun
sky launch -t a3-highgpu-8g --disk-tier low --dryrun
Tested (run the relevant ones):
bash format.sh
/smoke-test
(CI) orpytest tests/test_smoke.py
(local)/smoke-test -k test_name
(CI) orpytest tests/test_smoke.py::test_name
(local)/quicktest-core
(CI) orpytest tests/smoke_tests/test_backward_compat.py
(local)