Merge pull request #525 from bernardladenthin/local/round1-diagnose-and-docs

A shipped diagnostic entry point, and the GPU jars' real requirements

Author: bernardladenthin

README.md

@@ -257,8 +257,8 @@ platforms you target, e.g. `cpu-linux-x86-64`.
 | `cpu-windows-aarch64` | CPU | Windows on ARM (Snapdragon X / Surface) | A JDK 8+ JVM; **Windows 10 or newer**. Built natively on `windows-11-arm` with `clang-cl`, same hybrid CRT as the x86-64 jar; one CPU backend module (ggml builds no ARM variant set for Windows), so the Vulkan and OpenCL module jars can join it. |
 | `metal-macos-aarch64` | Metal + CPU | macOS aarch64 (Apple silicon) | A JDK 8+ JVM. |
 | `cpu-android-aarch64` / `cpu-android-x86-64` | CPU | Android | For Android use the [`llama-android` AAR](#importing-in-android); these jars are the same libraries (with the CPU modules, aarch64 one per ARM feature level) for other Android JVM setups. |
-| `cuda13-linux-x86-64` | CUDA 13 | Linux x86-64 with NVIDIA GPU | NVIDIA driver + CUDA 13 runtime libraries (`libcudart.so.13`, `libcublas.so.13`). |
-| `cuda13-windows-x86-64` | CUDA 13 | Windows x86-64 with NVIDIA GPU | NVIDIA driver + CUDA 13 Toolkit (`cudart64_13.dll`, `cublas64_13.dll`, `cublasLt64_13.dll` on `PATH`). |
+| `cuda13-linux-x86-64` | CUDA 13 | Linux x86-64 with NVIDIA GPU | NVIDIA driver + CUDA 13 runtime libraries (`libcudart.so.13`, `libcublas.so.13`). **The driver must be new enough for CUDA 13.4** -- see the note below. |
+| `cuda13-windows-x86-64` | CUDA 13 | Windows x86-64 with NVIDIA GPU | NVIDIA driver + CUDA 13 Toolkit (`cudart64_13.dll`, `cublas64_13.dll`, `cublasLt64_13.dll` on `PATH`). **The driver must be new enough for CUDA 13.4** -- see the note below. |
 | `vulkan-linux-x86-64` | Vulkan | Linux x86-64 with a Vulkan 1.2+ GPU (NVIDIA / AMD / Intel) | A Vulkan runtime (`libvulkan.so.1`), which current GPU drivers install. The most portable Linux GPU option. glibc ≈ 2.39 (built on `ubuntu-latest`). |
 | `vulkan-linux-aarch64` | Vulkan | Linux aarch64 with a Vulkan 1.2+ GPU | A Vulkan runtime (`libvulkan.so.1`). glibc ≥ 2.39. |
 | `vulkan-windows-x86-64` | Vulkan | Windows x86-64 with a Vulkan 1.2+ GPU | A Vulkan runtime (`vulkan-1.dll`), which current GPU drivers install. The most portable Windows GPU option. |
@@ -270,19 +270,46 @@ platforms you target, e.g. `cpu-linux-x86-64`.
 | `rocm-windows-x86-64` | ROCm / HIP | Windows x86-64 with AMD GPU | The AMD ROCm **10** runtime DLLs (`amdhip64.dll`, `rocblas.dll`, `hipblas.dll`) on `PATH`; every Radeon target TheRock builds for Windows, gfx900 through RDNA4 (gfx900/gfx906/gfx90c/gfx1153 best effort). |
 | `sycl-linux-x86-64` | SYCL (Intel oneAPI, fp16) | Linux x86-64 with Intel GPU (Arc / iGPU) | An Intel oneAPI / Level-Zero runtime. fp16 accumulation, as upstream's SYCL release builds. |
 | `sycl-windows-x86-64` | SYCL (Intel oneAPI, fp16) | Windows x86-64 with Intel GPU (Arc / iGPU) | The Intel oneAPI / Level-Zero runtime DLLs on `PATH`. |
-| `openvino-linux-x86-64` | OpenVINO | Linux x86-64 (Intel GPU / NPU / CPU) | An Intel OpenVINO runtime. |
-| `openvino-windows-x86-64` | OpenVINO | Windows x86-64 (Intel GPU / NPU / CPU) | The Intel OpenVINO runtime DLLs on `PATH`. |
+| `openvino-linux-x86-64` | OpenVINO | Linux x86-64 with an Intel GPU or NPU | **Opt-in** (see the note below): the jar does nothing until `-Dnet.ladenthin.llama.backend=openvino` names it. An Intel OpenVINO runtime **and an OpenCL ICD loader** (`libOpenCL.so.1`, package `ocl-icd-libopencl1` -- the OpenVINO archive does not ship it; without it the module silently does not open). |
+| `openvino-windows-x86-64` | OpenVINO | Windows x86-64 with an Intel GPU or NPU | **Opt-in** (see the note below): the jar does nothing until `-Dnet.ladenthin.llama.backend=openvino` names it. The Intel OpenVINO runtime DLLs on `PATH` (the OS supplies `OpenCL.dll`). |
 
 > [!NOTE]
 > No vendor runtime is bundled; it comes from the GPU driver or toolkit on the host. The GPU
 > module jars are validated **build-only** in CI (GitHub runners have no GPU): the smoke jobs put
 > every GPU module of the platform next to the library and see ggml skip each one for lack of a
 > runtime, so end-to-end GPU inference is verified locally / on self-hosted hardware. A module
 > whose runtime is installed but finds no device (e.g. the CUDA toolkit without an NVIDIA GPU)
-> simply registers none. `-Dnet.ladenthin.llama.backend=<a>,<b>` restricts the modules put in
+> simply registers none -- **with two measured exceptions, both described below.** `-Dnet.ladenthin.llama.backend=<a>,<b>` restricts the modules put in
 > place to the named ones (`cpu` for none) and fails loud when a named jar is not on the
 > classpath; unset, every GPU module jar on the classpath is used.
 
+> [!IMPORTANT]
+> **Two GPU jars need more than their runtime, and both were found by measurement rather than by
+> reading the code** ([`docs/benchmarks/gpu-module-jars.md`](docs/benchmarks/gpu-module-jars.md)).
+>
+> **CUDA: the driver must be new enough for the toolkit the module was built with (13.4).** The
+> module ships native code for the GPU generations listed in `CMAKE_CUDA_ARCHITECTURES`
+> (Turing through Blackwell); a GPU outside that set runs only if the driver can compile the
+> module's PTX, and **a driver too old to do so does not fall back to the CPU -- it ends the
+> process.** ggml calls `abort()`, which no Java `catch` can intercept, and it happens even with
+> `-ngl 0`: having the jar on the classpath is enough, because the backend registers and the device
+> is used during setup whatever the layer count says. Measured on a Tesla T4 with driver
+> 595.91.07, where that generation still shipped as PTX only. If you cannot control the driver,
+> keep the jar off the classpath or pass `-Dnet.ladenthin.llama.backend=cpu`.
+>
+> **OpenVINO is opt-in, and that is why.** On a host *without* an Intel accelerator its CPU plugin
+> opens perfectly well and contributes a device, so llama.cpp prefers it -- measured at
+> **17x slower prompt processing and 172x slower generation** than the CPU modules (490.4/34.5
+> down to 29.4/**0.2** tokens per second). It therefore breaks the premise every other module jar
+> keeps ("a module that cannot help is skipped") in both directions: it also fails to open at all
+> without an OpenCL ICD loader, which its own runtime archive does not include. So its jar on the
+> classpath does nothing until `net.ladenthin.llama.backend` names it, and the loader says so on
+> stderr.
+>
+> **Vulkan is the one that behaves exactly as designed** and is the recommended portable choice:
+> without an ICD it is skipped, with throughput identical to the CPU baseline (201.8/24.1 against
+> 202.2/24.1 on the same machine).
+
 > [!NOTE]
 > On the **module path** each natives jar is an automatic module
 > (`net.ladenthin.llama.natives.<classifier>`, with `_` for `-`) that nothing requires, so

TODO.md

@@ -143,15 +143,44 @@ host-CPU line says so).
 
 **Round 1 -- things that need no run, done while round 0 is in flight.** None of these touch the same
 files as each other, so they can go in one branch:
-1. **The shipped diagnostic entry point** (item 5 of the modular-natives follow-ups). The only item on
-   any list here that pays off *per user* rather than per release: it turns "it does not work" into a
-   paste. Testable without natives.
-2. **README: the GPU jars' real requirements.** `cuda13` needs a driver new enough for the toolkit
-   (and what happens when it is not: the JVM aborts); `openvino` is opt-in now and needs an Intel
-   GPU/NPU *plus* an OpenCL ICD loader; `vulkan` needs only a driver. None of that is documented, and
-   all three are measured.
-3. **`TestConstantsTest` / `TestModelPaths` parity.** `llama-langchain4j` carries its own copy of the
-   resolver; the vacuity guard added to the core's test has no counterpart there.
+1. ~~**The shipped diagnostic entry point**~~ **done 2026-10-11**: `net.ladenthin.llama.Diagnose`
+   (the main class) plus `loader.DiagnosticReport` (the half only the loader can answer). The split
+   is not tidiness: every platform and classpath fact is a package-private member of `LlamaLoader`,
+   and **Java access is per package, not per module** -- so that half has to live in the loader's
+   package, while the main class cannot, because that package is deliberately not exported and a
+   consumer on the module path could not run a main there. The architecture rule allows exactly this
+   direction (`Api -> Loader`, never back), which is also why the pinned-version constant (`value`)
+   and the load itself (`LlamaModel`) are read on the `Diagnose` side.
+   It prints the pinned tag, the JVM, `os.name`/`os.arch` and what `OSInfo` derives from them, every
+   set `net.ladenthin.llama.*` property, each natives directory of the platform **with the llama.cpp
+   tag of its `jllama-build.txt`** (which is how a mismatched set becomes visible at all) and, on a
+   failed load, the operating system's own reason. 9 tests, no natives and no model needed.
+   **Two defects of its own, both found by running it rather than by reading it, and both the same
+   mistake:** it composed the property name from `LlamaSystemProperties.PREFIX`, which carries **no**
+   trailing dot, and so read `net.ladenthin.llamabackend` -- a name nothing sets, so a filter that
+   was in effect was reported as unset; and `namedBy` compared case-insensitively while
+   `LlamaLoader.selectModules` compares with `equals` and **fails loud** on an unknown name, so
+   `OpenVino` would have been reported as an enabled module while the load throws on it. Both are now
+   read through the loader's own accessor and matched by the loader's own comparison, each with a
+   red/green test. The rule they both broke: **a diagnostic may not be kinder, or differently spelled,
+   than the code it describes.**
+2. ~~**README: the GPU jars' real requirements**~~ **done 2026-10-11**: the two `cuda13` rows name the
+   driver requirement, the two `openvino` rows say they are opt-in and name the OpenCL ICD loader the
+   OpenVINO archive does not ship, and an `[!IMPORTANT]` block carries the measured detail -- the
+   CUDA abort (`ggml_abort`, not catchable, and reached even with `-ngl 0`), the 17x/172x OpenVINO
+   regression that is the reason for the opt-in, and Vulkan as the one backend that behaves as
+   designed. The existing note's sentence "simply registers none" now names its two exceptions
+   instead of stating the rule as if it held. **The README did not mention the OpenVINO opt-in at
+   all**, so a consumer adding that jar would have seen nothing happen and no reason why.
+3. ~~**`TestConstantsTest` / `TestModelPaths` parity**~~ **done 2026-10-11, and the premise was
+   mostly outdated**: `llama-langchain4j`'s test file had grown since that line was written and
+   already covered nine of the core's ten cases, including the reactor-root branch non-vacuously
+   (it resolves `llama-langchain4j/pom.xml`, which exists only when looked up from the parent, so a
+   pass proves the second branch ran) and the "no test may read a model property without the
+   resolver" source rule. The one genuine gap was `fromProperty`: only the **unset** case was
+   tested, so a set property that bypassed the resolver would have been invisible -- which is
+   exactly the failure the resolver exists for, and the one that stayed hidden in the core for
+   hundreds of builds because a class-level assumption records no test entries. One test added.
 
 **Round 2 -- needs round 0 to have produced artifacts.**
 4. **A `g4dn` run proving a T4 works** with the new arch list (round 0 builds the module; the probe

llama-langchain4j/src/test/java/net/ladenthin/llama/langchain4j/TestModelPathsTest.java

@@ -6,6 +6,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -82,6 +83,38 @@ void fromPropertyIsNullWhenThePropertyIsUnset() {
         assertThat(TestModelPaths.fromProperty("net.ladenthin.llama.langchain4j.definitely.unset"), is(nullValue()));
     }
 
+    /**
+     * The half the unset case cannot see: a property that IS set has to go through the resolver, not
+     * through a bare {@code System.getProperty}. Without this, {@code fromProperty} could return the
+     * literal and every model-gated class in this module would abort in its {@code @BeforeAll}
+     * assumption and report as nothing at all -- which is the exact failure the resolver was written
+     * for, and the one that stayed invisible in the core module for hundreds of builds because a
+     * class-level assumption records no test entries.
+     *
+     * <p>The fixture is this module's own pom, which exists only when looked up from the reactor root
+     * -- so a pass proves the second branch ran rather than that the path happened to be right.
+     */
+    @Test
+    void fromPropertyResolvesAndDoesNotReturnTheLiteral() {
+        String property = "net.ladenthin.llama.langchain4j.resolver.probe";
+        String literal = "llama-langchain4j/pom.xml";
+        String previous = System.getProperty(property);
+        try {
+            System.setProperty(property, literal);
+            Path resolved = TestModelPaths.fromProperty(property);
+            assertThat("a set property must be resolved", resolved, is(notNullValue()));
+            assertThat("the resolved path must exist: " + resolved, Files.exists(resolved), is(true));
+            assertThat("the resolver's second branch must have run, not a bare getProperty",
+                    resolved, is(not(Paths.get(literal))));
+        } finally {
+            if (previous == null) {
+                System.clearProperty(property);
+            } else {
+                System.setProperty(property, previous);
+            }
+        }
+    }
+
     /**
      * The rule, not a snapshot: no test in this module may read a model path with a bare
      * {@code System.getProperty}, because that bypasses the resolver and silently re-mutes that class

llama/spotbugs-exclude.xml

@@ -46,6 +46,21 @@ SPDX-License-Identifier: MIT
         leaves no compile-time hint that an I/O failure happened; we accept that
         tradeoff at these three boundary points.
     -->
+    <!--
+        Diagnose.load catches the load failure and returns false, which SpotBugs reports as
+        EXS_EXCEPTION_SOFTENING_RETURN_FALSE. That is the class's entire contract: it is the
+        diagnostic a consumer runs BECAUSE the library does not load, so the failure is content to
+        print rather than a condition to propagate. A report that throws at the first problem would
+        withhold exactly the lines that explain it. Nothing is lost: the exception's own
+        toString() and the operating system's reason are both written to the report, and main()
+        rethrows afterwards so the process still exits non-zero.
+    -->
+    <Match>
+        <Class name="net.ladenthin.llama.Diagnose"/>
+        <Bug pattern="EXS_EXCEPTION_SOFTENING_RETURN_FALSE"/>
+        <Method name="load"/>
+    </Match>
+
     <Match>
         <Class name="net.ladenthin.llama.LlamaModel"/>
         <Bug pattern="EXS_EXCEPTION_SOFTENING_NO_CONSTRAINTS"/>