Automatically use ROCm when appropriate #328

ericcurtin · 2025-10-27T23:59:13Z

On Linux at least

gemini-code-assist · 2025-10-27T23:59:37Z

Summary of Changes

Hello @ericcurtin, I'm Gemini Code Assist¹! 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 an automatic detection mechanism for supported AMD GPUs on Linux. Upon detecting a compatible AMD GPU, the system will automatically configure the llama.cpp backend to utilize the ROCm variant, enhancing performance for users with AMD hardware without requiring manual intervention. This change streamlines the setup process and ensures optimal GPU utilization where available.

Highlights

Automatic ROCm Detection: Implemented logic to automatically detect supported AMD GPUs on Linux systems by inspecting KFD topology and gfx_target_version identifiers.
Dynamic Backend Selection: The llama.cpp backend will now dynamically select and download the appropriate ROCm variant if a compatible AMD GPU is detected on Linux, streamlining setup for users.
Platform-Specific Implementation: The AMD GPU detection logic is specifically implemented for Linux, with placeholder functions for other operating systems to ensure cross-platform compatibility without unnecessary checks.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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.

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

Hey there - I've reviewed your changes - here's some feedback:

The environment-based GPU autodetection in init() can be surprising; consider moving that logic into an explicit configuration method or flag instead of a package init side-effect.
The GPU check in main.go duplicates the detection later in the backend; consider removing this duplicate scan to avoid redundant file system accesses.
Scanning /sys/class/kfd on every download invocation may impact performance; consider caching the detected GPU presence per process.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The environment-based GPU autodetection in init() can be surprising; consider moving that logic into an explicit configuration method or flag instead of a package init side-effect.
- The GPU check in main.go duplicates the detection later in the backend; consider removing this duplicate scan to avoid redundant file system accesses.
- Scanning /sys/class/kfd on every download invocation may impact performance; consider caching the detected GPU presence per process.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

gemini-code-assist

Code Review

This pull request adds automatic detection of AMD GPUs to use the ROCm variant on Linux. The changes are well-intentioned, but there are several critical issues that will prevent the code from building due to function and method redeclarations across different build tags. I've provided comments and suggestions to resolve these build-breaking issues. Additionally, the core GPU detection logic contains a critical bug that could lead to a panic, and there are several opportunities for improving code quality, readability, and performance, for which I've also left suggestions.

pkg/gpuinfo/amd_gpu_linux.go

pkg/gpuinfo/gpuinfo.go

pkg/gpuinfo/memory_linux_nocgo.go

main.go

pkg/gpuinfo/amd_gpu_linux.go

On Linux at least Signed-off-by: Eric Curtin <eric.curtin@docker.com>

doringeman

This means docker model install-runner --gpu none won't have effect anymore, right? Because we'll automatically update to the ROCm variant if it's supported.

doringeman · 2025-10-28T08:09:03Z

pkg/inference/backends/llamacpp/download_linux.go

+	ShouldUseGPUVariantLock.Lock()
+	defer ShouldUseGPUVariantLock.Unlock()
+	ShouldUseGPUVariant = true
+}


I'd rather have ShouldUseGPUVariant = runtime.GOOS == "linux" in download.go instead of an init.
For more context, this variable is used by Docker Desktop to determine whether it should display the GPU backed engine checkbox in the UI.

doringeman · 2025-10-28T08:10:28Z

pkg/gpuinfo/gpuinfo_not_linux.go

+func (g *GPUInfo) HasSupportedAMDGPU() (bool, error) {
+	// AMD GPU detection is only supported on Linux
+	return false, nil
+}


Suggested change

}

}

doringeman · 2025-10-28T08:10:42Z

pkg/gpuinfo/gpuinfo_linux.go

@@ -0,0 +1,3 @@
+//go:build linux
+
+package gpuinfo


Suggested change

package gpuinfo

package gpuinfo

doringeman · 2025-10-28T08:10:51Z

pkg/gpuinfo/amd_gpu_linux.go

+	}
+
+	return false, nil // No supported AMD GPU found
+}


Suggested change

}

}

p1-0tr · 2025-10-28T08:33:52Z

pkg/inference/backends/llamacpp/download_linux.go

 ) error {
-	l.status = fmt.Sprintf("running llama.cpp version: %s",
-		getLlamaCppVersion(log, filepath.Join(vendoredServerStoragePath, "com.docker.llama-server")))
-	return errLlamaCppUpdateDisabled


I think it'd be better to stick to a consistent logic for picking whether we use acceleration or not. Currently on LInux we expect people to run the model-runner container (appropriate to their usecase and system), so the accelerate/not-accelerate question should be decided in the CLI rather than here.

yeahdongcn · 2025-10-28T13:59:58Z

Would you like me to add logic to auto-detect musa as well?

ericcurtin · 2025-10-28T15:39:58Z

Would you like me to add logic to auto-detect musa as well?

Sure, see what we did for Nvidia. But only if it's not crazy or unreliable. Ideally we should be able to probe /proc, /sys or /dev to figure this out (I know that is not always possible though)

yeahdongcn · 2025-10-29T11:20:20Z

@ericcurtin Please see: #342

I will rebase main once your PR is merged.

sourcery-ai bot reviewed Oct 27, 2025

View reviewed changes

gemini-code-assist bot reviewed Oct 28, 2025

View reviewed changes

ericcurtin force-pushed the auto-rocm branch 3 times, most recently from 3d3badd to b7e9f39 Compare October 28, 2025 00:15

Automatically use ROCm when appropriate

83baea7

On Linux at least Signed-off-by: Eric Curtin <eric.curtin@docker.com>

ericcurtin force-pushed the auto-rocm branch from b7e9f39 to 83baea7 Compare October 28, 2025 00:16

doringeman reviewed Oct 28, 2025

View reviewed changes

p1-0tr reviewed Oct 28, 2025

View reviewed changes

		@@ -0,0 +1,3 @@
		//go:build linux

		package gpuinfo No newline at end of file

Automatically use ROCm when appropriate #328

Are you sure you want to change the base?

Automatically use ROCm when appropriate #328

Uh oh!

Conversation

ericcurtin commented Oct 27, 2025

Uh oh!

gemini-code-assist bot commented Oct 27, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doringeman left a comment

Choose a reason for hiding this comment

Uh oh!

doringeman Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

doringeman Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

doringeman Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

doringeman Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

p1-0tr Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

yeahdongcn commented Oct 28, 2025

Uh oh!

ericcurtin commented Oct 28, 2025

Uh oh!

yeahdongcn commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants