Skip to content

fix(llama.cpp): fall back to an even layer split when the split weights sum to zero - #421

Merged
bernardladenthin merged 2 commits into
mainfrom
claude/fix-metal-zero-free-split
Sep 9, 2026
Merged

fix(llama.cpp): fall back to an even layer split when the split weights sum to zero#421
bernardladenthin merged 2 commits into
mainfrom
claude/fix-metal-zero-free-split

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

Summary

  • Adds patches/0012, which fixes the macOS-arm64 model-load failure red since the b10792→b10797 step: a GPU reporting zero free memory made load_tensors divide by zero, putting NaN in every layer split point and throwing std::out_of_range — whose libc++ what() is the bare string "vector", printed as the unactionable error loading model: vector.
  • The same line is reachable without any GPU memory pressure and on any backend: --tensor-split values are never range-checked, so -ts 1,-1 cancels out and lands on it too. Verified against the unfixed library, not reasoned about.
  • Ships both an upstream test (tests/test-model-split.cpp, 5 cases) and a runnable project-side guard (src/test/cpp/test_model_split.cpp, 7 cases), because a FetchContent subproject builds with LLAMA_BUILD_TESTS=OFF and would never compile the upstream one.

Root cause

llama_model_base::load_tensors weights the per-device layer split by ggml_backend_dev_memory()'s free, then normalises with splits[i] /= split_sum. When the weights sum to zero that is 0/0:

  1. every split point becomes NaN;
  2. NaN compares false against everything, so std::upper_bound returns the end iterator;
  3. layer_gpu == n_devices(), and devices.at(layer_gpu) throws std::out_of_range;
  4. libc++ renders that as what() == "vector", which the model-load catch prints verbatim.

Why it started at b10618..b10797. Upstream 8c0b9cd04 ("metal : fix memory query under low-memory conditions") changed the Metal query to *free = *total > cur ? *total - cur : 0. Before that clamp, an over-committed device (currentAllocatedSize > recommendedMaxWorkingSetSize) underflowed to a huge size_t, which normalised harmlessly. The free == 0 && total == 0 host-memory fallback does not fire, because total is recommendedMaxWorkingSetSize and is non-zero.

Why only macOS went red. act_gpu_layers is devices.empty() ? 0 : …, so with no GPU backend every layer returns early on cpu_dev and the .at() line is unreachable. That is the Metal / no-Metal discriminator seen in run 33994480652.

The second trigger, which is what makes this a general defect. common/arg.cpp parses --tensor-split with std::stof and never range-checks the values, so -ts 1,-1 cancels: split_sum is 0 again, the split points become [inf, -nan], and every layer maps one past the last device — on CUDA, Vulkan or ROCm as much as on Metal, in a fresh process. Reproduced here against the unfixed library.

The change

Both blocks move out of load_tensors into free functions declared in src/llama-model.h, purely so they can be tested — the failing state needs a real over-committed GPU and cannot be arranged through any public API.

Function Role
llama_model_splits_normalize() the fix — on split_sum == 0, warn and split evenly; the only neutral choice when no device can be preferred, and exactly right for a single device
llama_model_splits_select_device() the diagnostic — bounds-check the lookup and throw a message naming the function, the layer, the device index, the split count and the split values

A recurrence now reads

llama_model_splits_select_device: offloaded layer 0 of 32 mapped to device index 1,
but there are only 1 split point(s) [-nan] -- the split points are malformed

instead of vector. The [-nan] names the cause outright — which is precisely what was missing when this had to be diagnosed by reading source.

Test plan

  • Affected unit / integration tests pass locally
  • CI is green on this branch
  • Docs / CHANGELOG updated where applicable

Run locally on a fresh rm -rf llama/build, so the applier re-applied all 9 patches from a pristine fetch:

Gate Result
Patch applier 9/9 clean, stamp written
ctest 527/527 (the 7 new tests included)
mvn clean verify 1742 tests, 0 failures, 0 errors
Bytecode gate 616 classes / 5 jars, 0 over major 52
Shipped libjllama.so both new strings present
Upstream test, standalone-linked 5 cases / 22 assertions, green

Both guards were falsified, not just run: with the fix removed and llama rebuilt, the upstream test fails 12 of 23 assertions and the gtest guard 2 of 6, each printing the NaN split points through the new message. The -ts 1,-1 shape was falsified the same way and yields [inf, -nan].

What this does not prove

The failing path still needs a GPU backend — on Linux devices is empty and the line is unreachable — so these runs prove the arithmetic, which is what broke, not the end-to-end load. The macOS CI job on this branch is the end-to-end gate. This is stated in CLAUDE.md too, so a later reader does not over-trust the local green.

Related issues / PRs

  • Refs the investigation rows in docs/history/llama-cpp-breaking-changes.md (2026-09-05 / -06), whose open question "which commit, and has upstream fixed it?" this closes: 8c0b9cd04, and no — the bare splits[i] /= split_sum is still there at b10850.
  • Upstream-submittable; not yet filed with ggml-org/llama.cpp.

Checklist

  • I have read CONTRIBUTING.md and CODE_OF_CONDUCT.md
  • My commits follow Conventional Commits
  • No security-sensitive changes

🤖 Generated with Claude Code

https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH


Generated by Claude Code

…ts free memory

Adds patches/0012, which fixes the macOS-arm64 model-load failure that has been red
since the b10792 -> b10797 step, and makes the class of failure that produced it
diagnosable if it ever recurs.

Root cause. load_tensors() weights the per-device layer split by
ggml_backend_dev_memory()'s `free`, then normalises with `splits[i] /= split_sum`.
Upstream 8c0b9cd04 ("metal : fix memory query under low-memory conditions",
ggml-org/llama.cpp#27701), which lies inside the b10618..b10797 window, clamped the
Metal query to `*free = *total > cur ? *total - cur : 0`. An over-committed device
(currentAllocatedSize past recommendedMaxWorkingSetSize) previously underflowed to a
huge size_t, which normalised harmlessly; it now reports exactly 0. Upstream's
`free == 0 && total == 0` host-memory fallback does not fire, because total is
recommendedMaxWorkingSetSize and is non-zero. So split_sum is 0, every split point
becomes NaN, std::upper_bound returns the end iterator (NaN compares false against
everything), and devices.at(n_devices()) throws std::out_of_range -- whose libc++
what() is the bare string "vector", printed verbatim as "error loading model: vector".

Only a GPU build can reach it: act_gpu_layers is `devices.empty() ? 0 : ...`, so with
no GPU backend every layer returns early on cpu_dev and the .at() line is unreachable.
That is the Metal / no-Metal discriminator observed in run 33994480652.

The change. Both blocks move out of load_tensors into free functions declared in
src/llama-model.h, purely so they can be tested -- the failing state needs a real
over-committed GPU and cannot be arranged through any API.
llama_model_splits_normalize() carries the fix: on split_sum == 0 it warns and splits
evenly, the only neutral choice when no device can be preferred, and exactly right for
a single device. llama_model_splits_select_device() carries the diagnostic: it
bounds-checks the lookup and throws a message naming the function, the layer, the
index, the split-point count and the split points themselves, so a recurrence reads
"[-nan] -- the split points are malformed" instead of "vector".

Tests. The patch ships upstream tests/test-model-split.cpp (4 cases, 20 assertions).
A FetchContent subproject builds with LLAMA_BUILD_TESTS=OFF, so that file is
applied-but-never-compiled here; src/test/cpp/test_model_split.cpp therefore drives the
same two functions from jllama_test, which every platform runs in C++ Tests. A bump
that drops the patch now fails the build everywhere instead of resurfacing as one red
macOS Java job.

Verified: fresh rm -rf build, all 9 patches applied clean from a pristine fetch; ctest
526/526 including the 6 new tests; both new strings present in the shipped
libjllama.so; mvn clean verify green (1742 tests, 0 failures, 0 errors); bytecode gate
clean (616 classes / 5 jars, 0 over major 52). Both guards were falsified by removing
the fix and rebuilding: the upstream test then fails 12 of 23 assertions and the gtest
guard 2 of 6, each printing the NaN split points through the new message.

Not proven locally: the failing path itself needs a GPU backend, so these runs prove
the arithmetic -- which is what broke -- not the end-to-end load. The macOS CI job is
the end-to-end gate.

Upstream-submittable; not yet filed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
…um path

Follow-up to the patch in the previous commit, from checking whether it is fit to
submit upstream. It is, and for a stronger reason than the macOS failure that
prompted it.

--tensor-split values are parsed with std::stof and never range-checked
(common/arg.cpp), so `-ts 1,-1` makes the weights cancel and reaches the exact
same line: split_sum is 0, the split points become [inf, -nan], std::upper_bound
returns the end iterator and every layer maps one past the last device. Verified
against the unfixed library, not reasoned about: it reproduces on any multi-GPU
backend -- CUDA, Vulkan, ROCm as much as Metal -- in a fresh process with no
memory pressure at all. The macOS-15 failure is one instance of a general defect,
not a Metal edge case.

Two things in the patch were wrong given that, and are fixed here:

  * the warning claimed "no device reported any free memory", which is simply
    untrue when the user passed explicit weights that cancel. It now names both
    causes, as does the comment above it.
  * neither test suite covered the second trigger, so the cancelling shape could
    have regressed silently. Both now pin it (upstream 4 -> 5 cases, the project
    guard 6 -> 7).

Verified: ctest 527/527; the upstream test standalone-links and passes at
5 cases / 22 assertions; the new warning string is present in the shipped
libjllama.so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Summary

Strong fix with excellent root-cause analysis and defensive coding.

Key Strengths:

  • Correctly identified upstream commit 8c0b9cd04 as root cause
  • Discovered both metal memory AND --tensor-split parsing triggers
  • Two-part fix: normalize() + select() with good error messages
  • Comprehensive tests (7 cases covering all scenarios)
  • Exceptional documentation in CLAUDE.md

Code Quality: No issues found. Format string is literal (safe), float equality check is correct, bounds checking is solid.

Pre-merge: Verify CI passes for macOS-15 Metal jobs before merge.

@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@bernardladenthin
bernardladenthin merged commit 7792a00 into main Sep 9, 2026
9 of 17 checks passed
@bernardladenthin
bernardladenthin deleted the claude/fix-metal-zero-free-split branch September 9, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants