Skip to content

NPU: move the Qwen3.6-35B-A3B route to a private add-on, keep a generic hook - #85

Merged
bong-water-water-bong merged 1 commit into
mainfrom
npu/lax-private-addon
Sep 25, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
npu/lax-private-addon

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

The Qwen3.6-35B-A3B NPU route is now closed source. Its code has moved to a private add-on, and the engine keeps only a small hook for it.

Removed

  • The third_party/OpenFlowLM-Next submodule and its NOTICE entry
  • scripts/build-lax.sh
  • npu/lax*
  • app/npu_lax* and the 1bit npu-lax command
  • tests/npu_lax* and tests/golden/npu_lax/
  • docs/npu-lax.md
  • The ONEBIT_NPU_LAX* CMake options
  • The --npu-kernels, --npu-transport and --npu-snapshots flags

The docs that mentioned the route now have a short note instead.

Added

  • npu/private_route.{h,cpp}: a registry of private NPU routes keyed by model_type, plus optional 1bit subcommands.
  • CMake option -DONEBIT_NPU_PRIVATE=<npu-kernels checkout> (needs -DONEBIT_NPU=ON). It runs add_subdirectory on the checkout's addons/, links onebit_npu_private and defines ONEBIT_NPU_PRIVATE. At startup, 1bit then calls register_private_addon().
  • 1bit serve --npu-opt KEY=VALUE (in unified: --opt), which passes options through to a route.
  • Without the add-on, 1bit serve -m <Qwen3.6-35B-A3B dir> --device npu exits with: the Qwen3.6-35B-A3B NPU route is not part of this build; build with -DONEBIT_NPU_PRIVATE=<npu-kernels checkout>.
  • tests/npu_private_route_test.cpp (ctest npu_private_route), added to the CI target list.
  • docs/npu.md, "Private routes".

The fast lane is unchanged.

Tested on Strix Halo (2026-09-25)

  • Without the option (-DONEBIT_NPU=ON):
    • The CI target list builds and ctest passes 5/5.
    • serve on the 35B directory prints the message above and exits 1.
    • tests/serve_e2e.sh with Qwen3-0.6B on npu passes.
  • With -DONEBIT_NPU_PRIVATE:
    • The add-on's tests pass: parity against the fp64 reference (argmax 846 / 198 / 3710, corr >= 0.99999), and a serve round trip that answers Paris with no xclbin opened.
    • serve_e2e.sh with Qwen3-0.6B on npu still passes.

🤖 Generated with Claude Code

…ic hook

The 35B NPU route (its host code, kernel pin, build script, tests and golden
files) now lives in the private npu-kernels repository. The engine keeps only
npu/private_route.h: -DONEBIT_NPU_PRIVATE=<npu-kernels checkout> builds that
checkout's addons/ into 1bit, which registers routes by model_type and optional
subcommands; `1bit serve --npu-opt KEY=VALUE` passes options through. Without the
add-on, serving a Qwen3.6-35B-A3B directory says the route is not part of this
build. The fast lane is unchanged.

Removed: third_party/OpenFlowLM-Next (and its NOTICE entry), scripts/build-lax.sh,
npu/lax*, app/npu_lax*, tests/npu_lax*, tests/golden/npu_lax, docs/npu-lax.md,
the ONEBIT_NPU_LAX* options and --npu-kernels/--npu-transport/--npu-snapshots.
Added: npu/private_route.{h,cpp}, tests/npu_private_route_test.cpp (in CI), and
docs/npu.md "Private routes".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@context7

context7 Bot commented Sep 25, 2026

Copy link
Copy Markdown

Docs7 for 1bit-monster/engine

Result Status Action
Deployment ➖ Not used —
Content review ➖ Did not run. This site has no agent runs available this month. Wait for the monthly reset or check your Docs7 plan. —

Commit a544dae

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit a544dae)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Private Route Availability Check

The npu_model_problem function checks for the existence of model files and lane kernels, but does not validate that the private route's unavailable function returns an empty string when the route is available. This could lead to incorrect error messages if a private route is registered but its unavailable function has a bug or returns a non-empty string even when the route should be available.

std::string npu_model_problem(const std::string& dir, const npu::PrivateOptions& opts) {
    if (!has_model_files(dir)) return dir + " is not an NPU model directory (model.q4nx, config.json, tokenizer.json)";
    const std::string type = npu::model_type_of(dir);
    if (const auto* r = npu::find_private_route(type)) {
        const std::string why = r->unavailable ? r->unavailable(dir, opts) : "";
        return why.empty() ? "" : dir + ": " + why;
    }
    if (const std::string why = npu::private_route_missing(type); !why.empty()) return dir + ": " + why;
    if (!has_lane_kernels(dir)) return dir + " is not an NPU model directory (the fast lane needs npu/)";
    return "";
}
Option Parsing for Private Routes

The --npu-opt option parsing in run_serve does not validate that the parsed options are used correctly by the private route. If a private route requires specific options that are not provided, it may fail silently or behave unexpectedly. The error handling for missing or invalid options should be more robust.

npu::PrivateOptions opts;
for (const auto& kv : o.npu_opts) npu::parse_private_option(kv, opts);
if (const std::string why = npu_model_problem(o.model, opts); !why.empty()) throw std::runtime_error(why);
// The NPU serves in process (unified.cpp).
std::vector<std::string> args = {"-m", o.model, "-p", std::to_string(o.port), "--host", o.host};
if (!o.alias.empty()) { args.push_back("--alias"); args.push_back(o.alias); }
for (const auto& kv : o.npu_opts) { args.push_back("--opt"); args.push_back(kv); }
Private Route Registration

The register_private_route function replaces an existing route with the same model_type but does not provide a mechanism to verify that the new route is compatible with the existing one. This could lead to unexpected behavior if a private route is registered with a different interface than the one previously registered.

void register_private_route(PrivateRoute route) {
    for (auto& r : routes())
        if (r.model_type == route.model_type) {
            r = std::move(route);
            return;
        }
    routes().push_back(std::move(route));
}
Private Addon Validation

The CMake logic for ONEBIT_NPU_PRIVATE checks for the existence of addons/CMakeLists.txt but does not validate that the onebit_npu_private target is correctly defined and linked. If the add-on's CMakeLists.txt is malformed or does not define the required target, the build may succeed but the private route functionality will not work as expected.

set(ONEBIT_NPU_PRIVATE "" CACHE PATH "An npu-kernels checkout whose addons/ are built into 1bit (needs ONEBIT_NPU)")
if(ONEBIT_NPU_PRIVATE)
    if(NOT ONEBIT_NPU)
        message(FATAL_ERROR "ONEBIT_NPU_PRIVATE needs ONEBIT_NPU=ON")
    endif()
    if(NOT EXISTS "${ONEBIT_NPU_PRIVATE}/addons/CMakeLists.txt")
        message(FATAL_ERROR "ONEBIT_NPU_PRIVATE=${ONEBIT_NPU_PRIVATE} has no addons/CMakeLists.txt")
    endif()
    add_subdirectory(${ONEBIT_NPU_PRIVATE}/addons ${CMAKE_BINARY_DIR}/npu_private)
    if(NOT TARGET onebit_npu_private)
        message(FATAL_ERROR "${ONEBIT_NPU_PRIVATE}/addons defines no onebit_npu_private target")
    endif()
    target_link_libraries(onebit PRIVATE onebit_npu_private)
    target_compile_definitions(onebit PRIVATE ONEBIT_NPU_PRIVATE)
endif()

@bong-water-water-bong
bong-water-water-bong marked this pull request as ready for review September 25, 2026 15:02
@bong-water-water-bong
bong-water-water-bong merged commit 803b013 into main Sep 25, 2026
6 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the npu/lax-private-addon branch September 25, 2026 15:03
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit a544dae

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant