Skip to content

Commit 2b7a383

Browse files
michalharakalclaude
andcommitted
Deprecate GGUFModelReader legacy facade (W0d of #615)
The pull-style ModelReader.loadTensor(name) interface returns a raw TensorData<*, *> without an ExecutionContext, but the production GGUF loader path (StreamingGgufParametersLoader) is push-style and ctx-bound — it iterates every tensor in the file, dispatches per source dtype (F32, F16, BF16, Q4_K, Q8_0…) into the matching TensorData subtype with explicit logical shape, and calls back per tensor. Wiring the legacy facade as a real implementation would mean extracting a duplicate byte→TensorData helper and reconstructing what StreamingGgufParametersLoader already does. That's a refactor, not an easy win. Instead: clearly @deprecate the facade with a ReplaceWith hint, empty out the metadata/tensors maps (they were already empty), and replace the TODO with a self-explanatory `error(...)` that names the right replacement API and links to the dtype-model doc that W5 will land. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 66d2340 commit 2b7a383

1 file changed

Lines changed: 39 additions & 8 deletions

File tree

skainet-io/skainet-io-gguf/src/commonMain/kotlin/sk/ainet/io/gguf/GGUFModelReader.kt

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,50 @@ package sk.ainet.io.gguf
33
import sk.ainet.io.ModelReader
44
import sk.ainet.io.TensorInfo
55
import sk.ainet.lang.tensor.data.TensorData
6-
import sk.ainet.lang.tensor.Shape
76

8-
class GGUFModelReader : ModelReader {
9-
override val metadata: Map<String, Any> = mutableMapOf()
10-
override val tensors: Map<String, TensorInfo> = mutableMapOf()
7+
/**
8+
* **Legacy facade — use [StreamingGgufParametersLoader] for GGUF loading.**
9+
*
10+
* The pull-style `ModelReader.loadTensor(name)` contract returns a
11+
* raw `TensorData<*, *>` without an `ExecutionContext`. The
12+
* production GGUF loader ([StreamingGgufParametersLoader]) is a
13+
* push-style API bound to a context — it iterates every tensor in
14+
* the file, dispatches per source dtype (F32, F16, BF16, Q4_K,
15+
* Q8_0…) into the matching `TensorData` subtype with explicit
16+
* logical shape, and calls back into user code per tensor.
17+
*
18+
* That push-style API is the right shape for GGUF loading: GGUF
19+
* files store all tensor headers contiguously up front, so iterating
20+
* once is more efficient than seeking back into the file per
21+
* `loadTensor(name)` call. New consumers should construct a
22+
* [StreamingGgufParametersLoader] directly.
23+
*
24+
* This class is kept compiling so existing dependants don't break,
25+
* but `loadTensor` is intentionally a fail-fast stub — using the
26+
* legacy facade for actual GGUF loading silently corrupted dtype
27+
* metadata (no policy hook, no shape verification) and is exactly
28+
* the anti-pattern the dtype-policy RFC (#615) calls out.
29+
*/
30+
@Deprecated(
31+
message = "Use sk.ainet.io.gguf.StreamingGgufParametersLoader instead. " +
32+
"The streaming loader preserves source dtypes (Q4_K, Q8_0, etc.) as packed " +
33+
"TensorData subtypes and threads through DTypePolicy for fail-fast resolution.",
34+
replaceWith = ReplaceWith("StreamingGgufParametersLoader"),
35+
)
36+
public class GGUFModelReader : ModelReader {
37+
override val metadata: Map<String, Any> = emptyMap()
38+
override val tensors: Map<String, TensorInfo> = emptyMap()
1139

1240
override suspend fun loadTensor(name: String): TensorData<*, *> {
13-
val info = tensors[name] ?: error("Tensor $name not found")
14-
// Implementation will use MemoryMappedFileChunk or similar to slice the data
15-
TODO("Not yet implemented: streaming tensor loading for GGUF")
41+
error(
42+
"GGUFModelReader is a legacy facade and does not load GGUF tensors. " +
43+
"Use StreamingGgufParametersLoader(sourceProvider).load(ctx, dtype, onTensorLoaded) " +
44+
"to iterate tensors with their source dtypes preserved (Q4_K, Q8_0, F32, etc.). " +
45+
"See contributing/dtype-model.adoc for the GGUF dtype-mapping reference.",
46+
)
1647
}
1748

1849
override fun close() {
19-
// Close underlying resources (e.g. mapped file)
50+
// Nothing to close — this facade owns no resources.
2051
}
2152
}

0 commit comments

Comments
 (0)