Skip to content

Commit 98b1a3c

Browse files
wlejonclaude
andcommitted
docs: porting a kernel means porting its arithmetic, not just its signature
The CPU layernorm matched the CUDA slot's arguments, shapes and accumulation semantics exactly, and its header comment said it ported the CUDA kernel. It still returned NaN on every CLIP text row, because it computed the variance as E[x^2] - E[x]^2 while CUDA summed squared deviations. Nothing in the conventions said the numerics had to match — only the contract — so the divergence was invisible to review and, since the parity tests skip without a GPU, invisible to CI on a CPU-only machine. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5855d97 commit 98b1a3c

1 file changed

Lines changed: 1 addition & 0 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ Tests live under `tests/`, enabled by `BROTENSOR_TESTS=ON` (default ON when stan
7575
- **Dispatch is runtime, per-operand.** Each public op in `ops.h` is a thin wrapper in `src/ops.cpp`. The wrapper calls `detail::dispatch(...)`, which resolves the op's device from the first *committed* operand (`data != nullptr`), verifies every other committed operand agrees (throws on mismatch), and returns that backend's `OpsVTable`. An *uncommitted* output (`data == nullptr`) is a wildcard — skipped by the check, then pinned to the resolved device via `adopt_output` before the backend impl allocates it. A null vtable slot means the backend doesn't implement that op; the wrapper throws "not implemented on <device>".
7676
- **The op list is one X-macro.** `detail/op_table.h`'s `BROTENSOR_FOR_EACH_OP` is the single source of truth. It expands into the `OpsVTable` struct, the `src/ops.cpp` wrappers, and each backend's registration table — so the public surface and every backend stay in sync by construction.
7777
- **Op signatures mirror across CPU and GPU.** The vtable slot signature *is* the public signature. Same argument order, same shape contracts, same accumulation semantics for backward (caller zeros dW/dB; op accumulates). When adding a CPU op that already has a GPU counterpart, port the contract verbatim and document any FP32-only restriction.
78+
- **Porting a kernel means porting its arithmetic, not just its signature.** A CPU op can match the GPU slot's arguments, shapes and accumulation semantics exactly and still be wrong, because the numerics differ. Reductions are where this bites: compute a variance as the sum of squared deviations from the mean (two passes), never as `E[x^2] - E[x]^2`. The one-pass form cancels catastrophically as soon as a row's mean dwarfs its spread — both terms land on the same large value, their FP32 difference is rounding noise, and a negative variance makes `rstd` NaN. A CLIP text row sitting near 395 is enough to trigger it. `tests/test_layernorm_stability.cpp` pins this down for layernorm; the same rule holds for group_norm, batch_norm and the fused resblock norms. If a comment says a kernel "ports" its counterpart in another backend, the formula has to match too.
7879
- **CPU is FP32-only, but covers the whole FP32 surface.** The CPU backend implements essentially every op's FP32 forward *and* backward — the dense/attention/loss/optim core, the audio family, the vision primitives, the diffusion samplers, flash attention. It is **not** a thin subset; it's the simple, correct reference. What it doesn't do: FP16 / BF16 / INT8-W8A16 / GGUF-quant paths — those exist on the GPU because they pay for themselves there. Don't add them to the CPU side; the CPU backend leaves those vtable slots null and the dispatcher throws "not implemented on CPU".
7980
- **GPU dtype dispatch is on `Tensor::dtype`.** Ops select FP32 vs FP16 vs BF16 (vs INT8 for W8A16) internally; the public surface takes a single `Tensor&` per arg. `Dtype` is `FP32 / FP16 / BF16 / INT8 / INT32 / F64` plus the GGUF block-quant carriers (`Q4_0 … Q8_K`). FP32/FP16/BF16 are the arithmetic dtypes (BF16 GPU-only; FP16/BF16 are `uint16_t` bit patterns on the host). INT8/INT32 are storage carriers for quantised weights and index/offset buffers; the GGUF quant dtypes are non-element-addressable block carriers consumed only by the GGUF dequant / fused-matmul ops — no general arithmetic op dispatches on any of them. Element/block sizing goes through `dtype_size_bytes` / `dtype_block_size` / `dtype_block_bytes` / `dtype_storage_bytes` / `dtype_is_quant` (quant dtypes return 0 from `dtype_size_bytes` — use `dtype_storage_bytes`).
8081
- **Backend-resident storage stays opaque.** GPU `.cu` / `.mm` files include `<brotensor/tensor.h>` and treat `Tensor::data` as a raw device pointer (CUDA) or resolve it to its `MTLBuffer` via `metal_interop.h` (Metal). Use `from_host` / `to` / `copy_to_host` for host transfers.

0 commit comments

Comments
 (0)