-
Notifications
You must be signed in to change notification settings - Fork 510
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
[Cost Report] Add some UI fixes #1788
[Cost Report] Add some UI fixes #1788
Conversation
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 for submitting the PR @sumanthgenz! Left several comments.
sky/utils/cli_utils/status_utils.py
Outdated
num_lines_to_display = _NUM_COST_REPORT_LINES | ||
if show_all: | ||
num_lines_to_display = len(cluster_records) | ||
|
||
for record in cluster_records[:num_lines_to_display]: |
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.
The cost records seems to be in the order of launch time, but it might be better to place the clusters that have not been terminated yet at the beginning.
|
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 @sumanthgenz. This is great.
UX comments:
Clusters
NAME LAUNCHED DURATION RESOURCES STATUS HOURLY_PRICE COST (est.)
...
Managed spot controller (will be autostopped if idle for 10min)
NAME LAUNCHED DURATION RESOURCES STATUS HOURLY_PRICE COST (est.)
sky-spot-controller-8a3968f2 2 months ago 4 days 12h 41m 44s 1x GCP(n1-highmem-8, disk_size=50) TERMINATED $ 0.473 $51.436
Total Cost: $16524.966
NOTE: Since --all is not set, not all cost report records may be displayed above.
NOTE: This feature is experimental. Costs for clusters with auto{stop,down} scheduled may not be accurate.
-
Can we make
Total Cost
sum up what's being displayed, rather than everything in history? In the table above myCOST (est.)
column adds up to <$100, so it took me a while to realize whyTotal Cost
displays a 5-figure number (which I'm not sure is accurate). -
Consider removing
NOTE:
since we've yellow'd the font. -
Consider
Since --all is not set, not all cost report records may be displayed above.
->Showing the N most recent clusters. To see all clusters in history, pass the --all flag.
if status is None: | ||
return -1 | ||
return 1 |
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.
Why are we sorting a bunch of -1's and 1's? From the output, seems like we are sorting by launch-time?
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.
We wanted to bubble up clusters that were active and un-terminated to the top since they are actively incurring cost. In addition, we also sort by launch time.
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.
Can we put this comment as comment before L172?
22e28b3
to
70b07c9
Compare
|
After switching to this branch, now seeing
Expected? |
Hi @concretevitamin, thanks for showing this. @Michaelvll was seeing this issue too, but I myself have not been encountering it. In another branch I've added a fix where it skips entries where |
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 @sumanthgenz. It works now. A few comments.
Also:
» sky cost-report
Clusters
NAME LAUNCHED DURATION RESOURCES STATUS HOURLY_PRICE COST (est.)
llama-ckpts 3 weeks ago 2 weeks 2 days 12h 54m... 1x GCP(n2-standard-4, disk_size=512) STOPPED $ 0.194 $77.094
dbg 1 day ago 19m 25s 1x Lambda(gpu_1x_a100_sxm4, {'A100': 1}) TERMINATED $ 1.100 $0.356
...
Managed spot controller (will be autostopped if idle for 10min)
NAME LAUNCHED DURATION RESOURCES STATUS HOURLY_PRICE COST (est.)
sky-spot-controller-8a3968f2 2 months ago 4 days 12h 41m 44s 1x GCP(n1-highmem-8, disk_size=50) TERMINATED $ 0.473 $51.436
Total Cost: $53.12
...
Two questions:
- Why does total cost show $53.12, while the
COST (est.)
add up to something bigger? - For the first cluster,
llama-ckpts
, the duration2 weeks 2 days 12h 54m...
seems too long (I don't remember its uptime being this long). Is it a known issue for clusters that might have utilized autostop?
if status is None: | ||
return -1 | ||
return 1 |
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.
Can we put this comment as comment before L172?
|
Thanks @sumanthgenz.
but
|
e6958fd
to
70f1d1e
Compare
|
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.
LGTM, thanks @sumanthgenz.
Limit lines in each table to 5 lines (this number is up for debate).
Display total cost across all reports in cost-report (even costs that are not displayed due to the above limit).