-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[CLI] Improve ray status for placement groups #18289
Conversation
Btw, this is still confusing;
We uses 66 GPUs because they are all reserved by placement groups. I was thinking
this as an alternative format, but haven't implemented it because this can also be confusing... |
test_cli failing |
This doesn't grammatically make sense. Does it mean 66 is reserved in placement groups, but 0 is used within the group? |
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.
Comment
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.
See comments
Yes. Right now the representation is
where X is "used" resources in placement groups. and this will become
I agree this rep is a little weird. Another alternative is
or
One con of this option is that when your usages are mixed up with placement group usages, it is hard to read. Imagine
|
I vote for (2)
…On Thu, Sep 2, 2021, 4:38 PM SangBin Cho ***@***.***> wrote:
@ericl <https://github.com/ericl>
This doesn't grammatically make sense. Does it mean 66 is reserved in
placement groups, but 0 is used within the group?
Yes. Right now the representation is
66.0/66.0 GPU (X reserved in placement groups)
where X is "used" resources in placement groups.
and this will become
66.0/66.6 GPU (X/66.0 used in placement groups).
I agree this rep is a little weird. Another alternative is
1. Separate out resources used by pgs
0.0/0.0 GPU # Original resources
0.0/66.0 GPU (reserved by placement groups) # PG reserved resources
or
1. Use total reserved amount in the paranthesis and instead use the
real usage
X/66.0 GPU (66.0 reserved in placement groups) # X == used - pg_reserved + pg_used
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18289 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAADUSSATLWMISU2EPJUJ4DUAADHLANCNFSM5DHQXLNA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
LGTM. The extra redundancy helps understanding.
…On Thu, Sep 2, 2021, 11:18 PM SangBin Cho ***@***.***> wrote:
@ericl <https://github.com/ericl> @scv119 <https://github.com/scv119> I'd
like to get some final thoughts; I am thinking this;
66 CPU
56 pg reserved
4 used by CPU
40 used by pg reserved
# Current
60.0/66.0 (40 reserved in placement groups)
# New proposal
44.0/66.0 CPU (40 used, 56.0 reserved in placement groups )
Thought?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18289 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAADUSROEWJQL5J5RJ4WVCLUABSD5ANCNFSM5DHQXLNA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Why are these changes needed?
After
Before
Related issue number
Checks
scripts/format.sh
to lint the changes in this PR.