docs: document unified memory env configuration - #1051
amarkdotdev wants to merge 3 commits into
Conversation
Fixes docker#994 Signed-off-by: Aaron <amark@g.jct.ac.il>
Fixes docker#994 Signed-off-by: Aaron <amark@g.jct.ac.il>
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docs/unified-memory.md" line_range="20" />
<code_context>
+## docker model run
+
+```shell
+docker model run -e GGML_CUDA_ENABLE_UNIFIED_MEMORY=1 ai/gemma3 "Hello"
+```
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The documented `docker model run -e GGML_CUDA_ENABLE_UNIFIED_MEMORY=1 ...` command fails because this repository's `run` command does not define an `-e`/`--env` flag, so Cobra rejects the option instead of configuring the runner.
**Triggers:** When users follow the Docker Model Runner CLI example.
**Suggested fix:** Set the variable in the environment before invoking `docker model run`, or document a supported runner/container configuration mechanism instead of passing `-e` to `docker model run`.
```suggestion
GGML_CUDA_ENABLE_UNIFIED_MEMORY=1 docker model run ai/gemma3 "Hello"
```
</issue_to_address>
### Comment 2
<location path="docs/unified-memory.md" line_range="1" />
<code_context>
+# Unified memory configuration for integrated GPUs
+
+Docker Model Runner uses llama.cpp under the hood. On systems with integrated GPUs (for example AMD APUs), available shared memory may be reported incorrectly unless unified memory is enabled.
</code_context>
<issue_to_address>
**nitpick:** The new unified-memory document is not linked from `README.md` or any existing documentation index, so the repository's stated README-link verification is false and users browsing the README cannot discover this configuration guide.
**Suggested fix:** Add a README link to `docs/unified-memory.md`, preferably alongside the existing documentation resources.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Use env-prefix syntax for docker model run instead of invalid -e flag. Add unified-memory.md to README Additional Resources. Signed-off-by: Aaron <amark@g.jct.ac.il>
|
@amarkdotdev I recommend checking out: https://github.com/llmmanorg/llmman It's not annouced yet, but this is going into maintenance mode |
|
Maintainer review welcome whenever you have a moment to spare. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The examples apply a CUDA-only setting to the default Vulkan image and need to be scoped or corrected.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Documents unified-memory configuration for Docker Model Runner and links the guide from the README.
Changes:
- Adds Compose,
docker model run, anddmrexamples. - Links
docs/unified-memory.mdfromREADME.md.
| File | Summary |
|---|---|
README.md |
Links to the unified-memory guide. |
docs/unified-memory.md |
Documents unified-memory environment configuration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| model-runner: | ||
| image: docker/model-runner:latest | ||
| environment: | ||
| GGML_CUDA_ENABLE_UNIFIED_MEMORY: "1" |
| ## docker model run | ||
|
|
||
| ```shell | ||
| GGML_CUDA_ENABLE_UNIFIED_MEMORY=1 docker model run ai/gemma3 "Hello" |
|
Docs look good overall, but please confirm the env var actually propagates to the daemon/backend for all three invocation styles shown, and scope the doc to CUDA/HIP since Vulkan is the default runtime for most users. Also, model-runner is being deprecated in favor of llmman, please open future docs PRs there instead. Marking as draft, please mark ready for review once addressed. |


Summary
Fixes #994
Test plan