Skip to content

QVAC-24253 feat[api]: expose split-mode 'tensor' in llm-llamacpp - #4121

Open
iancris wants to merge 2 commits into
mainfrom
QVAC-24253/tensor-split-mode
Open

QVAC-24253 feat[api]: expose split-mode 'tensor' in llm-llamacpp#4121
iancris wants to merge 2 commits into
mainfrom
QVAC-24253/tensor-split-mode

Conversation

@iancris

@iancris iancris commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

⛔ DO NOT MERGE AS-IS

The second commit (bf394e8f1) is a vcpkg validation overlay pinning qvac-fabric to the head of
qvac-fabric-llm.cpp#214 — an unmerged PR branch commit.
Merging it would make every consumer resolving through packages/llm-llamacpp/vcpkg-configuration.json build fabric
from that commit instead of the registry.

Revert bf394e8f1 before merging. The first commit (15091b91e) is the change to land.

🎯 Problem

@qvac/llm-llamacpp ships pipeline parallelism (split-mode: 'layer') and accepts the legacy 'row' mode, which it degrades to 'layer' because no shipped backend provides split buffers. It does not expose LLAMA_SPLIT_MODE_TENSOR — qvac-fabric's meta-device tensor parallelism, which splits weights and KV cache across every visible GPU.

docs/multi-gpu.md said so outright: "CUDA … moved tensor parallelism to a separate LLAMA_SPLIT_MODE_TENSOR, which this package does not expose." So on every backend this package ships, no form of real tensor parallelism was reachable — row is inert and tensor did not exist.

Asana: QVAC-24253

📝 How

2 commits, 14 files — 11 under packages/llm-llamacpp/, plus 3 of overlay scaffolding.

Commit 1 — 15091b91e, the change to land

Core (addon/src/model-interface/LoadFitNormalization.cpp) — parse 'tensor'LLAMA_SPLIT_MODE_TENSOR, plus one consolidated block enforcing what fabric requires of the mode, each rejected up front with InvalidArgument instead of surfacing as an opaque native failure:

  • Flash attention required — fabric returns a null context otherwise. Both flash-attn and flash_attn spellings are checked; the underscore form is never normalised into the hyphen key when the caller sets it directly, so reading only one would let off through.
  • Auto-fit disabledcommon_params_fit refuses SPLIT_MODE_TENSOR, and common_fit_params swallows that into a WARN that common_init_result ignores, so the load proceeds unfitted either way. Disabling it explicitly makes the logs say what is actually happening. Logged at INFO.
  • Architecture allow-listllm_arch_supports_sm_tensor lives in fabric's internal src/llama-arch.h, outside the installed include tree, so the addon cannot call it. The list is mirrored and pinned by a test that fails if a fabric bump changes it; fabric's own check remains the backstop.

Placement — the block sits after tuneLoadConfigMap, not inside the GPU branch: that call applies the flash-attn defaults (on by default, off for BitNet), so it is the first point where the effective value can be read. It keys on params.split_mode, so a tensor request that fell back to CPU correctly skips every check.

Deliberately unchanged — the row-split probe stays ROW-only. SPLIT_MODE_TENSOR needs no split buffers; routing it through gpuBackendSupportsRowSplit() would silently degrade it to 'layer' on every shipped backend. A comment and a test pin that.

API (additive) — src/index.ts widens the split-mode union; index.d.ts regenerated.

Docsdocs/multi-gpu.md retitled "Tensor parallelism is unavailable in shipped builds" → "Two kinds of tensor parallelism" (both anchor links updated), new backend column, decision tree, usage example, fallback rows, strategy table. README.md config table. examples/multiGpuBenchmark.js gains a fourth mode.

Commit 2 — bf394e8f1, revert before merge

Pins qvac-fabric to 1d14f65 (head of fabric#214, "QVAC-24112 fit: budget against real memory availability") so this PR's tensor work is compiled and tested against that fit change before either lands — relevant because tensor mode disables auto-fit and #214 rewrites how fit budgets device memory.

Follows the /rollout-phase-a pattern with one narrowing: the overlay-ports key is added to packages/llm-llamacpp only, not all 7 fabric consumers. default-registry.baseline is not bumped and version>= stays at 10297.0.0. The overlay version is 10297.214.0 rather than the registry's 10297.0.0 on purpose — a wrong overlay path does not error, it silently falls back to the registry and validates the OLD fabric, so the distinct version is what makes the check falsifiable.

Not in this PR — multi-host TP over RPC (GGML_RPC is OFF in the shipped fabric build), changes to packages/model-fit, and performance tuning of the meta backend's all-reduce.

🧪 Tested

Base origin/main @ 6c495a608. macOS arm64 (M4), Apple clang, BUILD_TESTING=ON, Release — against fabric#214 via the overlay, not the registry version. Overlay confirmed live: qvac-fabric[core,gpu-backends,llama]:arm64-osx@10297.214.0, built from source.

  • ./addon-test --gtest_filter='*TensorSplit*'12/12 pass: parse + fit disabled; fit left enabled for none/layer/row; underscore key and uppercase; row probe not invoked; never degraded; flash-attn: off and flash_attn: off both rejected; flash-attn on/unset accepted; unsupported architecture rejected by name; supported architecture accepted; CPU fallback clears to NONE; the 30-entry denylist pin; invalid-value error text.
  • Regression pass over LoadFitNormalizationTest.* : LoadFitSnapshotTest.* : TuneConfigMap* : *BackendSelection*213/213 pass.
  • npm run typecheck, test:types:consumer, lint:js, lint:ts, check:generated — clean.
  • git-clang-format --diff (the exact command cpp-lint runs) — clean.

Not run locally: the model-level test (needs the multi-GB GGUF set, covered by cpp-tests) and the two new multi-GPU integration cases (gated on QVAC_HAS_MULTI_GPU=1, which no workflow sets — they need a two-GPU host and remain unexecuted).

run-cpp-addon-tests applied; the cpp-tests lane compiles the addon, so the overlay is exercised on darwin-arm64, linux-x64 and windows-x64.

⚠️ Breaking

None. Purely additive: a new accepted value for an existing optional config key. Existing split-mode values behave exactly as before, row still degrades to layer, and the mobile rejection is unchanged. package.json untouched — the version bump and release heading are left to the release flow.

The overlay commit is the one thing that must not reach main; see the banner above.

🔌 API Changes

// packages/llm-llamacpp/src/index.ts — LoadConfig
- "split-mode"?: "none" | "layer" | "row";
+ "split-mode"?: "none" | "layer" | "row" | "tensor";

Pinned by a type test asserting 'tensor' is assignable and the union stays closed — test/types/consumer-cjs.test-d.ts carries a @ts-expect-error on an invalid mode, which stops compiling if the union is ever widened to string.

'tensor' is EXPERIMENTAL and desktop-only. It requires flash attention, disables auto-fit (set ctx_size explicitly for large models, or the load can OOM where auto-fit would have trimmed the context), and is unavailable for some architectures. Fabric documents the mode as experimental and expects good performance mainly on multi-GPU CUDA, which this package does not ship — every shipped backend takes the meta backend's generic fallback reduction rather than a tuned all-reduce. Correct, but not assumed faster than 'layer' without measurement.

🤖 Generated with Claude Code


@iancris
iancris requested review from a team as code owners August 27, 2026 18:05
@iancris iancris added run-cpp-addon-tests CI: run C++ addon tests (requires verified) NLP llm and embed labels Aug 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Review Status

Current Status: ❌ PENDING
Approvals so far: none

Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member.

@github-actions

Copy link
Copy Markdown
Contributor

License compliance — clean

No new dependency license findings in this PR.

Warn-only (shadow) mode — this check does not block merges yet.

Updated automatically by the canonical license compliance workflow.

NOTICE presence (advisory)

Missing NOTICE (advisory, does not block):

  • ./.github/actions/release-merge-guard
  • ./docs/website
  • ./packages/ggml-coload-smoke
  • ./packages/fabric/test/integration
  • ./packages/inference-addon-cpp/mobile
  • ./packages/sdk/e2e
  • ./packages/llm-llamacpp/benchmarks/performance
  • ./packages/llm-llamacpp/benchmarks/server
  • ./packages/vla-ggml/sim/server
  • ./packages/embed-llamacpp/benchmarks/performance
  • ./packages/embed-llamacpp/benchmarks/server
  • ./packages/asr-ggml/benchmarks/server

@iancris iancris removed NLP llm and embed run-cpp-addon-tests CI: run C++ addon tests (requires verified) labels Aug 27, 2026
@iancris
iancris force-pushed the QVAC-24253/tensor-split-mode branch from f41f5a5 to 45a83e4 Compare August 27, 2026 18:13
iancris and others added 2 commits August 27, 2026 20:21
Adds `split-mode: 'tensor'`, selecting qvac-fabric's LLAMA_SPLIT_MODE_TENSOR —
meta-device tensor parallelism that splits weights and KV cache across every
visible GPU. The enum already exists in the pinned qvac-fabric 10297.0.0, so
this is addon-side exposure only: no fabric bump, no registry change.

Three fabric constraints are enforced up front with InvalidArgument rather than
being left to surface as an opaque native failure:

- Flash attention is required; fabric otherwise returns a null context. Both
  the `flash-attn` and `flash_attn` spellings are checked, because when the
  caller passes the underscore form none of tuneLoadConfigMap's branches fire
  and the value is never normalised into the hyphen key.
- Auto-fit is unavailable: common_params_fit refuses SPLIT_MODE_TENSOR, and
  common_fit_params swallows that into a WARN that common_init_result ignores,
  so the load proceeds unfitted either way. Disabled explicitly and logged at
  INFO so the logs say what is happening instead of reporting a fit failure.
- Some architectures are unsupported. llm_arch_supports_sm_tensor lives in
  fabric's internal src/llama-arch.h, outside the installed include tree, so
  the list is mirrored in the addon and pinned by a test that fails if a fabric
  bump changes it. Fabric's own check remains the backstop.

The constraint block sits after tuneLoadConfigMap, not in the GPU branch: that
call is what resolves flash-attn, so checking earlier would miss both the
default-on and the BitNet-off paths.

The row-split probe stays ROW-only — SPLIT_MODE_TENSOR needs no split buffers,
and routing it through that probe would silently degrade it to 'layer' on every
shipped backend.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scaffolding, not a change to land. REVERT THIS COMMIT BEFORE MERGING.

Adds a vcpkg overlay port pinning qvac-fabric to the head of
tetherto/qvac-fabric-llm.cpp#214 ("QVAC-24112 fit: budget against real
memory availability", base temp-10297) at 1d14f65, so this PR's
split-mode 'tensor' work is compiled and unit-tested against that fit
change before either lands. Relevant because tensor mode disables
auto-fit, and #214 rewrites how fit budgets device memory.

Follows the /rollout-phase-a overlay pattern with one deliberate
narrowing: the overlay-ports key is added to packages/llm-llamacpp only,
not to all 7 fabric consumers, because only this package needs
validating here. The other six keep resolving qvac-fabric from the
registry.

- vcpkg-overlays/ports/qvac-fabric/{portfile.cmake,vcpkg.json} copied
  verbatim from the registry port, with REF v${VERSION} replaced by the
  literal commit (no tag exists for a PR head) and the matching SHA512.
- Overlay version is 10297.214.0 rather than the registry's 10297.0.0 so
  the vcpkg install log distinguishes the two. A silently-wrong overlay
  path does not error — it falls back to the registry and validates the
  OLD fabric — so the distinct version is what makes the check honest.

default-registry.baseline is deliberately NOT bumped, and the consumer's
`version>=` is left at 10297.0.0; the overlay bypasses version
resolution and 10297.214.0 satisfies the existing pin either way.

Verified locally on macOS arm64: vcpkg reports
qvac-fabric[core,gpu-backends,llama]:arm64-osx@10297.214.0 built from
source, and addon-test passes 213/213 against it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@iancris
iancris force-pushed the QVAC-24253/tensor-split-mode branch from 45a83e4 to bf394e8 Compare August 27, 2026 18:36
@iancris iancris added the run-cpp-addon-tests CI: run C++ addon tests (requires verified) label Aug 27, 2026
@gianni-cor

Copy link
Copy Markdown
Contributor

Addon-side note: moving the tensor-mode params.fit_params = false enforcement to after the generic common-arg parsing looks feasible and would make the override authoritative.

Right now the PR disables fit before the remaining config map is converted into llama.cpp args and parsed. Since qvac-fabric exposes --fit [on|off], a caller can still pass e.g. { "split-mode": "tensor", fit: "on" }; the later parser then sets params.fit_params back to true, so tensor mode reaches qvac-fabric with auto-fit enabled again.

Suggested shape: keep the tensor-mode validation where it is if needed for the pre-parser flash-attn/default logic, but either move just the params.fit_params = false assignment/log to after the arg parsing loop, or add a second post-parse tensor block that forces it off. A small unit test with split-mode=tensor + fit=on would pin the behavior.

@gianni-cor

Copy link
Copy Markdown
Contributor

Could you also add an explicit flash-attention config field to the LLM addon typings, similar to the embedding addon, and propagate it through the SDK schema?

Right now index.d.ts mentions flash-attn: off only in the split-mode docs, but LlamaConfig has no narrow typed field for it; direct addon consumers only get it via the broad [key: string] escape hatch. The embed addon exposes this deliberately as flashAttention?: "on" | "off" | "auto", and the SDK schema also knows about that field.

For this PR, making flash attention part of the typed LLM surface would make the new tensor-mode requirement much clearer and testable. Please add the LLM addon field(s) ("flash-attn" and/or flash_attn, whichever spelling you want to support publicly), narrow it to the values qvac-fabric accepts, add type tests, and then update @qvac/inference / SDK llamacppCompletionConfigSchema so SDK loadModel callers can pass it too. That also lets the tensor-mode flash-attn=off rejection be exercised consistently through both direct-addon and SDK paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-cpp-addon-tests CI: run C++ addon tests (requires verified)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants