Skip to content

Add MLX backend for Apple Silicon neural network inference#16

Closed
ChinChangYang wants to merge 130 commits into
masterfrom
feature/mlx-backend
Closed

Add MLX backend for Apple Silicon neural network inference#16
ChinChangYang wants to merge 130 commits into
masterfrom
feature/mlx-backend

Conversation

@ChinChangYang

Copy link
Copy Markdown
Owner

Summary

This PR adds a new MLX backend for KataGo, enabling neural network inference on Apple Silicon using Apple's MLX framework. The implementation includes full model support with several performance optimizations.

Key Features

  • Complete neural network support: Implements all required layers (convolution, batch normalization, residual blocks, policy/value heads, SGF metadata encoding)
  • Graph compilation: Uses mx::compile() to JIT-compile inference functions into fused Metal kernels, with thread-safe caching per configuration
  • Performance optimizations:
    • Skip mask operations when requireExactNNLen=true (all boards are exactly nnXLen × nnYLen)
    • Use logaddexp for Mish activation (reduces 7 ops to 2 ops)
    • Use addmm for fused matmul+bias operations (reduces 2 ops to 1 op)
  • FP16 support: Optional half-precision mode (defaults to FP32 as FP16 doesn't improve performance on Apple Silicon MLX)
  • Batch normalization: Computes merged scale/bias parameters for compatibility with layer tests

Build Instructions

cd cpp
cmake -G Ninja -DUSE_BACKEND=MLX
ninja

Requires CMake 3.27+

Configuration

Enable FP16 (optional, not recommended):

mlxUseFP16 = true

Testing

  • Passes ./katago runtests (unit tests)
  • Passes ./katago runnnlayertests (neural network layer tests)
  • Cross-backend validation via ./katago testgpuerror with Eigen reference
  • Benchmark testing shows functional parity with other backends

Changes

  • cpp/neuralnet/mlxbackend.cpp: New 1518-line implementation
  • cpp/CMakeLists.txt: MLX backend build configuration
  • cpp/configs/gtp_example.cfg: MLX configuration options
  • cpp/main.cpp, cpp/command/benchmark.cpp, cpp/program/setup.cpp: Backend registration

Performance Notes

  • Graph compilation reduces dispatch overhead and enables operation fusion
  • Compiled functions are cached per (batchSize, nnXLen, nnYLen, useMask, hasMeta)
  • FP16 mode available but not recommended (no performance benefit observed)

🤖 Generated with Claude Code

ChinChangYang and others added 5 commits January 15, 2026 21:12
Implements neural network inference using Apple's MLX framework:
- Add mlxbackend.cpp with full model support (conv, batchnorm, residual blocks, policy/value heads)
- Update CMakeLists.txt with MLX backend configuration (requires CMake 3.27+)
- Register MLX in backend prefixes and version info
- Compute merged batchnorm parameters for layer tests compatibility

Build with: cmake -G Ninja -DUSE_BACKEND=MLX && ninja

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use logaddexp for Mish activation (7 ops → 2 ops)
  logaddexp handles numerical stability internally, eliminating
  the need for manual clamping with minimum/where

- Use addmm for fused matmul+bias operations (2 ops → 1 op)
  Applied in SGFMetadataEncoder and ValueHead for reduced
  memory round-trips

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When all boards are exactly nnXLen x nnYLen, all mask values are 1,
so mask operations can be skipped for better performance. This follows
the same optimization pattern used in the CUDA backend.

Changes:
- Store requireExactNNLen in ComputeHandle
- Add useMask parameter to layer apply() methods
- Skip mask multiplication in BatchNormLayer when useMask=false
- Use direct max instead of mask-aware max in global pooling
- Skip trunk * mask multiplication when useMask=false
- Pre-compute fixed maskSum when requireExactNNLen=true

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use mx::compile() to JIT-compile inference functions into fused Metal
kernels. The compiled functions are cached per configuration
(batchSize, nnXLen, nnYLen, useMask, hasMeta) for reuse.

Key changes:
- Add applyArrays() method for mx::array-based inference
- Add createCompiledFunc() to compile inference lambda
- Add applyCompiled() to use pre-compiled functions
- Add thread-safe compilation cache to ComputeHandle
- Modify NeuralNet::getOutput() to use compiled execution

The compilation reduces dispatch overhead and enables MLX to fuse
compatible operations (element-wise chains, matmul+bias, etc).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add optional FP16 (half precision) computation mode to MLX backend:
- All neural network layers now accept useFP16 parameter
- Weights are converted to FP16 at load time when enabled
- Inputs converted to FP16, outputs converted back to FP32
- Cache keys include FP16 mode to avoid mixing compiled functions

Default to FP32 since benchmarks show FP16 does not improve
performance on Apple Silicon MLX. Users can opt-in via
mlxUseFP16=true in config.

Also adds FP16 status reporting to benchmark command.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ChinChangYang ChinChangYang marked this pull request as draft January 18, 2026 06:29
- Set CMAKE_OSX_DEPLOYMENT_TARGET=13.3 before project() so the
  toolchain probe picks up the correct SDK; fail fast on CMake <3.27
- Add platform/arch guards: Apple-only, arm64-only (checks both
  CMAKE_OSX_ARCHITECTURES and CMAKE_SYSTEM_PROCESSOR)
- 3-tier MLX discovery ordered cheapest-first:
  1. Honor user-supplied -DMLX_ROOT
  2. find_package(MLX 0.18 CONFIG QUIET) for Homebrew users
     (/opt/homebrew/share/cmake/MLX/ is on default search path)
  3. Fall back to 'python -m mlx --cmake-dir' for pip-only installs,
     with actionable error messages on Python/probe failure
- Pin find_package(MLX 0.18) for minimum version
- Document MLX prereqs and install options in Compiling.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ChinChangYang ChinChangYang marked this pull request as ready for review April 23, 2026 13:12
@ChinChangYang

Copy link
Copy Markdown
Owner Author

Here’s what we learned making the MLX backend faster, written as general advice — not about our code, just things you can apply in your own fp32 backend.
The starting problem was that the plain fp32 path was noticeably slower than the MPSGraph backend, and the cost was almost all in the convolutions. Everything useful came from attacking that.
- Replace the generic convolution with a Winograd convolution for the main trunk. That is where the speed comes back. It is the single biggest lever for a fp32 network like this.
- Don’t hand-tune the convolution kernel’s launch settings. Parameterize them and let an autotuner search, ideally copying the method from a mature tuner you already trust. When we did this, the only setting that actually mattered was the data layout (looping so channels are the fast axis); the rest were noise. A hand-guess would almost certainly have tuned the wrong knob.
- Once fp32 is competitive, fp16 is the next big jump, but expect it to break first. In a deep network, fused low-precision accumulation can overflow the fp16 range and produce non-finite outputs, even though no single operation looks dangerous on its own. The fix is cheap: at the accumulation step that overflows (for us, BatchNorm), accumulate in fp32 and store back as fp16. Keep everything else fp16. Done right, the fp32 path is bit-for-bit unchanged and fp16 ends up much faster — for us, roughly a quarter to over a third faster than fp32, at the same accuracy.
- Be very careful how you measure. A thread sweep prints one number per thread count, and a careless parser will grab the slowest one and hand you a fake speedup; this once gave us a bogus “+20%”. Always pin the thread count, run the two versions back to back, and check the raw output. Measured honestly, our fp32 path only ties MPSGraph — the real win is fp16.
So the order is: Winograd to catch up, autotuning to find the one setting that matters, then fp16 (expect an overflow, fix it with fp32 accumulation at the bad step) to pull clearly ahead — and don’t trust any benchmark number whose parser you haven’t read.


Drafted by Claude Opus 4.7. Reviewed, experimented, confirmed, and edited by me.

ChinChangYang and others added 12 commits May 19, 2026 20:49
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ghten tolerance

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… seam

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tive

Move useWinograd before weights in ConvLayer field declarations and
constructor init list, then make weights conditional on !useWinograd.
When Winograd is active, a cheap dummy mx::array(0.0f) is stored instead
of the full OHWI weight tensor, saving ~170MB + load-time CPU for b18.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…E FAIL)

Manual fallback parse (script parser hit known pipefail/regex fragility,
extracted 0.0 for all reps -> spurious GATE PASS; corrected via raw audit).
n=6 (r1..r6, r0 warmup discarded), from PARSED 10/10-positions visits/s:
  Metal: mean=493.71 95%CI=+/-9.69
  MLX  : mean=10.65  95%CI=+/-0.10
MLX-fp32 Winograd is ~46x slower than Metal. GATE FAIL.
Raw audit: tmp.YhWuC1zcRG/bench_raw.txt

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-xform) + harness parser fix

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…4 vs 513.64±5.21 v/s, +3.1%, CI-aware GATE PASS)
@ChinChangYang ChinChangYang marked this pull request as draft May 20, 2026 00:10
ChinChangYang and others added 8 commits May 20, 2026 09:22
Mirrors KataGo's OpenCL Winograd tuner pattern:
- Per-stage InputTransform/OutputUntransform configs ({tg0, tg1} each)
- Exhaustive grid search with shuffle + reference baseline
- Plain-text cache, version-line + KEY=VALUE per OpenCLTuneParams
- One file per (GPU, dims, model) under <homeDataDir>/mlxwinotuning/
- Acceptance: search converges within 5% of {tg0=32, tg1=1} baseline,
  bench_mlx_honest.sh shows no regression vs SP1

Deliberate divergences from OpenCL:
- 2-D output launch (not 3-D); kernel rewrite deferred
- vec/axis/tileSize dropped from schema (dead SP1 seams, never tuned by OpenCL)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Original assertion (winner within 5% of {32,1}'s time) was tautologically
satisfiable: the grid contains {32,1} by construction (tg0=32 in tg0-list,
tg1=1 in tg1-list), so a broken search that only times the baseline still
passes the assertion. Also, 'search-converges from a seed' is wrong category
for exhaustive grid — grid doesn't converge.

Split into two independent assertions per stage:
  (a) winner_time ≤ 0.5 × time({1,32})   - beats the bad seed by 2×
  (b) winner_time ≤ 1.05 × time({32,1})  - within 5% of known optimum

Both measurements taken outside the tuner so the assertion doesn't depend
on the tuner's own measurement plumbing being correct.

Failure-mode coverage:
- Broken-only-times-baseline (returns currentConfig)   → fails (a)
- Broken-picks-random                                  → fails (b)
- Correct search                                       → passes both

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six tasks, TDD, frequent commits:
  Task 1: Schema split in mlxwinograd.h (foundation for downstream)
  Task 2: MLXWinogradTuneParams struct + plain-text save/load (no MLX dep)
  Task 3: mx::eval-based measurement primitive (OpenCL :2172-2206 shape)
  Task 4: Grid search + loadOrAutoTune + search-works test (gated by env)
  Task 5: Wire tuner into ComputeHandle/Model/ConvLayer
  Task 6: Honest benchmark acceptance + traceability commit

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Untransform

Replaces single shared {tg0, tg1, vec, axis, tileSize} struct with two
per-stage structs each carrying only {tg0, tg1}. Mirrors OpenCL's
Conv3x3Params split between transLocalSize* and untransLocalSize*.

Drops vec/axis/tileSize because:
- OpenCL Winograd tuner doesn't tune them either (macro-baked, not search dims)
- SP1 named them as template_arg seams but only ever implemented axis=1, vec=1
- tileSize=4 is structural (F(2,3) ratified in SP1 spec for fp32+fp16 stability)

ConvLayer::apply() uses local defaults; Task 5 will replace with tuner-supplied
values captured at construction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors OpenCLTuneParams::save/load:
- VERSION line at top (VERSION=1)
- '#section' comments + 'KEY=VALUE KEY=VALUE' lines
- Parsed via FileUtils::readFileLines + Global::stripComments
- Corrupt version throws IOError (caller retunes)

isValid() enforces tg0*tg1 <= 1024 (Metal threadgroup cap, identical to
OpenCL line 424). Search loop and measurement primitive come in Tasks 3-4.

Tests: file round-trip, corrupt-version rejection, isValid edge cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-quality review caught two issues:

1. parseKeyValueLine reported "could not parse value" for tokens with
   empty key ("=value") or empty value ("key=") — misleading. Added
   explicit empty-key / empty-value checks before the parse attempt,
   matching the OpenCL reference at opencltuner.cpp:31-34.

2. isValid boundary tg0*tg1 == 1024 was not exercised — the constraint
   is '<= 1024' so 1024 must be valid. Added the at-boundary assertion.

Also removed unreachable 'if(tok.empty()) continue;' (Global::split
never yields empty tokens; the guard was dead code).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds static scoreInputTransform / scoreOutputUntransform in
mlxwinotuner.cpp. Each runs 20 reps with rotation across
{trunkNumChannels, midNumChannels, maxConvChannels3x3}, rep 0 is warmup
(weight 0), remaining 19 reps weighted into a mean wall-ms via mx::eval +
std::chrono::steady_clock. Mirrors OpenCL line 2172-2206 shape.

Uses mx::fast::metal_kernel with distinct kernel names (suffix '_tune')
to keep the lazy-graph cache separate from production.

Search loop and bad-seed convergence assertion come in Task 4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…age cleanup

Code-quality review caught two functional defects and two style issues:

1. Important: timeOneInputTransform/timeOneOutputUntransform measured the
   first eval after building the lazy node, which can include MLX runtime
   warmup / Metal PSO compile overhead. Added an explicit untimed warmup
   pass before the timed pair, so the timing captures steady-state cost.

2. Important: scoreInputTransform/scoreOutputUntransform selected the
   synthetic-input array via channel-count comparison; when
   trunkNumChannels == midNumChannels (common: KataGo b18 has both=384),
   the mid slot is silently dropped because the trunk-comparison matches
   first. Restructured to select by slot-index (0/1/2) determined from
   the switch case directly. ASSERT_UNREACHABLE replaces the dead default.

3. Minor: replaced six [[maybe_unused]] static helpers with an anonymous
   namespace to match the style Task 4 will use for its new helpers.

4. Minor: subsumed by Fix 2's switch restructure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ChinChangYang and others added 5 commits May 21, 2026 19:14
Refactors formatConv3x3Distribution to call a new buildConv3x3Histograms
helper that returns (channels, count) vectors. mlxbackend.cpp will use
this directly at model load to feed both the diagnostic log and the
adaptive tuner with a single descriptor walk.

The pure core takes std::vector<const ConvLayerDesc*> rather than
std::vector<ConvLayerDesc> because ConvLayerDesc has a deleted copy
ctor (desc.h:29); the shim collects pointers into the ModelDesc-owned
descriptors and the test uses a std::deque storage with pointer
indirection. Behavior of formatConv3x3Distribution is preserved: the
total field is now derived as Σ output_c counts, which equals the
old inline total++ since each 3x3 conv contributes exactly one
increment to exactly one output_c bucket.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends ModelInfoForTuning with conv3x3InputHistogram and
conv3x3OutputHistogram vectors, populated at the one production call
site (mlxbackend.cpp) and the five gated-test sites. The existing
mid/maxConvChannels3x3 fields are kept until Task 5; scoring code is
not yet rewired (Task 4 does that). No behavior change in this commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the hardcoded {trunk, mid, max} 20-rep slot rotation in
scoreInputTransform/scoreOutputUntransform with planShapeRotation over
the model's actual 3x3 conv channel distribution. Per-slot diagnostic
helpers become per-shape. Flat-sweep log format changes from
'trunk_ms=X mid_ms=Y max_ms=Z' to 'shape_ms=cNNN:X,...' and gated test
regexes are updated to match. mid/maxConvChannels3x3 fields are no
longer read by anything in this file but remain in the struct (removed
in Task 5).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cleans up dead fields and the per-slot diagnostic symbols replaced in
Task 4. Renames KATAGO_MLX_WINOTUNER_RUN_PER_SLOT_TEST to
KATAGO_MLX_WINOTUNER_RUN_PER_SHAPE_TEST and updates the stale walk-once
comment in formatConv3x3Distribution. ModelInfoForTuning now carries
only the four fields the tuner actually reads.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review of Task 5 found three missed slot→shape renames:
- /tmp/per_slot_consistency.txt → per_shape_consistency.txt
- perSlotStr variable in flatSweepInput/flatSweepOutput → perShapeStr
- formatConv3x3Distribution header comment now reflects delegation

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ChinChangYang

Copy link
Copy Markdown
Owner Author

Update: MLX tuner adaptive scoring landed

Pushed 29 commits to feature/mlx-backend (51c9db60..c656f840). Two related features completed in this batch:

MLX tuner shape diagnostic (predecessor, commits 49e6dccb..67fe303a)

Added two pure-observability log lines so a single tuner run produces empirical data on the model's 3×3 conv shape distribution. No scoring logic change.

MLX tuner adaptive scoring (3cb2df5c..c656f840)

Replaced the hardcoded {trunk, mid, max} three-slot scoring rotation with a model-adaptive rotation driven by planShapeRotation over the loaded model's actual 3×3 conv channel distribution, work-weighted by count × channels.

Motivation: On b18c384nbt (the recommended model), the dominant 3×3 conv runs at C=192 (mid, 72 of 78 calls) but the old scoring weighted it ~33% while the rarely-used C=384 trunk shape received ~67%. The new scoring derives weights from the actual distribution.

Selection rule: top-3 shapes, drop below 3% of total work, 3-rep floor per included shape, proportional remainder.

Validation on this host:

  • runnnlayertests + runtests: all pass
  • Gated tests (LOG_FORMAT, SWEEP, PER_SHAPE): all pass
  • testgpuerror vs Eigen reference: bit-identical to the pre-rewrite MLX FP32 (winrateError max 0.00065%)
  • Benchmark MLX FP16 on b18c384nbt: 491.78 v/s (mean of 2 re-tuned samples) vs pre-rewrite 499.45 — within run-to-run noise

Empirical: the new scoring picked a meaningfully different winner: input tg0=8 tg1=25 wpt=1 vw=4 (vs pre-rewrite tg0=64 tg1=16 wpt=1 vw=2), output tg0=32 tg1=2 wpt=1 (vs tg0=192 tg1=2 wpt=2). Throughput-equivalent on this model.

Specs and plans included in the diff:

  • docs/superpowers/specs/2026-05-21-mlx-tuner-shape-diagnostic-design.md
  • docs/superpowers/plans/2026-05-21-mlx-tuner-shape-diagnostic.md
  • docs/superpowers/specs/2026-05-21-mlx-tuner-adaptive-scoring-design.md
  • docs/superpowers/plans/2026-05-21-mlx-tuner-adaptive-scoring.md

🤖 Generated with Claude Code

ChinChangYang and others added 22 commits May 21, 2026 20:39
Three surgical commits to fix plainly-broken comments, strip
internal-stage labels (SP1-SP5/Task N) from historical comments
while preserving their invariant content, and rewrite the larger
block comments in mlxwinograd.h into present-tense API docs.
Plus one test case to close the planShapeRotation rounding-repair
coverage gap.

No behavior change beyond the added unit test. No cross-backend
re-validation needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A fuller scan during plan-writing found ~80 SP/Task references across
the four MLX backend files, not ~25 as the first cut suggested. The
spec's original Commit 2 site list missed mlxwinotuner.cpp entirely
(~25 sites) and most of mlxbackend.cpp's test-runner SP/Task tags
(~15 sites).

Replace the enumerated table with a principle ("rules of engagement")
and defer per-site detail to the plan document. The principle is what
the user approved during brainstorming; the literal table was
illustrative.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three-task plan implementing the spec at
docs/superpowers/specs/2026-05-21-mlx-backend-comment-cleanup-design.md.

Task 1: Fix plainly broken items + close rounding-repair test gap
(single commit, mlxwinotuner.cpp).

Task 2: De-tag SP/Task historical references across all four MLX
backend files (~80 sites, two function renames, two log string
updates). Single commit.

Task 3: Rewrite the three large block comments in mlxwinograd.h into
present-tense API docs. Single commit.

Every Edit step shows exact old_string and new_string. Test F is
exhaustively specified including the (9, 10) post-repair allocation
derived from {(200, 1), (100, 2)} histogram with tied work
tie-broken by larger C.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four cleanup items in mlxwinotuner.cpp:

1. Replace the dangling "see Step 4.2 comment" pointer (no such
   anchor exists) with a present-tense description of the
   mlxbackend.cpp histogram pre-computation.

2. Rewrite the two for(const auto& [ch, n] : ...) loops to use
   for(const auto& p : ...) ... p.first since n was unused and
   [[maybe_unused]] on structured bindings is not portable
   across older clangs.

3. Add the missing "// Warmup: 1 rep on dominant, discarded."
   comment to scoreInputTransformPerShape and
   scoreOutputUntransformPerShape, matching the placement on
   their score-function siblings.

4. Add Test F to runPlanShapeRotationTests covering the
   Σ lround(weight_i * 19) ≠ 19 rounding-repair branch — the
   existing tests A–E all happen to land on 19 exactly. The
   {(200,1), (100,2)} histogram has tied work tie-broken by C,
   each share is 0.5 → lround(9.5) = 10 each → pre-repair sum
   20 → dominant absorbs -1 → final (9, 10).

Comment-only except for the test addition; behavior unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Strips internal-stage labels (SP1-SP5, Task N) from ~70 comment sites
across the four MLX backend files, plus two function renames whose
suffix carried the SP3 stage label:

  runMLXBatchNormFP16Test_SP3        -> runMLXBatchNormFP16Test
  runMLXConvLayerFP16WinogradTest_SP3 -> runMLXConvLayerFP16WinogradTest

Principle: keep the invariant the comment was documenting; drop the
stage label that names *when* the invariant became true. Pure-history
comments (e.g. "SP5 Task 5: matmulOrient axis removed end-to-end. ...
have been deleted along with the enum.") that point at code that is
already gone are deleted entirely. Specific engineering rationale
(empirical sensitivity sweep numbers, fp16 acceptance criteria) is
preserved.

Two cout log strings updated to match:
  "MLX Winograd Task 7 candidate enumeration validity passed"
    -> "MLX Winograd candidate enumeration validity passed"
  "SP5 flat-sweep convergence (gated) OK"
    -> "flat-sweep convergence (gated) OK"

The three large block comments at mlxwinograd.h:12-15, :161-171,
:291-302 are deferred to the next commit (they need full rewrite into
present-tense API docs, not in-place de-tag).

Comment-only diff plus function renames (no behavior change).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three block comments in mlxwinograd.h that were too dense for the
in-place de-tag pattern of the previous commit because they described
what each tunable knob *is* in terms of when it was added or removed:

  * lines 12-15  — struct preamble ("SP2 tunes (tg0, tg1); SP4 adds
    (wpt, vw, gridOrder). SP5 removed output vw (Task 3), output
    gridOrder (Task 4)...").

  * lines 161-171 — input kernel header (SP5 Task 5 prelude + template-arg
    docs annotated with "Tasks 3+; 1 = SP3 behavior").

  * lines 291-302 — output kernel header ("SP5 Task 3: VW dropped...
    SP5 Task 4: GRID_ORDER dropped... SP5 Task 5: MATMUL_ORIENT dropped").

Rewritten into present-tense API docs that state what the configurable
knobs *are* and what the monomorphic invariants *are*, without naming
internal sprint stages. The empirical Cfast-vs-Tfast sensitivity-sweep
rationale (engineering judgment a future contributor would otherwise
have to re-derive) is preserved.

Comment-only diff.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The MLX backend implementation has been de-tagged of stage references in
prior commits; the remaining brainstorm/plan/acceptance scaffolding is
process-only artifact unneeded in merged history. Compacts the
master->feature/mlx-backend diff by ~14.6k lines.

Removes:
- All 10 MLX design specs under docs/superpowers/specs/
- All 10 MLX implementation plans under docs/superpowers/plans/
- cpp/tools/bench_sp{3,5}_acceptance.sh (SP3/SP5 stages no longer named)
- The lone surviving spec back-reference in mlxbackend.cpp's module
  docstring (rewritten as a self-contained statement of the FP16 invariant)

bench_mlx_honest.sh is retained (not SP-tagged).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CPU-oracle loop in runMLXWinogradTests() declared rng/dist at function
scope, where they remained live for the rest of the function. Five later
per-test blocks redeclared local rng/dist with their own seeds, each
triggering -Wshadow (9 warnings total).

Scope the outer pair into a { } block around just the CPU-oracle loop so
they don't leak into the inner test blocks. No renames; no behavior change.

After: mlxbackend.cpp compiles warning-free; runnnlayertests passes all
14 configurations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Probed re-tuning the MLX Winograd tuner at batch sizes 8/16/32/64: the
winning transform-kernel configs do differ per batch size, but end-to-end
throughput stayed flat within run-to-run noise. Records the finding inline
so the pinning isn't re-investigated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove leftover "SP3"/"Task-3" sprint codenames and "spec §" cross-
references (the design spec was deleted in 2a229c2) from MLX backend
comments, the user-facing gtp_example.cfg, the bench script, and two
test descriptor name strings. Also drop the two header files from
NEURALNET_BACKEND_SOURCES to match every other backend's convention,
and correct the scoreInputTransform comment so it accurately states
that only the dominant shape gets a warmup rep.

Comments, config text, and descriptor strings only; no behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extracts the MLX Winograd test entry point out of the production
backend translation unit. runMLXBatchNormFP16Test /
runMLXConvLayerFP16WinogradTest stay in mlxbackend.cpp (they use
file-local classes) and are called via forward declarations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Completes the MLX test-code extraction: the tuner test entry point
now lives alongside runMLXWinogradTests in the dedicated test
translation unit, leaving mlxwinotuner.cpp focused on tuner logic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two corrections to the USE_BACKEND=MLX path in cpp/CMakeLists.txt:

- Stop pinning CMAKE_OSX_DEPLOYMENT_TARGET. KataGo links Homebrew's
  prebuilt libmlx.dylib, whose minos reflects the macOS it was bottled
  on; that dylib, not this build, sets the real minimum macOS. Pinning a
  lower value stamped a misleading minos on the executable and made the
  linker warn "linking with dylib built for newer version". Letting
  CMake default the target to the build host keeps minos honest.

- Reword the CMake-version guard message and comment so the 3.27 floor
  is attributed to KataGo, not MLX (MLX itself requires only 3.25; 3.27
  matches cmake_policy(VERSION 3.27)).

The METAL path is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The MLX-specific settings block was only present in gtp_example.cfg.
Copy it into the three remaining example configs so MLX backend
documentation is consistent across all of them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The mlxUseFP16 comment cited measured throughput deltas and CI bounds
from one machine. Remove the machine-dependent parenthetical so the
example configs stay generally accurate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The MLX backend hard-coded the pass-policy buffer stride to
`numPolicyChannels`, but `gpoolToPassMul.outChannels` can exceed it for
nets with an extended pass head — humanv0 has 2 spatial policy channels
but 48 pass channels. With the wrong stride, batched memcpy only copied
the first `batchSize * numPolicyChannels` floats from a much larger
source, and the extraction loop read row-r pass values from offset
`r * numPolicyChannels`, hitting zero-initialized memory for r > 0.

Effect: batched humanv0 had `policyProbs[passIdx] = 0` for every batch
row beyond row 0 — `testgpuerror` reported batched-fp32 topPolicyDelta
up to 34.80237% and policyKLDiv up to 0.983418 versus the Eigen
reference, while unbatched and value/score/ownership were all clean.

Fix: derive a `numPolicyPassChannels` field on `Model` from
`desc.policyHead.gpoolToPassMul.outChannels` and use it as the
pass-policy stride in the two `memcpy` calls, `InputBuffers` sizing, the
assertion, and the `getOutput` extraction offset. Only the first 1-2
values per row are still consumed by `NNOutput`, but every row is now
read from its correct offset. For standard nets where
`gpoolToPassMul.outChannels == numPolicyChannels` (b18 uec, 40b v8),
behavior is unchanged — verified clean batched in `testgpuerror`.

After fix, batched humanv0:
  batched fp32 topPolicyDelta max 0.00027% (was 34.80237%)
  batched fp32 policyKLDiv max     0.000000 (was 0.983418)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This ad-hoc script is no longer needed for MLX/Metal comparison work;
benchmarking is driven directly via `katago benchmark` with the standard
configs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ChinChangYang

Copy link
Copy Markdown
Owner Author

Moved to lightvector#1199.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant