diff --git a/CHANGELOG.md b/CHANGELOG.md index c83163396..c98fa516b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ ### Fixed +- **Streaming GGUF loads fail fast on unsupported tensor types instead of silently skipping them.** + `StreamingGgufParametersLoader` used to emit a `SKIP` progress string for any tensor type outside + its `when` and deliver a model with silently missing weights — the failure then surfaced far away + in the forward pass (the load-time half of the Q4_1 report in + [#654](https://github.com/SKaiNET-developers/SKaiNET/issues/654)). An eager pre-scan of the tensor + directory now throws `IllegalArgumentException` before any tensor is delivered, naming every + offending tensor, its type (including raw values for unknown types), and the supported set; the + per-tensor `else` is a hard error guarding against drift from the new + `SUPPORTED_TENSOR_TYPES` companion set. **Behavior change:** files that previously "loaded" with + skipped tensors now fail at load — the legacy `GgufParametersLoader` already behaved this way. + Q4_0 / Q5_0 / Q5_1 — which had packed `TensorData` and matmul kernels but were missing from the + loader — now load as packed blocks instead of being skipped. Closes + [#919](https://github.com/SKaiNET-developers/SKaiNET/issues/919). - **Random file access on Android: streaming model loads instead of full-file heap loads.** `createRandomAccessSource` unconditionally returned `null` on Android in `skainet-io-gguf`, `skainet-io-safetensors` and `skainet-io-onnx`, forcing every load through the legacy diff --git a/skainet-io/skainet-io-gguf/src/commonMain/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoader.kt b/skainet-io/skainet-io-gguf/src/commonMain/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoader.kt index d1ae2937a..f7d11d780 100644 --- a/skainet-io/skainet-io-gguf/src/commonMain/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoader.kt +++ b/skainet-io/skainet-io-gguf/src/commonMain/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoader.kt @@ -7,7 +7,10 @@ import sk.ainet.lang.tensor.Shape import sk.ainet.lang.tensor.Tensor import sk.ainet.lang.tensor.data.Bf16DenseTensorData import sk.ainet.lang.tensor.data.Fp16DenseTensorData +import sk.ainet.lang.tensor.data.Q4_0BlockTensorData import sk.ainet.lang.tensor.data.Q4_KBlockTensorData +import sk.ainet.lang.tensor.data.Q5_0BlockTensorData +import sk.ainet.lang.tensor.data.Q5_1BlockTensorData import sk.ainet.lang.tensor.data.Q5_KBlockTensorData import sk.ainet.lang.tensor.data.Q6_KBlockTensorData import sk.ainet.lang.tensor.data.Q8_0BlockTensorData @@ -25,13 +28,19 @@ import kotlin.reflect.KClass * Unlike [GgufParametersLoader] (which uses the legacy [GGUFReader] and rejects * quantized types), this loader: * - Uses [StreamingGGUFReader] for memory-efficient parsing - * - Supports quantized types (Q4_K, Q8_0) as packed [TensorData] + * - Supports quantized types ([SUPPORTED_TENSOR_TYPES]) as packed [TensorData] * - Loads tensor data on-demand without heap-loading the full file * - Preserves quantized layout through the loading pipeline * * For F32 and I32 tensors, data is returned as standard dense arrays. * For quantized tensors, data is returned as packed block storage * (e.g., [Q4_KBlockTensorData], [Q8_0BlockTensorData]). + * + * A file containing tensors outside [SUPPORTED_TENSOR_TYPES] (e.g. Q4_1) + * fails fast: [load] throws before any tensor is delivered, naming the + * offending tensors and the supported set, instead of silently skipping + * them and letting the missing weights crash the forward pass later + * (#919). */ public class StreamingGgufParametersLoader( private val sourceProvider: () -> RandomAccessSource, @@ -55,6 +64,7 @@ public class StreamingGgufParametersLoader( ) { StreamingGGUFReader.open(sourceProvider()).use { reader -> val tensors = reader.tensors + failFastOnUnsupportedTensorTypes(tensors) val total = tensors.size.toLong() var current = 0L @@ -127,10 +137,30 @@ public class StreamingGgufParametersLoader( ctx.fromData(packed as sk.ainet.lang.tensor.data.TensorData, dtype) } - else -> { - onProgress(current, total, "SKIP: ${tensorInfo.name} (unsupported type ${tensorInfo.tensorType})") - null + GGMLQuantizationType.Q4_0 -> { + @Suppress("UNCHECKED_CAST") + val packed = Q4_0BlockTensorData.fromRawBytes(shape, rawBytes) + ctx.fromData(packed as sk.ainet.lang.tensor.data.TensorData, dtype) + } + + GGMLQuantizationType.Q5_0 -> { + @Suppress("UNCHECKED_CAST") + val packed = Q5_0BlockTensorData.fromRawBytes(shape, rawBytes) + ctx.fromData(packed as sk.ainet.lang.tensor.data.TensorData, dtype) } + + GGMLQuantizationType.Q5_1 -> { + @Suppress("UNCHECKED_CAST") + val packed = Q5_1BlockTensorData.fromRawBytes(shape, rawBytes) + ctx.fromData(packed as sk.ainet.lang.tensor.data.TensorData, dtype) + } + + else -> throw IllegalStateException( + "StreamingGgufParametersLoader: tensor '${tensorInfo.name}' of type " + + "${tensorInfo.tensorType} passed the load-time pre-scan but has no load " + + "branch — SUPPORTED_TENSOR_TYPES and this when-expression have drifted. " + + "Please report this as a bug." + ) } if (tensor != null) { @@ -189,6 +219,56 @@ public class StreamingGgufParametersLoader( public companion object { + /** + * The tensor types [load] can materialize. The when-expression in [load] + * and the eager pre-scan both derive from this set, so a type added to + * one place cannot silently drift from the other. + */ + public val SUPPORTED_TENSOR_TYPES: Set = setOf( + GGMLQuantizationType.F32, + GGMLQuantizationType.I32, + GGMLQuantizationType.F16, + GGMLQuantizationType.BF16, + GGMLQuantizationType.Q4_0, + GGMLQuantizationType.Q5_0, + GGMLQuantizationType.Q5_1, + GGMLQuantizationType.Q4_K, + GGMLQuantizationType.Q5_K, + GGMLQuantizationType.Q6_K, + GGMLQuantizationType.Q8_0, + ) + + private const val MAX_LISTED_TENSORS = 8 + + /** + * Eager pre-scan over the file's tensor directory: throws before any + * tensor is delivered if the file contains types this loader cannot + * materialize. This follows the RFC's "fail before execution" rule + * (see [withPolicy]) — the alternative, skipping the tensor, produces + * a model with silently missing weights whose failure surfaces far + * away in the forward pass (#919). + */ + internal fun failFastOnUnsupportedTensorTypes(tensors: List) { + val unsupported = tensors.filter { it.tensorType !in SUPPORTED_TENSOR_TYPES } + if (unsupported.isEmpty()) return + + val listed = unsupported.take(MAX_LISTED_TENSORS).joinToString(", ") { + val type = if (it.isUnknownType) "unknown type value ${it.rawTypeValue}" else it.tensorType.name + "'${it.name}' ($type)" + } + val more = if (unsupported.size > MAX_LISTED_TENSORS) { + " and ${unsupported.size - MAX_LISTED_TENSORS} more" + } else { + "" + } + throw IllegalArgumentException( + "GGUF contains ${unsupported.size} tensor(s) with quantization types this loader " + + "does not support: $listed$more. Supported types: " + + "${SUPPORTED_TENSOR_TYPES.joinToString(", ") { it.name }}. " + + "Re-quantize the model to a supported format (e.g. Q8_0, Q4_0, Q4_K or F16).", + ) + } + /** * Convenience constructor that takes a [DTypePolicy] and * validates it against the dtypes the GGUF loader supports @@ -214,6 +294,9 @@ public class StreamingGgufParametersLoader( * policy that's satisfiable in principle but happens to * conflict with the specific file's tensors will surface at * iteration time via the `null`-return path in [load]. + * Tensor *types* outside [SUPPORTED_TENSOR_TYPES], by + * contrast, fail eagerly once the file is opened — see + * [failFastOnUnsupportedTensorTypes]. */ public fun withPolicy( sourceProvider: () -> RandomAccessSource, @@ -254,9 +337,9 @@ public class StreamingGgufParametersLoader( FP32, FP16, BF16 -> Unit else -> throw IllegalArgumentException( "StreamingGgufParametersLoader: Require(${policy.target.name}) is not satisfiable — " + - "this loader produces FP32 / Int32 / Q4_K / Q8_0 tensors only, and does not cast " + - "between source dtypes. Use Any to inherit the source dtype, or open a follow-up " + - "to add a ${policy.target.name} cast path.", + "this loader preserves source tensors (dense FP32/Int32 or packed quantized " + + "blocks) and does not cast between dtypes. Use Any to inherit the source dtype, " + + "or open a follow-up to add a ${policy.target.name} cast path.", ) } } diff --git a/skainet-io/skainet-io-gguf/src/commonTest/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoaderFailFastTest.kt b/skainet-io/skainet-io-gguf/src/commonTest/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoaderFailFastTest.kt new file mode 100644 index 000000000..bf88939ee --- /dev/null +++ b/skainet-io/skainet-io-gguf/src/commonTest/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoaderFailFastTest.kt @@ -0,0 +1,95 @@ +package sk.ainet.io.gguf + +import kotlin.test.Test +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +/** + * Unit tests for `StreamingGgufParametersLoader.failFastOnUnsupportedTensorTypes` — + * the eager pre-scan that rejects GGUF files containing tensor types the loader + * cannot materialize, instead of silently skipping them and shipping a model with + * missing weights (#919). + */ +class StreamingGgufParametersLoaderFailFastTest { + + private fun tensorInfo( + name: String, + type: GGMLQuantizationType, + rawTypeValue: Int = type.value, + ): StreamingTensorInfo = StreamingTensorInfo( + name = name, + shape = listOf(32u), + tensorType = type, + rawTypeValue = rawTypeValue, + nElements = 32, + nBytes = 32, + relativeOffset = 0, + absoluteDataOffset = 0, + ) + + @Test + fun supported_types_pass_the_pre_scan() { + val tensors = StreamingGgufParametersLoader.SUPPORTED_TENSOR_TYPES.map { + tensorInfo("t_${it.name}", it) + } + // No throw. + StreamingGgufParametersLoader.failFastOnUnsupportedTensorTypes(tensors) + } + + @Test + fun q4_1_fails_the_pre_scan_with_tensor_name_and_supported_set() { + val e = assertFailsWith { + StreamingGgufParametersLoader.failFastOnUnsupportedTensorTypes( + listOf( + tensorInfo("good", GGMLQuantizationType.Q8_0), + tensorInfo("blk.0.ffn_down.weight", GGMLQuantizationType.Q4_1), + ) + ) + } + val msg = e.message ?: "" + assertTrue("blk.0.ffn_down.weight" in msg, "names the tensor: $msg") + assertTrue("Q4_1" in msg, "names the type: $msg") + assertTrue("Supported types" in msg, "lists the supported set: $msg") + assertTrue("good" !in msg, "must not implicate supported tensors: $msg") + } + + @Test + fun unknown_raw_type_value_is_reported_verbatim() { + val e = assertFailsWith { + StreamingGgufParametersLoader.failFastOnUnsupportedTensorTypes( + listOf(tensorInfo("weird", GGMLQuantizationType.UNKNOWN, rawTypeValue = 4711)) + ) + } + val msg = e.message ?: "" + assertTrue("4711" in msg, "reports the raw on-disk type value: $msg") + } + + @Test + fun long_offender_lists_are_truncated_with_a_count() { + val tensors = (0 until 12).map { tensorInfo("bad_$it", GGMLQuantizationType.Q4_1) } + val e = assertFailsWith { + StreamingGgufParametersLoader.failFastOnUnsupportedTensorTypes(tensors) + } + val msg = e.message ?: "" + assertTrue("12 tensor(s)" in msg, "reports the full count: $msg") + assertTrue("and 4 more" in msg, "truncates the listing: $msg") + } + + @Test + fun quant_formats_with_load_branches_are_in_the_supported_set() { + // Q4_0/Q5_0/Q5_1 gained load branches together with the fail-fast (#919); + // this pins them so a refactor can't silently drop them back out. + for (type in listOf( + GGMLQuantizationType.Q4_0, + GGMLQuantizationType.Q5_0, + GGMLQuantizationType.Q5_1, + GGMLQuantizationType.Q8_0, + GGMLQuantizationType.Q4_K, + )) { + assertTrue( + type in StreamingGgufParametersLoader.SUPPORTED_TENSOR_TYPES, + "$type should be supported", + ) + } + } +} diff --git a/skainet-io/skainet-io-gguf/src/jvmTest/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoaderTest.kt b/skainet-io/skainet-io-gguf/src/jvmTest/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoaderTest.kt index 657fc76a5..cba0953eb 100644 --- a/skainet-io/skainet-io-gguf/src/jvmTest/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoaderTest.kt +++ b/skainet-io/skainet-io-gguf/src/jvmTest/kotlin/sk/ainet/io/gguf/StreamingGgufParametersLoaderTest.kt @@ -6,7 +6,6 @@ import sk.ainet.io.JvmRandomAccessSource import sk.ainet.lang.tensor.Shape import sk.ainet.lang.tensor.Tensor import sk.ainet.lang.tensor.data.FloatArrayTensorData -import sk.ainet.lang.tensor.data.Q8_0TensorData import sk.ainet.lang.tensor.storage.PackedBlockStorage import sk.ainet.lang.types.FP32 import java.io.File @@ -14,22 +13,55 @@ import java.io.RandomAccessFile import java.nio.ByteBuffer import java.nio.ByteOrder import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertTrue class StreamingGgufParametersLoaderTest { + private data class TestTensor( + val name: String, + val type: GGMLQuantizationType, + val elementCount: Long, + val data: ByteArray, + ) + + /** F32 data: [1.0, 2.0, 3.0, 4.0] */ + private fun f32Tensor(name: String = "weight_f32"): TestTensor { + val buf = ByteBuffer.allocate(16).order(ByteOrder.LITTLE_ENDIAN) + floatArrayOf(1.0f, 2.0f, 3.0f, 4.0f).forEach { buf.putFloat(it) } + return TestTensor(name, GGMLQuantizationType.F32, 4, buf.array()) + } + /** - * Build a minimal GGUF file with F32 and Q8_0 tensors. - * Reuses the approach from StorageIntegrationTest. + * One block (32 elements) of a simple-quant format: f16 scale 1.0 + * (0x3C00), optional extra header bytes zeroed, then code bytes. + * Block layouts per [GGML_QUANT_SIZES]: Q8_0 = 2+32, Q4_0 = 2+16, + * Q4_1 = 2+2+16, Q5_0 = 2+4+16, Q5_1 = 2+2+4+16. */ - private fun createTestGgufFile(): File { + private fun quantTensor(name: String, type: GGMLQuantizationType): TestTensor { + val (blockElems, blockBytes) = GGML_QUANT_SIZES.getValue(type) + val bytes = ByteArray(blockBytes) + bytes[0] = 0x00 + bytes[1] = 0x3C // f16 1.0 scale + val codeBytes = when (type) { + GGMLQuantizationType.Q8_0 -> 32 + else -> 16 + } + for (i in 0 until codeBytes) { + bytes[blockBytes - codeBytes + i] = (i + 1).toByte() + } + return TestTensor(name, type, blockElems.toLong(), bytes) + } + + /** Build a minimal single-block-per-tensor GGUF file. */ + private fun createGgufFile(tensors: List): File { val file = File.createTempFile("loader_test_", ".gguf") RandomAccessFile(file, "rw").use { raf -> - val buf = ByteBuffer.allocate(4096).order(ByteOrder.LITTLE_ENDIAN) + val buf = ByteBuffer.allocate(8192).order(ByteOrder.LITTLE_ENDIAN) buf.putInt(0x46554747.toInt()) // Magic buf.putInt(3) // Version - buf.putLong(2) // Tensor count + buf.putLong(tensors.size.toLong()) // Tensor count buf.putLong(1) // KV count // KV: "general.architecture" = "test" @@ -41,38 +73,23 @@ class StreamingGgufParametersLoaderTest { buf.putLong(value.size.toLong()) buf.put(value) - // Tensor 1: "weight_f32", F32, shape [4] - val name1 = "weight_f32".encodeToByteArray() - buf.putLong(name1.size.toLong()) - buf.put(name1) - buf.putInt(1) - buf.putLong(4) - buf.putInt(GGMLQuantizationType.F32.value) - buf.putLong(0) - - // Tensor 2: "weight_q80", Q8_0, shape [32] - val name2 = "weight_q80".encodeToByteArray() - buf.putLong(name2.size.toLong()) - buf.put(name2) - buf.putInt(1) - buf.putLong(32) - buf.putInt(GGMLQuantizationType.Q8_0.value) - buf.putLong(16) - - // Alignment padding + var dataOffset = 0L + for (t in tensors) { + val name = t.name.encodeToByteArray() + buf.putLong(name.size.toLong()) + buf.put(name) + buf.putInt(1) // n dims + buf.putLong(t.elementCount) + buf.putInt(t.type.value) + buf.putLong(dataOffset) + dataOffset += t.data.size + } + + // Alignment padding before the data section val padding = (32 - (buf.position() % 32)) % 32 for (i in 0 until padding) buf.put(0) - // F32 data: [1.0, 2.0, 3.0, 4.0] - buf.putFloat(1.0f) - buf.putFloat(2.0f) - buf.putFloat(3.0f) - buf.putFloat(4.0f) - - // Q8_0 data: scale=1.0 (f16 0x3C00) + codes 1..32 - buf.put(0x00.toByte()) - buf.put(0x3C.toByte()) - for (i in 1..32) buf.put(i.toByte()) + for (t in tensors) buf.put(t.data) buf.flip() val bytes = ByteArray(buf.remaining()) @@ -82,20 +99,24 @@ class StreamingGgufParametersLoaderTest { return file } + private fun loadAll(file: File): Map> { + val ctx = DefaultDataExecutionContext() + val loaded = mutableMapOf>() + kotlinx.coroutines.runBlocking { + StreamingGgufParametersLoader( + sourceProvider = { JvmRandomAccessSource.open(file) } + ).load(ctx, FP32::class) { name, tensor -> + loaded[name] = tensor + } + } + return loaded + } + @Test fun `load F32 tensor produces dense float tensor`() { - val file = createTestGgufFile() + val file = createGgufFile(listOf(f32Tensor(), quantTensor("weight_q80", GGMLQuantizationType.Q8_0))) try { - val ctx = DefaultDataExecutionContext() - val loaded = mutableMapOf>() - - kotlinx.coroutines.runBlocking { - StreamingGgufParametersLoader( - sourceProvider = { JvmRandomAccessSource.open(file) } - ).load(ctx, FP32::class) { name, tensor -> - loaded[name] = tensor - } - } + val loaded = loadAll(file) assertTrue("weight_f32" in loaded) val t = loaded["weight_f32"]!! @@ -111,18 +132,9 @@ class StreamingGgufParametersLoaderTest { @Test fun `load Q8_0 tensor produces packed block TensorData`() { - val file = createTestGgufFile() + val file = createGgufFile(listOf(f32Tensor(), quantTensor("weight_q80", GGMLQuantizationType.Q8_0))) try { - val ctx = DefaultDataExecutionContext() - val loaded = mutableMapOf>() - - kotlinx.coroutines.runBlocking { - StreamingGgufParametersLoader( - sourceProvider = { JvmRandomAccessSource.open(file) } - ).load(ctx, FP32::class) { name, tensor -> - loaded[name] = tensor - } - } + val loaded = loadAll(file) assertTrue("weight_q80" in loaded) val t = loaded["weight_q80"]!! @@ -134,9 +146,89 @@ class StreamingGgufParametersLoaderTest { } } + @Test + fun `load Q4_0 Q5_0 Q5_1 tensors produce packed block TensorData`() { + val file = createGgufFile( + listOf( + quantTensor("weight_q40", GGMLQuantizationType.Q4_0), + quantTensor("weight_q50", GGMLQuantizationType.Q5_0), + quantTensor("weight_q51", GGMLQuantizationType.Q5_1), + ) + ) + try { + val loaded = loadAll(file) + + for (name in listOf("weight_q40", "weight_q50", "weight_q51")) { + assertTrue(name in loaded, "$name should load") + val t = loaded[name]!! + assertEquals(Shape(32), t.shape) + assertTrue(t.data is PackedBlockStorage, "$name should be PackedBlockStorage") + } + } finally { + file.delete() + } + } + + @Test + fun `load Q4_1 tensor fails fast before any tensor is delivered`() { + val file = createGgufFile( + listOf( + f32Tensor(), + quantTensor("blk_0_ffn_down_weight", GGMLQuantizationType.Q4_1), + ) + ) + try { + val ctx = DefaultDataExecutionContext() + val loaded = mutableListOf() + val e = assertFailsWith { + kotlinx.coroutines.runBlocking { + StreamingGgufParametersLoader( + sourceProvider = { JvmRandomAccessSource.open(file) } + ).load(ctx, FP32::class) { name, _ -> + loaded.add(name) + } + } + } + // Nothing delivered: the pre-scan runs before the tensor loop. + assertTrue(loaded.isEmpty(), "no tensor may be delivered before the fail-fast, got $loaded") + val msg = e.message ?: "" + assertTrue("blk_0_ffn_down_weight" in msg, "message should name the tensor: $msg") + assertTrue("Q4_1" in msg, "message should name the offending type: $msg") + assertTrue("Supported types" in msg, "message should list the supported set: $msg") + assertTrue("Q8_0" in msg, "message should include a supported alternative: $msg") + } finally { + file.delete() + } + } + + @Test + fun `fail fast lists every unsupported tensor`() { + val file = createGgufFile( + listOf( + quantTensor("bad_one", GGMLQuantizationType.Q4_1), + quantTensor("bad_two", GGMLQuantizationType.Q8_1), + ) + ) + try { + val ctx = DefaultDataExecutionContext() + val e = assertFailsWith { + kotlinx.coroutines.runBlocking { + StreamingGgufParametersLoader( + sourceProvider = { JvmRandomAccessSource.open(file) } + ).load(ctx, FP32::class) { _, _ -> } + } + } + val msg = e.message ?: "" + assertTrue("2 tensor(s)" in msg, "message should count the offenders: $msg") + assertTrue("bad_one" in msg && "bad_two" in msg, "message should name every offender: $msg") + } finally { + file.delete() + } + } + @Test fun `progress callback invoked correctly`() { - val file = createTestGgufFile() + val file = createGgufFile(listOf(f32Tensor(), quantTensor("weight_q80", GGMLQuantizationType.Q8_0))) try { val ctx = DefaultDataExecutionContext() val progressCalls = mutableListOf>()