fix(local-ai): use CUDA for allocatable GPU memory - #1253
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
cd3a6d6 to
c6765a7
Compare
|
Codex review: needs real behavior proof before merge. Reviewed August 28, 2026, 11:34 AM ET / 15:34 UTC. ClawSweeper reviewWhat this changesThe PR replaces NVML/DXGI-based Local AI GPU qualification with direct CUDA-driver probing and migrates affected persisted GPU selectors. Regression provenancePossible regression — probable (reviewed change; failure trace). No predecessor PR is attributed. Merge readiness⛔ Blocked until stronger real behavior proof is added - 9 items remain Keep this PR open for correction. It can qualify WDDM systems for models that later fail allocation, and its partial GPU-ID migration makes existing verified Local AI installs terminally unreusable. Priority: P1 Review scores
Verification
How this fits togetherLocal AI setup probes NVIDIA hardware, selects a model, persists the chosen GPU in an installation manifest, then launches llama-server with that GPU selector. The reported capacity controls whether setup downloads and attempts to load a full-offload model. flowchart LR
A[NVIDIA driver] --> B[GPU hardware probe]
B --> C[Model eligibility]
C --> D[Local AI install manifest]
D --> E[llama-server launch]
E --> F[GPU allocation and inference]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep a conservative dedicated-memory eligibility baseline on WDDM, prove any UMA exception with successful full model allocation, and preserve verified UUID-backed manifests through an explicit compatibility path. Do we have a high-confidence way to reproduce the issue? Yes. On a Windows WDDM system where CUDA reports a larger total than llama-server can allocate, run the selected full-offload setup recipe; the linked hardware report records this outcome, although it was not executed in this review environment. Is this the best way to solve the issue? No. The related WDDM allocation trace shows CUDA total is not a safe standalone eligibility signal, so the fix needs a conservative allocation policy and an explicit legacy-selector compatibility path. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 56a169f5b71e. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
c6765a7 to
aff93c1
Compare
Query the CUDA driver directly for device identity and total/free allocatable memory instead of combining NVML and DXGI accounting. This should qualify both discrete RTX and UMA devices using the same allocator-visible source of truth. Closes openclaw#1191. Related: Dallin's openclaw#1237 and Pedro's openclaw#1239.
f84912d to
effc11a
Compare
| { | ||
| if (!OperatingSystem.IsWindows() || CuInit(0) != CudaSuccess || | ||
| CuDeviceGetCount(out int count) != CudaSuccess) | ||
| { |
There was a problem hiding this comment.
we should surface an error to the user here if cuda is not found or can't be initialized
| : null; | ||
| var gpus = new List<GpuInfo>(); | ||
| for (int ordinal = 0; ordinal < count; ordinal++) | ||
| { |
There was a problem hiding this comment.
Suggest to refactor this in a more functional idiom:
private static IReadOnlyList<GpuInfo> CaptureCudaGpus()
{
if (!OperatingSystem.IsWindows() || CuInit(0) != CudaSuccess ||
CuDeviceGetCount(out int count) != CudaSuccess)
{
return [];
}
int? cudaMajorVersion =
CuDriverGetVersion(out int driverVersion) == CudaSuccess && driverVersion > 0
? driverVersion / 1000
: null;
return Enumerable.Range(0, count)
.Select(ordinal => TryCaptureGpu(ordinal, cudaMajorVersion))
.Where(gpu => gpu is not null)
.Select(gpu => gpu!)
.ToList();
}
private static GpuInfo? TryCaptureGpu(int ordinal, int? cudaMajorVersion)
{
if (CuDeviceGet(out int device, ordinal) != CudaSuccess)
return null;
string? name = ReadDeviceName(device);
string? pciBusId = ReadPciBusId(device);
if (string.IsNullOrWhiteSpace(name) || string.IsNullOrWhiteSpace(pciBusId))
return null;
return WithCudaContext(device, () =>
{
if (CuMemGetInfo(out nuint freeBytes, out nuint totalBytes) != CudaSuccess ||
totalBytes == 0 || totalBytes > long.MaxValue || freeBytes > totalBytes)
{
return null;
}
return new GpuInfo(
GpuVendor.Nvidia,
name,
GpuVisibleMemoryBytes: (long)totalBytes,
FreeGpuVisibleMemoryBytes: (long)freeBytes,
CudaMajorVersion: cudaMajorVersion,
StableId: ToCudaVisibleDevicesSelector(pciBusId));
});
}
private static GpuInfo? WithCudaContext(int device, Func<GpuInfo?> action)
{
if (CuCtxCreate(out IntPtr context, 0, device) != CudaSuccess)
return null;
try
{
return action();
}
finally
{
_ = CuCtxDestroy(context);
}
}
Summary
Closes #1191.
Related work: Dallin's investigation in #1237 and Pedro's follow-up in #1239. Rather than adding or excluding DXGI shared memory based on adapter type, CUDA itself supplies the capacity that its allocator can use. This should handle ordinary discrete RTX GPUs and unified-memory NVIDIA systems with one source of truth.
Validation
LocalInferenceQualificationTests: 14 passedLocalAiGpuVerificationTestsandSetupPipelineTests: 33 passed