diff --git a/CMakeLists.txt b/CMakeLists.txt index ce984737f..71260bad5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -200,6 +200,12 @@ add_test(NAME serve_hangup COMMAND ${CMAKE_SOURCE_DIR}/tests/serve_hangup_test.s # engine#1945: scripts/bump-hrx-sync.sh tells AMD's own commit from a line AMD rebased apart from a commit that # is not AMD's. Local git repositories only: no network, no GPU. add_test(NAME bump_hrx_sync COMMAND bash ${CMAKE_SOURCE_DIR}/tests/bump_hrx_sync_test.sh ${CMAKE_SOURCE_DIR}/scripts/bump-hrx-sync.sh) +# engine#2102: the sentence a serve owes an operator when no 16-row decode kernel was found (app/npu_decode_note.h). Pure: +# the silent cases are the ones that matter, and they are arms. +add_executable(npu_decode_note_test tests/npu_decode_note_test.cpp) +target_include_directories(npu_decode_note_test PRIVATE app) +add_test(NAME npu_decode_note COMMAND npu_decode_note_test) +add_dependencies(onebit npu_decode_note_test) add_test(NAME bump_hrx_report COMMAND bash ${CMAKE_SOURCE_DIR}/tests/bump_hrx_report_test.sh ${CMAKE_SOURCE_DIR}/scripts/bump-hrx-report.sh) add_test(NAME metrics_serve COMMAND ${CMAKE_SOURCE_DIR}/tests/metrics_serve.sh $) # serve --print-backend: the llama-server Lemonade starts itself, one hop (docs/serve.md): no GPU. diff --git a/app/forward_serve.cpp b/app/forward_serve.cpp index b5da1fa66..5799a6738 100644 --- a/app/forward_serve.cpp +++ b/app/forward_serve.cpp @@ -35,6 +35,8 @@ // resets it from position 0. #include "forward_serve.h" +#include "npu_decode_note.h" + #include "../npu/forward/include/q4nx_forward.h" #include "../npu/target/xdna/xdna_target.h" // XdnaTarget::kName for --npu-target #include "npu_device_lock.h" @@ -1365,6 +1367,13 @@ int run_forward_serve(int argc, char** argv) { // The target that is actually serving, so a log says which one answered rather than leaving it to the build. std::fprintf(stderr, " [npu-target=%s]", npu_target.c_str()); std::fprintf(stderr, "\n"); + // engine#2102: the machine line `[forward] decode family: …` stays byte-for-byte (gates grep it). This is the sentence + // an operator needs at the ready line's level when it says 0 of N -- the route that answers correctly and 50x slower. + { + const std::string note = onebit::npu::decode_family_note(e.fwd->decode_roles(), e.fwd->design_roles(), kernels, + e.fwd->decode_disabled(), e.fwd->decode_image()); + if (!note.empty()) std::fprintf(stderr, "%s\n", note.c_str()); + } while (true) std::this_thread::sleep_for(std::chrono::seconds(3600)); return 0; diff --git a/app/npu_decode_note.h b/app/npu_decode_note.h new file mode 100644 index 000000000..0f208661c --- /dev/null +++ b/app/npu_decode_note.h @@ -0,0 +1,64 @@ +// Copyright 2026 bong-water-water-bong +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// The sentence a serve owes an operator when it is about to run the slow route (engine#2102). +// +// The fact already exists as a machine line — `[forward] decode family: 0 role(s) at M=16, 4 on M=128` — and that line is +// grepped byte-for-byte by the gates, so it must not be reworded. What was missing is a sentence at the ready line's level: +// a serve with no 16-row decode kernels runs every decode step as prefill-width launches, measured at 1.9 tok/s where the +// fused route on the same box does 88-101, and nothing said so. This header is the whole decision, as a pure function, so it +// can be armed without a device and so the "say nothing" case is as testable as the "say something" one. +// +// It deliberately returns "" for the cases it cannot judge (a host-GEMM arm has no family at all; a count of 0 of 0 is a +// different fault, reported elsewhere) — a warning that fires on healthy serves is a warning nobody reads. + +#pragma once + +#include +#include + +namespace onebit::npu { + +// decode_roles: roles that got an M=16 decode kernel. design_roles: roles the model has (0 on the host-GEMM arm, which loads +// no decode family). kernels_dir: what the operator pointed at, echoed so the message can be acted on without reading the +// rest of the log. disabled: NPU_DECODE_M16=0 turned the family off, so nothing is missing. image: a ternary (pb16i/t2s) +// container, whose loader refuses a missing decode kernel; the roles it leaves on M=128 are ones its geometry or the split +// designs cover (named per role in the load log), so for it only 0 of N is this issue. +// +// The way out names the fused route, not the _m16 designs: measured on Qwen3-0.6B (#2102), the per-op forward with all four +// twins decodes at ~2.7 tok/s against ~1.7 without them and 88-101 on the fused route, so the twins are not the demo rate. +inline std::string decode_family_note(int decode_roles, int design_roles, const std::string& kernels_dir, + bool disabled = false, bool image = false) { + if (design_roles <= 0) return {}; // no roles (or the host arm): not this issue's fault, and no number to quote + const char* dir = kernels_dir.empty() ? "(no kernel directory)" : kernels_dir.c_str(); + char buf[640]; + if (disabled) { + std::snprintf(buf, sizeof buf, + "npu: NPU_DECODE_M16=0 turned the 16-row decode family off -- decode runs prefill-width launches on all %d " + "role(s) (engine#2102). Unset it to use the decode kernels in %s.", + design_roles, dir); + return std::string(buf); + } + if (decode_roles >= design_roles) return {}; // a full decode family: say nothing + if (image && decode_roles > 0) return {}; // a ternary family's M=128 roles are its geometry's, already named + std::snprintf(buf, sizeof buf, + "npu: %d of %d role(s) have a 16-row decode kernel in %s -- decode runs prefill-width launches and is far " + "slower than the fused route (engine#2102). The _m16 designs help only part of the way; the demo rate is the " + "fused route's (docs/npu.md, plan.json: decode.dirs).", + decode_roles, design_roles, dir); + return std::string(buf); +} + +} // namespace onebit::npu diff --git a/docs/npu.md b/docs/npu.md index cd6d4b81c..43673cf4a 100644 --- a/docs/npu.md +++ b/docs/npu.md @@ -90,6 +90,25 @@ The schema, the refusal rules and the generator flags are in [npu-plan.md](npu-p | `kernels.tag` (`""`/`_b8`/`_pb16i`/`_t2s`) | the artifact family came from `q4nx_weight_layout`, so a kernel dir of the other family changed which kernels ran | | `decode.dirs` | the M=16 twins were looked for in `/kernels-m16`, then `/m16`, then `` | +**A decode off the 16-row family is said at ready (engine #2102).** forward-serve prints one line after `ready on +http://...` when roles have no M=16 decode kernel. In that case every decode step runs a 128-row prefill launch: + +``` +npu: 0 of 4 role(s) have a 16-row decode kernel in -- decode runs prefill-width launches and is far slower than the fused route (engine#2102). ... +``` + +The load line `[forward] decode family: N role(s) at M=16, ...` stays byte-for-byte, because gates grep it. The new line +is silent when: + +- the family is full; +- the arm is host-GEMM (`NPU_INFER_HOST_GEMM`, which loads no family); +- the container is ternary (pb16i/t2s) with some roles on M=128. Its loader refuses a missing decode kernel, and the + roles it leaves are covered by its geometry, which the load log names per role. + +`NPU_DECODE_M16=0` gets its own sentence, which names the switch. The twins are not the way to the demo rate. Measured on +Qwen3-0.6B, the per-op forward decodes at ~1.7 tok/s without them and ~2.7 tok/s with all four, against 88-101 tok/s on +the fused route. + `tools/npu_plan.py ` writes one from `config.json` plus the kernel directory (it derives layer roles from `layer_types` and projection shapes from the model's own dimensions, the way the forward does). `--check` re-generates and diffs against the file on disk, so a stale plan is a gate failure rather than a diff --git a/npu/forward/include/q4nx_forward.h b/npu/forward/include/q4nx_forward.h index aeb771585..22901baaf 100644 --- a/npu/forward/include/q4nx_forward.h +++ b/npu/forward/include/q4nx_forward.h @@ -96,6 +96,16 @@ class Q4nxNpuForward { // noncausal_full_attention, so a caller can check the served pack against its own declaration // (app/forward_serve.cpp, engine#796). bool noncausal() const { return noncausal_attn_; } + // engine#2102: what the decode family turned out to be, so a caller can say it out loud. `decode_roles()` is the number of + // roles that got an M=16 kernel, `design_roles()` the number of roles the model has. 0 of N is the state that serves at a + // fraction of the fused route's rate while printing nothing an operator would notice at the ready line: every decode step + // then runs prefill-width launches. The values are recorded where the family is opened (open_decode_family_local), not + // inferred later, because the derived designs are not kept per role. + int decode_roles() const { return decode_roles_; } + // The host-GEMM arm loads no decode family, so it reports no roles rather than "0 of N" (engine#2102). + int design_roles() const { return host_gemm_ ? 0 : ndesigns_; } + bool decode_disabled() const { return decode_disabled_; } // NPU_DECODE_M16=0 + bool decode_image() const { return pb16i_; } // a ternary (pb16i/t2s) container's decode family const char* last_error() const { return err_.c_str(); } // Clear the KV cache (start a new sequence). @@ -332,6 +342,8 @@ class Q4nxNpuForward { // kernels for fused gate/up, five when gate and up are split. Q4nxDesignGeom designs_[Q4NX_MAX_DESIGNS]; int ndesigns_ = 0; + bool decode_disabled_ = false; // engine#2102: NPU_DECODE_M16=0 turned the family off + int decode_roles_ = 0; // engine#2102: roles with an M=16 decode kernel, as open_decode_family_local() found them // M=16 decode twins of the same roles (empty/skipped where no ELF or no legal geometry). The geometry // stays here because the model needs it (max_batch_rows, the layer loop's row count); the contexts // themselves live in the target. diff --git a/npu/forward/src/q4nx_forward.cpp b/npu/forward/src/q4nx_forward.cpp index ce4ec9424..e28008415 100644 --- a/npu/forward/src/q4nx_forward.cpp +++ b/npu/forward/src/q4nx_forward.cpp @@ -439,6 +439,7 @@ bool Q4nxNpuForward::open_decode_family_local(bool want_m16, xrt::device* dev, c if (!want_m16) { fprintf(stderr, " [forward] decode family: %d role(s) at M=%d, %d on M=%d%s\n", 0, Q4NX_DESIGN_M_DECODE, ndesigns_, Q4NX_DESIGN_M_PREFILL, " (NPU_DECODE_M16=0: disabled)"); + decode_disabled_ = true; // engine#2102: turned off on purpose, which the ready line must not call missing return true; } q4nx_derive_decode_designs(designs_, ndesigns_, designs16_); @@ -591,6 +592,7 @@ bool Q4nxNpuForward::open_decode_family_local(bool want_m16, xrt::device* dev, c designs16_[i].N, elf.c_str()); } xd_->note_m16_made(made > 0); + decode_roles_ = made; // engine#2102: the caller-facing count, beside the byte-for-byte log line below if (pb16i_ && made < ndesigns_ - 0) { fprintf(stderr, " [forward] %s: %d of %d designs have a decode kernel\n", t2s_ ? "t2s" : "pb16i", made, ndesigns_); diff --git a/tests/npu_decode_note_test.cpp b/tests/npu_decode_note_test.cpp new file mode 100644 index 000000000..33f3a3094 --- /dev/null +++ b/tests/npu_decode_note_test.cpp @@ -0,0 +1,92 @@ +// Copyright 2026 bong-water-water-bong +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// tests/npu_decode_note_test.cpp -- the "say it out loud" decision of app/npu_decode_note.h (engine#2102). +// +// The arms that matter are the silent ones: a warning that fires on a healthy serve is worse than no warning, because it +// teaches an operator to ignore the line that exists to be noticed. Every arm here is a hand-computed case; no device. + +#include +#include + +#include "npu_decode_note.h" + +namespace { + +int failures = 0; + +void check(bool ok, const char* what) { + if (!ok) { + std::printf("FAIL %s\n", what); + ++failures; + } else { + std::printf("ok %s\n", what); + } +} + +} // namespace + +int main() { + using onebit::npu::decode_family_note; + + // 1. the measured case: 0 of 4 roles, which is 1.9 tok/s where the fused route does 88-101 + { + const std::string note = decode_family_note(0, 4, "/kernels/i8"); + check(!note.empty(), "0 of 4 roles: a note is produced"); + check(note.find("0 of 4 role(s)") != std::string::npos, "... it names the counts"); + check(note.find("/kernels/i8") != std::string::npos, "... and the directory the operator pointed at"); + check(note.find("engine#2102") != std::string::npos, "... and the issue, so the message can be followed"); + check(note.find("fused route") != std::string::npos, "... and what the healthy route is called"); + } + + // 2. THE SILENT CASES -- a warning on a healthy serve is worse than none + check(decode_family_note(4, 4, "/k").empty(), "4 of 4 roles (the fused route): silent"); + check(decode_family_note(5, 4, "/k").empty(), "more families than roles (a wide set too): silent"); + check(decode_family_note(0, 0, "/k").empty(), "0 of 0 (no roles at all, a different fault): silent"); + check(decode_family_note(3, 0, "/k").empty(), "a role count of zero is not this issue's business: silent"); + + // 3. partial families are still the slow path, and the count says how slow to expect + { + const std::string note = decode_family_note(2, 4, "/k"); + check(!note.empty(), "2 of 4 roles: a partial family still warns (the other two run prefill width)"); + check(note.find("2 of 4") != std::string::npos, "... naming the partial count"); + } + + // 3b. the cases that are not missing kernels (engine#2102 review, all three reproduced on 692ae19) + check(decode_family_note(0, 0, "/k").empty(), "host-GEMM arm (design_roles() is 0 there): silent"); + check(decode_family_note(8, 9, "/k", false, true).empty(), "healthy t2s container, 8 of 9 (docs/npu.md): silent"); + check(!decode_family_note(0, 9, "/k", false, true).empty(), "a ternary container with no decode kernel at all: warns"); + { + const std::string off = decode_family_note(0, 4, "/k", true, false); + check(off.find("NPU_DECODE_M16=0") != std::string::npos, "NPU_DECODE_M16=0: names the switch"); + check(off.find("of 4 role(s) have") == std::string::npos, "... and does not call the kernels missing"); + check(!decode_family_note(4, 4, "/k", true, false).empty(), "... even when every twin is on disk"); + } + // 3c. the way out is the fused route: the twins alone measured ~2.7 tok/s against 88-101 (#2102) + check(decode_family_note(0, 4, "/k").find("Ship the") == std::string::npos, "the note does not promise speed from the _m16 designs"); + + // 4. no directory: the message must not read as a path + { + const std::string note = decode_family_note(0, 4, ""); + check(note.find("(no kernel directory)") != std::string::npos, "an empty kernel directory is named as absent, not as ''"); + } + + if (failures != 0) { + std::printf("npu decode note: %d failure(s)\n", failures); + return 1; + } + std::printf("npu decode note: all cases ok\n"); + return 0; +}