Allow running ramalama without a GPU
#1909
Conversation
kpouget
requested review from
rhatdan,
bmahabirbu,
maxamillion,
swarajpande5,
jhjaggars,
cgruver and
engelmi
as code owners
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR enables CPU-only execution by allowing users to specify Class diagram for updated device handling in RamaLama engineclassDiagram
class Engine {
+add_device_options()
-exec_args
-args
}
class Args {
+device: list[str]
}
Engine --> Args: uses
Args : device can be ['none']
Engine : add_device_options() skips device flags if device == ['none']
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 @kpouget, 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 enhances the ramalama tool by allowing users to explicitly opt out of GPU usage, even when a GPU is present on the system. This is achieved through a new --device none option, which prevents any device arguments from being passed to the underlying container runtime. This capability is essential for accurately testing and benchmarking CPU-only performance of ramalama in environments where GPUs might otherwise be automatically detected and utilized.
Highlights
- CPU-only Execution: Introduced the ability to run
ramalama run/servewithout a GPU by specifying--device none. - Enhanced Testing Capabilities: This feature is crucial for nightly testing of RamaLama's CPU-only performance on systems that are equipped with GPUs, ensuring accurate benchmarks.
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. ↩
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The check for
['none']via str(args.device) is brittle—compare the list directly (e.g.args.device == ['none']) or leverage argparse choices to handle a specialnonevalue. - You may want to explicitly error or warn if users pass multiple devices including 'none' to prevent ambiguous behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The check for `['none']` via str(args.device) is brittle—compare the list directly (e.g. `args.device == ['none']`) or leverage argparse choices to handle a special `none` value.
- You may want to explicitly error or warn if users pass multiple devices including 'none' to prevent ambiguous behavior.
## Individual Comments
### Comment 1
<location> `ramalama/engine.py:121` </location>
<code_context>
self.exec_args += ["-p", f"{host}{self.args.port}:{self.args.port}"]
def add_device_options(self):
+ request_no_device = str(getattr(self.args, "device", None)) == "['none']"
+ if request_no_device:
+ return
+
</code_context>
<issue_to_address>
The check for 'none' as device is fragile and may not handle all input cases.
Directly comparing to the string "['none']" is brittle. Instead, check if the device argument is a list containing 'none', or if 'none' appears in the list, to ensure the logic works for all input formats.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ramalama/engine.py
Outdated
| request_no_device = str(getattr(self.args, "device", None)) == "['none']" | ||
| if request_no_device: |
There was a problem hiding this comment.
issue: The check for 'none' as device is fragile and may not handle all input cases.
Directly comparing to the string "['none']" is brittle. Instead, check if the device argument is a list containing 'none', or if 'none' appears in the list, to ensure the logic works for all input formats.
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to run ramalama without a GPU by using the --device none flag. This is a useful feature for CPU-only testing. The implementation involves updating the CLI help text and adding logic to the engine to handle this new option. While the intent is clear, the implementation for checking the --device none option in ramalama/engine.py is brittle and can lead to errors in certain edge cases. I have provided a suggestion to make this check more robust and user-friendly.
ramalama/engine.py
Outdated
| request_no_device = str(getattr(self.args, "device", None)) == "['none']" | ||
| if request_no_device: | ||
| return |
There was a problem hiding this comment.
The check for request_no_device is brittle because it relies on the string representation of a list (str(...) == "['none']"). This approach has a few issues:
- Fragility: It depends on Python's internal
list.__str__implementation, which could theoretically change. - Incorrectness: It fails to handle cases where
--deviceis used multiple times. For example,ramalama run --device none --device none ...would result inself.args.devicebeing['none', 'none'], andstr(['none', 'none'])is not"['none']". The same issue occurs with... --device none --device /dev/kvm. In these cases, invalid arguments would be passed to the container engine, causing it to fail.
I suggest a more robust check that directly inspects the list of devices. This also provides a helpful warning to the user if they specify --device none along with other devices.
| request_no_device = str(getattr(self.args, "device", None)) == "['none']" | |
| if request_no_device: | |
| return | |
| devices = getattr(self.args, "device", None) | |
| if devices and "none" in devices: | |
| if len(set(devices)) > 1: | |
| logger.warning( | |
| "`--device=none` was specified with other device arguments, which will be ignored." | |
| ) | |
| return |
There was a problem hiding this comment.
fixed the "['none']" fragility
|
Please document this change in man pages. |
Useful for testing the CPU performance Signed-off-by: Kevin Pouget <[email protected]>
Signed-off-by: Kevin Pouget <[email protected]>
|
LGTM |
|
I probably should have made you add a test for this, so could you add a follow up with a test using --dryrun to make sure there are no --device options in the podman line. |
|
sure, done in #1911 👍🏻 |
Needs containers/ramalama#1909 to work. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a Ramalama “no‑GPU” platform option, available in presets and platform flavors for easier selection on systems without GPU access. * **Bug Fixes** * Non‑GPU runs now use a safe “none” device configuration for improved stability. * Automatically disables GPU when the selected inference server flavor is set to no‑GPU, preventing unintended GPU usage. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This PR adds the ability to run
ramalama run/serve --device none ..., and pass no device to the Podman command line.This will be useful for nightly testing of RamaLama CPU-only performance. Before this PR, it was not possible to run CPU-only on a system with a GPU.
Summary by Sourcery
Allow using '--device none' to skip passing any device to the container, enabling CPU-only runs.
New Features:
Documentation: