[Controller] feat: add busy threshold for global gpu controller input args - #39
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Wangmerlyn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new busy_threshold parameter to the global GPU controller, providing more precise control over GPU activity management. Concurrently, it refactors the utility for parsing human-readable memory sizes into a dedicated module, enhancing code modularity and reusability. These changes improve the configurability and maintainability of the GPU management system.
Highlights
- New Busy Threshold Parameter: A new
busy_thresholdparameter has been introduced to theGlobalGPUControllerand subsequently passed toCudaGPUControllerinstances. This parameter allows for more granular control over when a GPU is considered 'busy' and thus influences the controller's behavior. - Centralized Size Parsing Utility: The logic for parsing human-readable memory size strings (e.g., '10GiB', '2MB') has been extracted from
cuda_gpu_controller.pyinto a new, dedicated utility file:src/keep_gpu/utilities/humanized_input.py. This refactoring centralizes the parsing functionality, making it reusable and improving code organization. - Flexible VRAM Input: The
vram_to_keepparameter inGlobalGPUControllernow supports both integer (representing bytes) and string inputs. When a string is provided, it is parsed using the newhumanized_inpututility, offering more flexible configuration options.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds a busy_threshold parameter to the GPU controllers and refactors the size-parsing logic into a shared utility module. The refactoring is a good improvement for code reuse. My review includes suggestions to improve exception handling, remove redundant code, and clarify a magic number to enhance the code's robustness and maintainability.
| @staticmethod | ||
| def parse_size(text: str) -> int: | ||
| return parse_size(text) |
There was a problem hiding this comment.
This static method is a redundant wrapper around the imported parse_size function. It's cleaner to call the imported function directly where needed (e.g., in __init__). Removing this method will reduce a layer of indirection and simplify the class.
| @staticmethod | ||
| def parse_size(text: str) -> int: | ||
| text = text.strip().replace(" ", "") | ||
| m = re.fullmatch(r"([0-9]*\.?[0-9]+)([A-Za-z]*)", text) | ||
| if not m: | ||
| raise ValueError(f"invalid format: {text}, should be like '1000 MB'") | ||
| value, unit = m.groups() | ||
| unit = unit or "GB" | ||
| if len(unit) > 1: | ||
| unit = unit[:-1].upper() + unit[-1] | ||
| if unit not in _UNITS: | ||
| raise ValueError(f"unknown unit: {unit}, should be one of {_UNITS.keys()}") | ||
| return int(float(value) * _UNITS[unit] / 4) | ||
| return parse_size(text) |
There was a problem hiding this comment.
This static method is a redundant wrapper around the imported parse_size function. It would be cleaner to remove this method and call the imported parse_size function directly from __init__ (i.e., change self.parse_size(...) to parse_size(...)). This would reduce a layer of indirection and make the code more direct.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
No description provided.