Repository navigation
feat(tensor): w.t() returns Wᵀ unmaterialized instead of refusing - #1112
Merged
Merged
Conversation
Closes #1108. #973 made `transpose` throw for a block-quantized weight. That was right about the operation — blocks quantize runs along the input dimension, so a real transpose needs requantization, and what the code did instead was an O(bytes) relabel that fed kernels the wrong byte order. It was wrong about the caller: it meant `x.matmul(w.t())` compiled for a dense weight and threw for a packed one, so the code an author writes depended on which file the user loaded and which device it ran on. Neither is knowable when the model is written. `transpose` now returns `TransposedWeightTensorData` — the shape is the transpose, the payload is still the weight's, and `matmul` recognises it and asks `matmulWeightTransposed` for the product, which relayouts once per weight and caches. `transpose` applied twice unwraps, so `w.t().t()` is `w` again; that involution has been broken for as long as packed data existed. This is not the relabel that was removed. That one claimed to be ordinary packed data, so kernels read canonical bytes as input-block-major ones and returned plausible garbage. The marker is deliberately *not* `PackedBlockStorage`: it has no `packedData`, and every `is PackedBlockStorage` probe answers no. The only route to a product is the primitive. The ordering in `matmul` is load-bearing for exactly that reason — the marker fails every "are you packed?" check, so reaching a dense fast path would read a packed payload as floats. It is consulted first in both the common and the JVM override, above the strict-kernel check in the latter. `AccelerateCpuOps` needs no change: its fast paths guard on `FloatArrayTensorData` and the marker falls through to the base. Gate: scripts/pr-gate.sh — all legs passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
📖 Documentation Preview The documentation has been built successfully for this PR. Generated Files:
Artifacts:
This comment will be updated automatically when the PR is updated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1108.
The problem
#973 made
transposethrow for a block-quantized weight, so this compiled for one weight and threw for another:x.matmul(w.t()) // dense: fine. packed: UnsupportedOperationExceptionThe refusal was right about the operation. Blocks quantize runs along the input dimension, so a real transpose needs requantization, and what the code did before was an O(bytes) relabel that handed kernels the wrong byte order. It was wrong about the caller: whether a weight arrives packed is a property of the file the user loaded and the device it runs on, and the model author knows neither.
Linearhid this by calling the primitive itself; hand-written tensor code did not.The change
transposereturnsTransposedWeightTensorData— shape transposed, payload still the weight's, nothing materialized.matmulrecognises it and routes tomatmulWeightTransposed, which relayouts once per weight and caches (#1096). Applied twice,transposeunwraps, sow.t().t()isw— an involution that has been broken for as long as packed data has existed.Why this is not the relabel that was removed
The transpose #973 deleted was a bare metadata relabel: same bytes, swapped shape, still claiming to be ordinary packed data. Kernels believed it and read canonical bytes as input-block-major ones — not an error, just plausible garbage (
NativeLazyTransposeGroundTruthReproTest, #968, SKaiNET-transformers#307).The marker is not
PackedBlockStorage. It has nopackedData, and everyis PackedBlockStorageprobe answers no, so no kernel can reach the bytes through it. The only route to a product is the primitive that relayouts properly.That is also why the ordering in
matmulis load-bearing: because the marker fails every "are you packed?" check, reaching a dense fast path would read a packed payload as floats. It is consulted first in both the common implementation and the JVM override — in the latter above the strict-kernel check, which would otherwise report a missing kernel for an operand that has a perfectly good one behind the marker.AccelerateCpuOpsneeds no change; its fast paths guard onFloatArrayTensorData, so the marker falls through to the base.Tests
TransposedWeightViewTest, seven cases: routing equals the primitive bit-for-bit across seven encodings, the involution, the marker refusing to look like packed storage, element access mirroring with swapped indices, stability under reuse, dense untouched, and a ground-truth check.Two things about that ground-truth check are worth stating, because both were failures first:
It failed on NaN. Random fixture bytes make invalid fp16 scales, so the K-quants dequantized to NaN and
NaN != NaN. That was the fixture's fault, not the code's. It now hand-builds valid blocks (scale 1.0, min 0.0) for the four encodings with a simple block layout, and leaves the K-quants to the routing and safety tests plus the existing golden parity suite — rather than pretend a NaN fixture proved anything.It has to be able to fail. A ground-truth test here is worthless if the fixture cannot distinguish the two block orders — that is how #968 survived its own tests. So the test also computes the answer you would get reading the blocks in the other order and requires it to differ. If a future fixture ever makes the orders coincide, the test says so instead of passing quietly. Every fixture is three blocks per row for the same reason.
A third failure was mine and target-specific: the test read elements through the repo's fixture idiom, which casts
TensorData<FP32, Byte>to<FP32, Float>for construction. The JVM erases that cast; Kotlin/Wasm checks it and threwClassCastException. It now reads through a star projection — the values compared are quantized codes either way, which is the point of the assertion.What this does not do
The form a weight arrives in is still whatever the loader happened to produce, with no one consulting the backend about what its kernels can feed on. That is #1109 — a declared in-memory form resolved from the file ×
PlannerProfile× kernel capabilities, with repack and dequant priced into theMemoryPlan. With it, the loader would usually hand the kernel bytes already in feed order and this marker would resolve to a matmul with no relayout at all.API: additive — one new public class, one
protectedhelper.Gate
scripts/pr-gate.sh— all legs passed.🤖 Generated with Claude Code