6.6 kB 76 lines
2f329ddb
round 40: add apple silicon README, setup script, and complete strata metal diff
dawn
⋯
audits/r40_opus_readme_review.md
sourcerendered
download raw history
at main Markdown

round 40 opus review: repo organization, README, fresh-clone path #

read-only. i inspected the files and replayed the diff into /tmp/r40_apply; nothing was built and no models were downloaded.

blockers #

  1. setup crashes at the end of the ~70 GB download, on a stock python3. scripts/setup_apple_silicon.sh runs python3 scripts/build_k512_iq2_xs_metal.py and python3 scripts/build_mtp_q2_0.py with the bare interpreter.
    • the k512 builder ends with ST.extract(...). vendor/strata/tools/strata_tokenizer.py imports regex at module top, so a fresh mac python3 dies after the dense, expert and PLE downloads, before it writes tokenizer/ and strata-k512-iq2xs.json.
    • build_mtp_q2_0.py runs vendor/strata/tools/mtp_pack.py, which imports numpy and gguf (llama.cpp's gguf-py, itself numpy-based).
    • fix: run both through uv, which the README already requires, as the serve scripts do: "$UV_BIN" run --with regex --with numpy python3 …. mtp_pack.py is started via sys.executable, so it inherits the uv env. prepare_strata_k256.py has the same regex dependency when tokenizer/ is missing; the README command should be uv run --with regex python3 scripts/prepare_strata_k256.py.
  2. the README promises k=320 / k=224 but the scripts only handle k=256. download_k256_local.py and prepare_strata_k256.py hardcode K256_reap_l2_sum and NE = 256. either say "k=256" only, or add a --k argument.
  3. the macOS requirement is probably wrong. the README says "macos 15+". dense_mm.metal, moe_mm.metal, qsa_prompt.metal and mm_bench.metal include <MetalPerformancePrimitives/MetalPerformancePrimitives.h>, which ships with macOS 26. Window::m5_layout() → coop_k4() compiles mm_bench.metal unconditionally on the prompt path. Context::supports_metal4() exists but nothing in the engine calls it. upstream docs/MAC.md says 15+, but our tested system is macOS 27 on the M5. unless someone has run it on 15, write "macos 26+ (tested: macOS 27, M5)".

(1) hardcoded user paths #

  • zero /Users/dawn (or any /Users/) in scripts/ (setup, serve, build, prepare, download, mtp) and zero in scripts/strata_metal_complete.diff. the serve scripts fall back to $HOME/.local/bin/uv only when uv is not on PATH.
  • the generated configs (strata-k512-iq2xs.json, strata-k256.json) contain absolute paths computed from ROOT at build time, under the git-ignored models/. that is fine. the comment "portable repo-relative" in build_k512_iq2_xs_metal.py:348 is inaccurate (they are absolute, per machine); reword it.
  • download_k256_local.py uses the cwd-relative Path("models/K256_reap_l2_sum"). run from anywhere but the repo root, it downloads 2 shards into the wrong place. use a ROOT from __file__ like the other scripts.

(2) README opts: test-only or rejected items listed as live? #

checked against the engine dispatch sites (metal/src/**) and env defaults:

  • the round 39 rejections are absent: no cuda butterfly, no qsa_merge 4-partial batching, no qsa_merge2 batching. the qsa_merge items listed are the earlier live ones (dispatched at window.cpp:1285/2737/3662, prompt_chain.inc:262). qsa_topk2 is live (dispatched under the qsa_topk key).
  • defaults match the README wording: STRATA_METAL_MTP_CHAIN (on unless "0"), MTP_SINK 64, MTP_WIN 256, PREFETCH_UNPERT 1, STRATA_HC_Q8 on, MTP_SLOP/DRAFT_MOCE/PUBLISH_PREFETCH on. the opt-ins are labeled as such (STRATA_SPEC_GATE, STRATA_SPEC_SAMPLE, STRATA_HC_Q4/Q5, rank-1 unscaling). --save-session and --load-session exist in strata_window_cli. the helpers gemv_q2_raw_combine, gemv_iq4_combine, gemv_direct_x and tg_argmax are inlined into dispatched kernels.
  • wording to check: "qsa_flash … unconditional hoisted rv/rk fills". if that refers to round 39's clamped-row / value-select port, it only runs on the non-cooperative (M1/M4 or COOP_K4=0) path, not on the M5 default. say so, or drop it. if it is the earlier cooperative-path hoist, it is fine.
  • k=256-only artifacts are listed without that scope: the healed K256-q2_0-healed-metal.gguf, the q2_0 quantizer, alpha_l/ridge healing and the layer-47 wfld edit. the default k=512 service uses none of them, and nothing in the README or scripts builds the healed gguf (prepare_strata_k256.py silently falls back to the unhealed IQ3_S). either tag those bullets "(k=256 path)" and give the convert/heal commands, or mark them as research artifacts.
  • "m4 / m5 recommended for matmul2d cooperative tensors": only the M5 gets the coop_k4 matrix-unit layout; the M4 takes the older prompt kernels (prompt_chain.inc:10-11). "m5 (tuned); m4 runs the older prompt kernels" is more accurate.

(3) fresh-clone completeness #

  • scripts/strata_metal_complete.diff reproduces the patched tree exactly: applied to git archive 296b59d2 of the 58 pre-existing paths, plus the 20 new files, it gives 78/78 files byte-identical to the current vendor/strata working tree. the working tree has no changes outside the diff (excluding third_party/metal-cpp, build-metal, __pycache__). the diff has no binary hunks. git apply exits 0 with one whitespace warning (the known blank line at the qsa.metal EOF).
  • everything the build and configs need is either upstream at the pinned commit (metal/tools/metal_repack.py, tools/mtp_fetch.py, tools/mtp_pack.py, data/expert-profile.bin, data/draft_vocab_en.bin) or in the diff (strata_window_cli.cpp, convert_k256_q2_0.cpp, heal_k256_q2_0.cpp, slop4.h, scr_state.hpp, spec_gate.hpp, …). the CMake variables METAL_CPP_DIR and STRATA_GGML_DIR match the setup invocation.
  • repo-tracked inputs exist: results/reap_saliency_v2.json, results/keep_maps/keep_reap_l2_sum_k256.json, mine_llm_isms_from_transcripts.py, scripts/prepare_phase3_sft_data.py, scripts/patch_m5_lazy_moe.py. README.md and scripts/setup_apple_silicon.sh exist only in the jj working-copy change (git shows them as A, not in the last commit), so make sure they land in the round 40 commit.
  • minor: setup's git -C vendor/strata clean -fd wipes untracked non-ignored files on every re-run (metal-cpp gets re-downloaded, sha-checked). harmless on a fresh clone, surprising in a dev tree.
  • minor: the k512 builder caches the source GGUF headers in /tmp/iq2xs_s1.hdr etc. and only checks their size, so a stale header from another upload would be reused silently. low risk while the URLs are pinned.