Automatically use ROCm when appropriate by ericcurtin · Pull Request #328 · docker/model-runner

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.

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

+	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

+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

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


Suggested change

package gpuinfo

package gpuinfo

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

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


Suggested change

}

}

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

 ) 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.

Copilot

Copilot review overview

🟡 Changes recommended

Critical duplicate declarations break builds, and the ROCm artifact path mismatch prevents successful installation.

Review effort: Lite
Findings: 3

Open (3)

Duplicate HasSupportedAMDGPU method on non-Linux builds · New
Duplicate AMD detector with CGO disabled on Linux · New
ROCm artifact path omits version and breaks AMD GPU installation · New

What changed in this PR

Adds Linux AMD GPU detection and automatic ROCm llama.cpp selection.

Changes:

Detects supported AMD GPUs via KFD topology.
Selects CPU or ROCm variants on Linux.
Adds platform-specific fallbacks and status logging.

File	Summary
`pkg/inference/backends/llamacpp/download_linux.go`	Selects and downloads CPU or ROCm variants.
`pkg/gpuinfo/memory_windows.go`	Adds Windows fallback behavior.
`pkg/gpuinfo/memory_linux_nocgo.go`	Adds Linux no-cgo memory fallback.
`pkg/gpuinfo/memory_darwin_nocgo.go`	Adds Darwin no-cgo fallback.
`pkg/gpuinfo/memory_darwin_cgo.go`	Adds Darwin cgo fallback.
`pkg/gpuinfo/gpuinfo.go`	Exposes AMD GPU detection.
`pkg/gpuinfo/gpuinfo_not_linux.go`	Adds non-Linux fallback behavior.
`pkg/gpuinfo/gpuinfo_linux.go`	Adds Linux GPU targeting.
`pkg/gpuinfo/amd_gpu_linux.go`	Implements Linux AMD GPU detection.
`main.go`	Logs detected AMD GPU support.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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


+// hasSupportedAMDGPU returns true if the system has supported AMD GPUs
+func hasSupportedAMDGPU() (bool, error) {
+	return false, errors.New("unimplemented without cgo")
+}


+	// Use ROCm if supported AMD GPU is detected
+	if hasAMD {
+		log.Info("Supported AMD GPU detected, using ROCm variant")
+		desiredVariant = "rocm"
+	}


Copilot

Copilot review overview

🟡 Changes recommended

Critical ROCm runtime and multiple platform/build compatibility issues remain unresolved.

Review effort: Lite
Findings: 4

Open (4)

Ensure ROCm runtime libraries are available before selection · New
ROCm artifact path omits version and breaks AMD GPU installation
Duplicate AMD detector with CGO disabled on Linux
Duplicate HasSupportedAMDGPU method on non-Linux builds

+	// Use ROCm if supported AMD GPU is detected
+	if hasAMD {
+		log.Info("Supported AMD GPU detected, using ROCm variant")
+		desiredVariant = "rocm"


Copilot

Copilot review overview

🔵 Needs a closer look

Duplicate declarations can break builds, and ROCm selection has inconsistent artifact naming and redundant detection.

Review effort: Lite
Findings: 4

Open (4)

Ensure ROCm runtime libraries are available before selection
ROCm artifact path omits version and breaks AMD GPU installation
Duplicate AMD detector with CGO disabled on Linux
Duplicate HasSupportedAMDGPU method on non-Linux builds

Copilot

Copilot review overview

🟡 Changes recommended

Four unresolved moderate issues affect build correctness, detection consistency, and ROCm artifact selection.

Review effort: Lite
Findings: 4 · 1

Open (5)

Ensure ROCm runtime libraries are available before selection
ROCm artifact path omits version and breaks AMD GPU installation
Duplicate AMD detector with CGO disabled on Linux
Duplicate HasSupportedAMDGPU method on non-Linux builds
Redundant GPU detection performs unused I/O · New

+	// Auto-detect GPU type and set appropriate variant
+	// Check if we have supported AMD GPUs and set ROCm variant accordingly
+	if hasAMD, err := gpuInfo.HasSupportedAMDGPU(); err == nil && hasAMD {
+		log.Info("Supported AMD GPU detected, ROCm will be used automatically")
+		// This will be handled by the llama.cpp backend during server download
+	} else if err != nil {
+		log.Debugf("AMD GPU detection failed: %v", err)
+	}


Copilot

Copilot review overview

🔵 Needs a closer look

Duplicate definitions cause platform build failures, and ROCm artifact/runtime handling prevents reliable automatic selection.

Review effort: Lite
Findings: 4 · 1

Open (5)

Ensure ROCm runtime libraries are available before selection
ROCm artifact path omits version and breaks AMD GPU installation
Duplicate AMD detector with CGO disabled on Linux
Duplicate HasSupportedAMDGPU method on non-Linux builds
Redundant GPU detection performs unused I/O

ericcurtin · 2026-09-29T03:05:06Z

Closing, conflicts and the direction has moved on. Thanks for the reviews!

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

ericcurtin requested a lite review from Copilot September 26, 2026 19:00

Copilot started reviewing on behalf of ericcurtin September 26, 2026 19:01 View session

Copilot AI reviewed Sep 26, 2026

View reviewed changes

ericcurtin requested a lite review from Copilot September 27, 2026 07:00

Copilot started reviewing on behalf of ericcurtin September 27, 2026 07:01 View session

Copilot AI reviewed Sep 27, 2026

View reviewed changes

Comment thread pkg/inference/backends/llamacpp/download_linux.go

Comment on lines +41 to +44

// Use ROCm if supported AMD GPU is detected

if hasAMD {

log.Info("Supported AMD GPU detected, using ROCm variant")

desiredVariant = "rocm"

ericcurtin requested a lite review from Copilot September 27, 2026 19:00

Copilot started reviewing on behalf of ericcurtin September 27, 2026 19:01 View session

Copilot AI reviewed Sep 27, 2026

View reviewed changes

ericcurtin requested a lite review from Copilot September 28, 2026 07:00

Copilot started reviewing on behalf of ericcurtin September 28, 2026 07:01 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

ericcurtin requested a lite review from Copilot September 28, 2026 19:00

Copilot started reviewing on behalf of ericcurtin September 28, 2026 19:01 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

ericcurtin closed this Sep 29, 2026

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

		package gpuinfo No newline at end of file

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

ericcurtin commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development