Skip to content

TensorData and TensorStorage are parallel layers — unify the storage model (ownership, views, dtype/encoding, placement): SKEEP-003 discussion anchor #932

Description

@michalharakal

SKaiNET carries two storage abstractions:

  • TensorData (skainet-lang-core/.../tensor/data/) — the live one. Every Tensor holds one; backends dispatch by downcasting to concrete classes/markers (is Q4_KTensorData -> .packedData); all common-code storage is heap arrays.
  • TensorStorage (.../tensor/storage/, 20 files) — a designed descriptor layer with the right concepts (BufferHandle.{Owned,Borrowed,Aliased,FileBacked,DeviceResident}, Placement/MemoryDomain, TensorEncoding, MemoryPlanner, StorageSpec) whose own KDoc says "new loaders, planners, and backends should target TensorStorage directly" — but which no Tensor ever holds. StorageSpec has zero consumers; the planner is never consulted at any allocation; Aliased is never produced; FileBacked and DeviceResident throw in every consumer; LogicalDType.fromDType has no inverse, so the layer structurally cannot back Tensor<T> today.

The result is a set of related, recurring costs:

An SKEEP-003 draft (PR to follow) lays out the analysis and two candidate end-states — (a) TensorStorage becomes the single byte-owner and TensorData a typed view protocol over it; (b) TensorData stays primary and absorbs BufferHandle/Placement, retiring the parallel descriptor — deliberately without a recommendation: the trade-off (dispatch rewrite + dtype coherence vs minimal churn + status quo dispatch) is a maintainer decision. Both directions share two prerequisites (two-way LogicalDType ↔ DType bridge; decide StorageSpec's fate) and one hard constraint: the packed-encoding system (7 GGML block formats, ternary, TurboQuant, kernel dispatch, StableHLO skainet.tensor_encodings export) must survive bit-identically.

Mechanical bugs found during the same audit are filed separately (TensorStorageFactory contract violations, GGUF encoding mapping dropping five formats, transfer-API gaps, rank-broken copyToFloatArray default, memory-diagnostics paper-cuts) — they are fixable under either end-state.

Activity

  1. michalharakal commented on Aug 11, 2026

    @michalharakal
    ContributorAuthor

    The SKEEP-003 draft this issue promised is in the docs tree: docs/modules/skeep/pages/003-unified-tensor-storage.adoc (landed via #933, registered in the SKEEP index/nav; Status: Draft). The end-state question — storage-first vs data-first — is deliberately left open here for maintainer discussion; the two shared prerequisites (two-way LogicalDType bridge, StorageSpec decision) and the packed-encoding preservation constraint hold under either answer.

    Two follow-ups since the draft:

    Discussion of the end-state (or a "B now, A when a device backend is scheduled" sequencing) is the open item on this anchor.

  2. michalharakal commented on Aug 22, 2026

    @michalharakal
    ContributorAuthor

    Decision (2026-08-22): SKEEP-003 is accepted — end-state A (storage-first), delivered with end-state B's incremental mechanics: new types (Storage owns bytes · TensorView interprets them · Tensor is the DSL handle over a view or a graph node · kernels take views) introduced beside the existing ones in package sk.ainet.lang.memory, every TensorData implementation becomes a façade over the view type, dispatch migrates kernel by kernel on declared Format(dtype, encoding) keys, façades are deleted at the next major. All thirteen open design decisions are recorded in the SKEEP (docs PR #1043) and in the committed design record docs/design/memory/memory-architecture-proposal.md + memory-architecture-milestones-prd.md.

    Roadmap (this issue is the umbrella):

    Working rules for every slice: one sub-issue = one feature/<issue>-<slug> branch = one PR into develop; additive or behind a façade / opt-in so develop stays usable after each merge; deprecate-don't-delete; BCV dumps updated; full local test gate (scripts/pr-gate.sh: jvmTest, apiCheck, JS/Wasm, linuxX64Test, assemble, Java API tests) before each PR. First code slice: #1006 (two-way LogicalDType ↔ DType bridge).

  3. michalharakal commented on Aug 23, 2026

    @michalharakal
    ContributorAuthor

    M0 — Know before you load is complete (2026-08-23): all twelve slices merged into develop (last #1059), wrap-up with the acceptance status and the benchmark comparison on #1001. M1 (#1002) starts with the Phase-2 access-path spike #1016.

  4. michalharakal commented on Aug 25, 2026

    @michalharakal
    ContributorAuthor

    Closing: SKEEP-003 is implemented, and the contract it depended on is settled

    This issue asked why TensorData and TensorStorage were parallel layers, and what a unified storage model would look like. SKEEP-003 answered it, three milestones delivered it, and the byte-order contract that the answer kept running into is now written down.

    The model. Storage owns bytes · TensorView interprets them (Shape + Format + Layout + Storage) · Tensor stays the DSL handle · kernels receive views. Format = (DType, TensorEncoding), so a Q4_K weight is Format(FP32, Q4_K) and the logical dtype is never erased by the packing. Scope owns lifetime — Model, Forward (a pre-sized slab, reset per step), Ambient. materialize() is the only copy point, and every conversion the dispatcher inserts is a visible AdapterInserted event rather than a hidden allocation.

    The milestones.

    The contract (#973): block order is now part of the layout, kernels declare the order they read, the relayout is engine-owned and idempotent, and ops.transpose refuses a packed weight instead of performing a per-forward copy of a semantic lie.

    What the work found on the way

    Several defects that were live and silent, none of which this issue set out to find:

    • both GGML ternary decoders walked elements in the wrong order, and GGUF typed those tensors Opaque;
    • a blocked layout assumed its block axis was the last one, so transposing a packed view decoded the wrong elements — zero-copy and wrong;
    • the planner assumed bf16 KV against an FP32 ring, understating a dense cache by 2×;
    • adopted/mapped weights were invisible to plan-vs-actual;
    • three copies of one source factory had drifted apart across format modules;
    • and two CI gaps: Android host tests and plain-JVM module tests had never run in CI at all.

    What stays open, deliberately

    • The skainet-decode sample in SKaiNET-transformers, which owns a model: M1-A2, the tok/s half of M1-A5 and M2-A1's measured run belong there.
    • M2-A5 on an Android device — the mechanism and the fit check exist; the measurement needs hardware with ART.
    • Downstream migration to matmulWeightTransposed / PackedWeights, and making WeightOrientation.OUT_IN the default once that has happened.
    • P7/P8 of the proposal (device placement, graph-level planning) were always post-M2.

    The full record is in docs/design/memory/ — the proposal, the milestone PRD, the M2 acceptance tables, and the packed-weight layout contract.

    Closing. SKEEP-003 is Accepted and implemented; SKEEP-002 keeps its status note about what Android still waits on.

  5. michalharakal commented on Aug 25, 2026

    @michalharakal
    ContributorAuthor

    Two corrections to the closing summary above, so it does not mislead anyone reading it later. Both are things that changed after it was written; neither reopens the issue.

    1. ops.transpose no longer refuses a packed weight.

    The summary says the #973 contract ends with "ops.transpose refuses a packed weight instead of performing a per-forward copy of a semantic lie". The refusal was right about the operation and 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.

    #1108 replaced it. transpose now returns TransposedWeightTensorData — the shape is the transpose, the payload is still the weight's, and the type is deliberately not PackedBlockStorage, so no kernel can reach the bytes through it. matmul recognises the marker and asks matmulWeightTransposed for the product. w.t().t() is w again.

    What the summary says about the contract still holds: transposing block-quantized bytes is still not a representable operation, and nothing performs a per-call copy. Only the way that is expressed changed — from a refusal to a value that knows it is unmaterialized.

    2. The design record is not in docs/design/memory/.

    That directory was deleted in #1106. The record is Antora-only now, under docs/modules/ — explanation/memory-model.adoc, explanation/packed-weight-layout.adoc, explanation/eager-execution.adoc, how-to/plan-model-memory.adoc, and arc42 in reference/architecture.adoc.


    The "stays open, deliberately" list now has issues. It was living only in the prose of a closed issue, which is not tracking:

    The fourth item, downstream migration to matmulWeightTransposed/PackedWeights and then WeightOrientation.OUT_IN by default, has been overtaken: #1109 replaced the three loader policy flags with one resolved WeightForm, and WeightOrientation is deprecated in favour of its shape axis. The migration is now "adopt WeightForm", which is #1129's concern where it matters (the sample) and otherwise a downstream repository's.

  6. michalharakal commented on Aug 26, 2026

    @michalharakal
    ContributorAuthor

    P7/P8 arc (#1131) resolved and closed: placement is resolver-owned (AllocationResolver, #1142–#1144), eager buffer lifetime is the Scope split wired at creation (#1145), and graph-level memory planning stays downstream in IREE — core decides and carries (#1134 rationale, SKEEP-003a). SKEEP-003's 'scheduled for deletion' items (StorageSpec, the old MemoryPlanner, @Place/@Weights) are discharged by #1142, and cross-cutting improvement 5 (placement consulted at creation) by #1143/#1145 in resolver form. First façade-removal slice (#1159, legacy loader axes) in flight. This umbrella stays open as the SKEEP-003 anchor for what remains: #1146 (op outputs through the scope), #1147/#1148 (IREE-milestone carriage + parity harness).

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions