Skip to content
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

fix: Change resource display basis from Ratio to current amount #2686

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

ironAiken2
Copy link
Contributor

@ironAiken2 ironAiken2 commented Sep 4, 2024

TL;DR

After https://github.com/lablup/backend.ai-webui/pull/2640/files PR, With the use of ternary operators, the traditional string '/' has been marked as an arithmetic operation to represent Ratio.

Updated session list display to show absolute memory usage instead of percentage.

What changed?

  • Modified the memory usage display in the session list to show absolute values in GiB instead of percentages.
  • Removed the calculation that divided current memory usage by total capacity.
  • Adjusted the decimal precision for memory display from 2 to 1 for system memory and kept it at 2 for CUDA memory.
  • Added a console log statement to output row data for debugging purposes.

How to test?

  1. Navigate to the session list view.
  2. Create or observe existing sessions in 'batch', 'interactive', 'inference', or 'running' states.
  3. Verify that the memory usage is displayed as absolute values in GiB rather than percentages.
  4. Check that system memory is displayed with one decimal place and CUDA memory with two decimal places.

Why make this change?

This change provides users with a clearer understanding of actual memory consumption in sessions. Displaying absolute values instead of percentages allows for more accurate resource monitoring and helps users make informed decisions about their session management.


Checklist: (if applicable)

  • Mention to the original issue
  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

Copy link

graphite-app bot commented Sep 4, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added the size:S 10~30 LoC label Sep 4, 2024
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ironAiken2 and the rest of your teammates on Graphite Graphite

@ironAiken2 ironAiken2 requested a review from yomybaby September 4, 2024 03:44
@ironAiken2 ironAiken2 force-pushed the fix/modify-resources-_display-basis branch from 0299bfb to 89b8f56 Compare September 4, 2024 03:45
@ironAiken2 ironAiken2 marked this pull request as ready for review September 4, 2024 03:47
@ironAiken2 ironAiken2 force-pushed the fix/modify-resources-_display-basis branch from 89b8f56 to 3773f0b Compare September 4, 2024 04:01
@github-actions github-actions bot added size:M 30~100 LoC and removed size:S 10~30 LoC labels Sep 4, 2024
Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

graphite-app bot commented Sep 4, 2024

Merge activity

### TL;DR

> After https://github.com/lablup/backend.ai-webui/pull/2640/files PR, With the use of ternary operators, the traditional string '/' has been marked as an arithmetic operation to represent Ratio.

Updated session list display to show absolute memory usage instead of percentage.

### What changed?

- Modified the memory usage display in the session list to show absolute values in GiB instead of percentages.
- Removed the calculation that divided current memory usage by total capacity.
- Adjusted the decimal precision for memory display from 2 to 1 for system memory and kept it at 2 for CUDA memory.
- Added a console log statement to output row data for debugging purposes.

### How to test?

1. Navigate to the session list view.
2. Create or observe existing sessions in 'batch', 'interactive', 'inference', or 'running' states.
3. Verify that the memory usage is displayed as absolute values in GiB rather than percentages.
4. Check that system memory is displayed with one decimal place and CUDA memory with two decimal places.

### Why make this change?

This change provides users with a clearer understanding of actual memory consumption in sessions. Displaying absolute values instead of percentages allows for more accurate resource monitoring and helps users make informed decisions about their session management.

---

<!--
Please precisely, concisely, and concretely describe what this PR changes, the rationale behind codes,
and how it affects the users and other developers.
-->

**Checklist:** (if applicable)

- [ ] Mention to the original issue
- [ ] Documentation
- [ ] Minium required manager version
- [ ] Specific setting for review (eg., KB link, endpoint or how to setup)
- [ ] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after
@yomybaby yomybaby force-pushed the fix/modify-resources-_display-basis branch from 3773f0b to 7eab309 Compare September 4, 2024 04:23
@graphite-app graphite-app bot merged commit 7eab309 into main Sep 4, 2024
5 checks passed
@graphite-app graphite-app bot deleted the fix/modify-resources-_display-basis branch September 4, 2024 04:24
yomybaby pushed a commit that referenced this pull request Sep 4, 2024
### TL;DR

> After https://github.com/lablup/backend.ai-webui/pull/2640/files PR, With the use of ternary operators, the traditional string '/' has been marked as an arithmetic operation to represent Ratio.

Updated session list display to show absolute memory usage instead of percentage.

### What changed?

- Modified the memory usage display in the session list to show absolute values in GiB instead of percentages.
- Removed the calculation that divided current memory usage by total capacity.
- Adjusted the decimal precision for memory display from 2 to 1 for system memory and kept it at 2 for CUDA memory.
- Added a console log statement to output row data for debugging purposes.

### How to test?

1. Navigate to the session list view.
2. Create or observe existing sessions in 'batch', 'interactive', 'inference', or 'running' states.
3. Verify that the memory usage is displayed as absolute values in GiB rather than percentages.
4. Check that system memory is displayed with one decimal place and CUDA memory with two decimal places.

### Why make this change?

This change provides users with a clearer understanding of actual memory consumption in sessions. Displaying absolute values instead of percentages allows for more accurate resource monitoring and helps users make informed decisions about their session management.

---

<!--
Please precisely, concisely, and concretely describe what this PR changes, the rationale behind codes,
and how it affects the users and other developers.
-->

**Checklist:** (if applicable)

- [ ] Mention to the original issue
- [ ] Documentation
- [ ] Minium required manager version
- [ ] Specific setting for review (eg., KB link, endpoint or how to setup)
- [ ] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:M 30~100 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants