From d126984bfe6711e89ec9200dd40708c6be551899 Mon Sep 17 00:00:00 2001 From: Michal Harakal Date: Wed, 2 Sep 2026 11:23:01 +0200 Subject: [PATCH] feat(compile-hlo)!: strict-by-default conversion errors, indexSelect routing, createBasic parity (#1247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Converter failures were MLIR comments: a graph whose first node failed to lower cascaded through every downstream node and still emitted a compute-free module with an empty return and exit 0. Conversion now aborts by default — ConversionErrorPolicy.STRICT throws HloConversionException on any unconvertible node, and resolveOperands throws MissingOperandException instead of silently dropping unresolved operands and shifting later ones into earlier positional slots (the gather-cascade defect). LENIENT restores the historical comment-and-continue behavior. Also closes two registry gaps the gemma3n export tripped over: the KSP tracing wrapper emits 'indexSelect' but only 'index_select' was routable, and createBasic never registered NeuralNetOperationsConverter (registered in createExtended's relative position so op-name precedence is unchanged). Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 23 +++ .../api/jvm/skainet-compile-hlo.api | 41 +++- .../sk/ainet/compile/hlo/ConversionContext.kt | 26 ++- .../compile/hlo/ConversionErrorPolicy.kt | 53 ++++++ .../ainet/compile/hlo/StableHloConverter.kt | 44 ++++- .../compile/hlo/StableHloConverterFactory.kt | 34 ++-- .../converters/GatherOperationsConverter.kt | 9 +- .../compile/hlo/ConversionStrictnessTest.kt | 179 ++++++++++++++++++ .../ainet/compile/hlo/GatherConverterTest.kt | 10 + 9 files changed, 395 insertions(+), 24 deletions(-) create mode 100644 skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/ConversionErrorPolicy.kt create mode 100644 skainet-compile/skainet-compile-hlo/src/commonTest/kotlin/sk/ainet/compile/hlo/ConversionStrictnessTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 22580be34..a3cccb6d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,29 @@ ## [Unreleased] +### Changed + +- **StableHLO conversion fails loudly by default** + ([#1247](https://github.com/SKaiNET-developers/SKaiNET/issues/1247)): converter failures were + MLIR comments — a graph whose first node failed to lower could cascade through every downstream + node and still "succeed" with an empty `return` and exit 0. `StableHloConverter` and every + `StableHloConverterFactory` entry point now take a `ConversionErrorPolicy` (default `STRICT`): + an unconvertible node throws `HloConversionException`, and an operand whose producer was never + converted throws `MissingOperandException` instead of being silently dropped and shifting later + operands into earlier positions. `ConversionErrorPolicy.LENIENT` restores the historical + comment-and-continue behavior for callers that inspect partially-converted modules. + +### Fixed + +- **`indexSelect` is now routable in the StableHLO gather converter** + ([#1247](https://github.com/SKaiNET-developers/SKaiNET/issues/1247)): the KSP tracing wrapper + emits `indexSelect`, but the registry only knew `index_select`, so traced index-select nodes + (e.g. Gemma per-layer embeddings) could not lower. Both spellings route to the gather lowering. +- **`StableHloConverterFactory.createBasic` registers `NeuralNetOperationsConverter`** + ([#1247](https://github.com/SKaiNET-developers/SKaiNET/issues/1247)): parity with + `createExtended` — a traced model with conv/pool/norm nodes no longer fails to lower via the + basic factory, with registration order preserving existing op-name precedence. + ## [0.52.0] - 2026-09-01 Headline: **the engine stops silently running on the scalar floor.** A downstream Gemma 4 port diff --git a/skainet-compile/skainet-compile-hlo/api/jvm/skainet-compile-hlo.api b/skainet-compile/skainet-compile-hlo/api/jvm/skainet-compile-hlo.api index 424ef2c94..386cbf230 100644 --- a/skainet-compile/skainet-compile-hlo/api/jvm/skainet-compile-hlo.api +++ b/skainet-compile/skainet-compile-hlo/api/jvm/skainet-compile-hlo.api @@ -47,7 +47,8 @@ public final class sk/ainet/compile/hlo/ConversionContext { public fun (Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/lang/graph/ComputeGraph;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;)V public fun (Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/lang/graph/ComputeGraph;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;)V public fun (Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/lang/graph/ComputeGraph;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;)V - public synthetic fun (Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/lang/graph/ComputeGraph;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/lang/graph/ComputeGraph;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;)V + public synthetic fun (Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/lang/graph/ComputeGraph;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun clear ()V public final fun emitComment (Ljava/lang/String;)V public final fun emitEncodingAnnotation (Ljava/lang/String;ILsk/ainet/lang/tensor/ops/TensorSpec;)V @@ -55,6 +56,7 @@ public final class sk/ainet/compile/hlo/ConversionContext { public final fun emitModuleDeclaration (Ljava/lang/String;)V public final fun emitOperation (Ljava/lang/String;)V public final fun getContent ()Ljava/lang/String; + public final fun getErrorPolicy ()Lsk/ainet/compile/hlo/ConversionErrorPolicy; public final fun getExternalParameters ()Ljava/util/List; public final fun getGranularity ()Lsk/ainet/compile/target/OpGranularityPolicy; public final fun getInputNodes (Lsk/ainet/lang/graph/GraphNode;)Ljava/util/List; @@ -74,6 +76,14 @@ public final class sk/ainet/compile/hlo/ConversionContext { public final fun setValueType (Ljava/lang/String;Ljava/lang/String;)V } +public final class sk/ainet/compile/hlo/ConversionErrorPolicy : java/lang/Enum { + public static final field LENIENT Lsk/ainet/compile/hlo/ConversionErrorPolicy; + public static final field STRICT Lsk/ainet/compile/hlo/ConversionErrorPolicy; + public static fun getEntries ()Lkotlin/enums/EnumEntries; + public static fun valueOf (Ljava/lang/String;)Lsk/ainet/compile/hlo/ConversionErrorPolicy; + public static fun values ()[Lsk/ainet/compile/hlo/ConversionErrorPolicy; +} + public abstract class sk/ainet/compile/hlo/ConversionResult { } @@ -153,6 +163,20 @@ public final class sk/ainet/compile/hlo/ExternalParameterRef { public fun toString ()Ljava/lang/String; } +public final class sk/ainet/compile/hlo/HloConversionException : java/lang/IllegalStateException { + public fun (Ljava/lang/String;Ljava/lang/Throwable;)V + public synthetic fun (Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V +} + +public final class sk/ainet/compile/hlo/MissingOperandException : java/lang/IllegalStateException { + public fun (Ljava/lang/String;Ljava/lang/String;ILjava/lang/String;I)V + public final fun getInputPort ()I + public final fun getNodeId ()Ljava/lang/String; + public final fun getOpName ()Ljava/lang/String; + public final fun getSourceNodeId ()Ljava/lang/String; + public final fun getSourceOutputPort ()I +} + public final class sk/ainet/compile/hlo/MlirValidator { public fun ()V public final fun validate (Ljava/lang/String;)Ljava/util/List; @@ -189,7 +213,8 @@ public final class sk/ainet/compile/hlo/StableHloConverter { public fun (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;)V public fun (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;)V public fun (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;)V - public synthetic fun (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;)V + public synthetic fun (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun convert (Lsk/ainet/lang/graph/ComputeGraph;Ljava/lang/String;)Lsk/ainet/compile/hlo/StableHloModule; public static synthetic fun convert$default (Lsk/ainet/compile/hlo/StableHloConverter;Lsk/ainet/lang/graph/ComputeGraph;Ljava/lang/String;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloModule; public final fun convertWithOptimization (Lsk/ainet/lang/graph/ComputeGraph;Lsk/ainet/compile/hlo/StableHloOptimizer;)Lsk/ainet/compile/hlo/StableHloModule; @@ -201,18 +226,22 @@ public final class sk/ainet/compile/hlo/StableHloConverterFactory { public static final fun createBasic (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createBasic (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;)Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createBasic (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; - public static synthetic fun createBasic$default (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloConverter; + public static final fun createBasic (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; + public static synthetic fun createBasic$default (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Ljava/lang/String;Lsk/ainet/compile/target/OpGranularityPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createCustom (Lsk/ainet/compile/hlo/StableHloOperationRegistry;)Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createCustom (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;)Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createCustom (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;)Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createCustom (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; - public static synthetic fun createCustom$default (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloConverter; + public static final fun createCustom (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; + public static synthetic fun createCustom$default (Lsk/ainet/compile/hlo/StableHloOperationRegistry;Lsk/ainet/compile/hlo/TypeMapper;Lsk/ainet/compile/hlo/MlirValidator;Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createExtended ()Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createExtended (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; - public static synthetic fun createExtended$default (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloConverter; + public static final fun createExtended (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; + public static synthetic fun createExtended$default (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createFast ()Lsk/ainet/compile/hlo/StableHloConverter; public static final fun createFast (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; - public static synthetic fun createFast$default (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloConverter; + public static final fun createFast (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;)Lsk/ainet/compile/hlo/StableHloConverter; + public static synthetic fun createFast$default (Lsk/ainet/compile/hlo/ConstantMaterializationPolicy;Lsk/ainet/compile/hlo/ConversionErrorPolicy;ILjava/lang/Object;)Lsk/ainet/compile/hlo/StableHloConverter; } public final class sk/ainet/compile/hlo/StableHloExportArchitecture { diff --git a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/ConversionContext.kt b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/ConversionContext.kt index 0c018a3b6..260e7932e 100644 --- a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/ConversionContext.kt +++ b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/ConversionContext.kt @@ -35,7 +35,13 @@ public class ConversionContext @kotlin.jvm.JvmOverloads constructor( * caller from the [sk.ainet.compile.opt.TargetOptimizers] registry and passed in; * `null` = decompose everything (portable default). The emitter only *reads* it. */ - public val granularity: sk.ainet.compile.target.OpGranularityPolicy? = null + public val granularity: sk.ainet.compile.target.OpGranularityPolicy? = null, + /** + * How operand-resolution failures behave: [ConversionErrorPolicy.STRICT] + * (default) throws [MissingOperandException]; [ConversionErrorPolicy.LENIENT] + * reproduces the historical silent-drop (see issue #1247). + */ + public val errorPolicy: ConversionErrorPolicy = ConversionErrorPolicy.STRICT ) { private val valueNames = mutableMapOf() private val valueTypes = mutableMapOf() @@ -78,10 +84,24 @@ public class ConversionContext @kotlin.jvm.JvmOverloads constructor( */ public fun resolveOperands(node: GraphNode): List { val g = graph ?: return emptyList() - return g.edges + val incoming = g.edges .filter { it.destination.id == node.id } .sortedBy { it.destinationInputIndex } - .mapNotNull { getValueName(it.source.id, it.sourceOutputIndex) } + if (errorPolicy == ConversionErrorPolicy.LENIENT) { + // Historical behavior: unresolved operands are dropped, and later + // operands slide into earlier positional slots (issue #1247). + return incoming.mapNotNull { getValueName(it.source.id, it.sourceOutputIndex) } + } + return incoming.map { edge -> + getValueName(edge.source.id, edge.sourceOutputIndex) + ?: throw MissingOperandException( + nodeId = node.id, + opName = node.operation.name, + inputPort = edge.destinationInputIndex, + sourceNodeId = edge.source.id, + sourceOutputPort = edge.sourceOutputIndex, + ) + } } /** diff --git a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/ConversionErrorPolicy.kt b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/ConversionErrorPolicy.kt new file mode 100644 index 000000000..e2c6640fd --- /dev/null +++ b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/ConversionErrorPolicy.kt @@ -0,0 +1,53 @@ +package sk.ainet.compile.hlo + +/** + * Governs how the converter reacts when a node cannot be lowered. + * + * Historically every failure was emitted as an MLIR comment and conversion + * continued, so a broken graph could produce a compute-free module that + * still "succeeded" (empty `return`, exit 0) — see issue #1247, where an + * operand-linkage failure at the first node cascaded through 1000+ nodes + * silently. [STRICT] makes any conversion failure a thrown exception; + * [LENIENT] preserves the historical comment-and-continue behavior for + * callers that diff or inspect partially-converted modules. + */ +public enum class ConversionErrorPolicy { + /** Any node that fails to convert aborts the conversion with an exception. Default. */ + STRICT, + + /** Failures become MLIR comments and conversion continues (pre-#1247 behavior). */ + LENIENT, +} + +/** + * A node's input operand could not be resolved to an SSA value — its + * producer was never converted (or was converted under a different name). + * + * Thrown by [ConversionContext.resolveOperands] under + * [ConversionErrorPolicy.STRICT] instead of silently dropping the operand, + * which both hid the failure and shifted later operands into earlier + * positional slots (issue #1247: a gather whose weight operand failed saw + * its indices operand slide into slot 0). + */ +public class MissingOperandException( + public val nodeId: String, + public val opName: String, + public val inputPort: Int, + public val sourceNodeId: String, + public val sourceOutputPort: Int, +) : IllegalStateException( + "Node '$nodeId' (op '$opName') has no SSA value for input port $inputPort: " + + "producer node '$sourceNodeId' (output port $sourceOutputPort) was never " + + "successfully converted. Under ConversionErrorPolicy.LENIENT this operand " + + "would be silently dropped, shifting later operands into earlier slots." +) + +/** + * A node failed to lower to StableHLO under [ConversionErrorPolicy.STRICT]. + * Carries the same diagnostic text the LENIENT mode would have emitted as + * an MLIR comment, plus the causing exception when one was thrown. + */ +public class HloConversionException( + message: String, + cause: Throwable? = null, +) : IllegalStateException(message, cause) diff --git a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/StableHloConverter.kt b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/StableHloConverter.kt index 9681b4f08..68f22beac 100644 --- a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/StableHloConverter.kt +++ b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/StableHloConverter.kt @@ -28,14 +28,20 @@ public class StableHloConverter @kotlin.jvm.JvmOverloads constructor( /** Selected compile target (iree device id, e.g. "torq"); handed to every context. */ private val target: String? = null, /** Per-target op-granularity policy; handed to every context (null = decompose all). */ - private val granularity: sk.ainet.compile.target.OpGranularityPolicy? = null + private val granularity: sk.ainet.compile.target.OpGranularityPolicy? = null, + /** + * How conversion failures behave: [ConversionErrorPolicy.STRICT] (default) + * throws on the first node that fails to lower; [ConversionErrorPolicy.LENIENT] + * restores the historical comment-and-continue behavior (issue #1247). + */ + private val errorPolicy: ConversionErrorPolicy = ConversionErrorPolicy.STRICT ) { /** * Convert a ComputeGraph to StableHLO MLIR format */ public fun convert(graph: ComputeGraph, functionName: String = "main"): StableHloModule { - val context = ConversionContext(typeMapper, graph, materializationPolicy, target, granularity) + val context = ConversionContext(typeMapper, graph, materializationPolicy, target, granularity, errorPolicy) // Pre-conversion validation (allow orphaned nodes for backward compatibility) val validationResult = graph.validate() @@ -182,6 +188,22 @@ public class StableHloConverter @kotlin.jvm.JvmOverloads constructor( try { processNode(node, context) } catch (e: Exception) { + if (errorPolicy == ConversionErrorPolicy.STRICT) { + // Already-precise diagnostics pass through unwrapped. + if (e is HloConversionException || e is MissingOperandException) throw e + // Quote the name so trailing whitespace / casing surprises are + // visible, and include the registry's full key set so "no + // converter found" failures are self-diagnostic. Note the cause + // may be an unrelated throw from a registered converter — a + // known name here does not mean a registry miss (issue #1247). + val known = registry.getSupportedOperations().sorted().joinToString(", ") + throw HloConversionException( + "Error processing node ${node.id}: op '${node.operation.name}' " + + "(type=${node.operation.type}) threw ${e::class.simpleName}: " + + "${e.message}. Registry known names: [$known]", + e + ) + } context.emitComment("Error processing node ${node.id}: ${e.message}") // Quote the name so trailing whitespace / casing surprises are visible, // and include the registry's full key set so "no converter found" @@ -229,10 +251,20 @@ public class StableHloConverter @kotlin.jvm.JvmOverloads constructor( } } is ConversionResult.Failure -> { + if (errorPolicy == ConversionErrorPolicy.STRICT) { + throw HloConversionException( + "Conversion failed for node ${node.id} (op '${node.operation.name}'): ${result.error}" + ) + } context.emitComment("Conversion failed for node ${node.id}: ${result.error}") result.fallbackComment?.let { context.emitComment(it) } } is ConversionResult.Unsupported -> { + if (errorPolicy == ConversionErrorPolicy.STRICT) { + throw HloConversionException( + "Unsupported operation ${result.operationName} for node ${node.id}: ${result.reason}" + ) + } context.emitComment("Unsupported operation ${result.operationName}: ${result.reason}") } } @@ -317,6 +349,14 @@ public class StableHloConverter @kotlin.jvm.JvmOverloads constructor( if (outputValues.isEmpty()) { // No output values found - this might happen if output nodes failed to convert + if (errorPolicy == ConversionErrorPolicy.STRICT) { + throw HloConversionException( + "Module produced no return values: none of the ${outputNodes.size} " + + "output node(s) [${outputNodes.joinToString(", ") { it.id }}] was " + + "successfully converted. A compute-free module with an empty " + + "return is never servable (issue #1247)." + ) + } context.emitComment("Warning: No output values found for return statement") context.emitLine(" return") } else { diff --git a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/StableHloConverterFactory.kt b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/StableHloConverterFactory.kt index 726c0ebef..2d7688269 100644 --- a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/StableHloConverterFactory.kt +++ b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/StableHloConverterFactory.kt @@ -35,7 +35,8 @@ public object StableHloConverterFactory { public fun createBasic( policy: ConstantMaterializationPolicy = ConstantMaterializationPolicy.InlineAlways, target: String? = null, - granularity: sk.ainet.compile.target.OpGranularityPolicy? = null + granularity: sk.ainet.compile.target.OpGranularityPolicy? = null, + errorPolicy: ConversionErrorPolicy = ConversionErrorPolicy.STRICT ): StableHloConverter { val registry = StableHloOperationRegistry() val typeMapper = TypeMapper() @@ -49,13 +50,21 @@ public object StableHloConverterFactory { // Register linear algebra operations converter registry.register(LinalgOperationsConverter()) - + + // Register neural network operations converter (conv / pool / norms). + // Registered in the same relative position as in createExtended so + // op-name precedence (last-writer-wins per name) stays identical — + // e.g. AttentionOperationsConverter still wins + // scaledDotProductAttention. Previously missing here, so a traced + // model with norms could not lower via createBasic (#1247). + registry.register(NeuralNetOperationsConverter()) + // Register activation operations converter registry.register(ActivationOperationsConverter()) - + // Register shape operations converter registry.register(ShapeOperationsConverter()) - + // Register reduction operations converter registry.register(ReductionOperationsConverter()) // argMax: logits -> index, lowered to reduce-max + broadcast + compare + iota + select + reduce-min @@ -80,7 +89,7 @@ public object StableHloConverterFactory { // LLM front-door op for token-id \u2192 embedding lookups. registry.register(GatherOperationsConverter()) - return StableHloConverter(registry, typeMapper, validator, policy, target, granularity) + return StableHloConverter(registry, typeMapper, validator, policy, target, granularity, errorPolicy) } /** @@ -91,7 +100,8 @@ public object StableHloConverterFactory { @JvmStatic @kotlin.jvm.JvmOverloads public fun createExtended( - policy: ConstantMaterializationPolicy = ConstantMaterializationPolicy.InlineAlways + policy: ConstantMaterializationPolicy = ConstantMaterializationPolicy.InlineAlways, + errorPolicy: ConversionErrorPolicy = ConversionErrorPolicy.STRICT ): StableHloConverter { val registry = StableHloOperationRegistry() val typeMapper = TypeMapper() @@ -139,7 +149,7 @@ public object StableHloConverterFactory { // LLM front-door op for token-id \u2192 embedding lookups. registry.register(GatherOperationsConverter()) - return StableHloConverter(registry, typeMapper, validator, policy) + return StableHloConverter(registry, typeMapper, validator, policy, errorPolicy = errorPolicy) } /** @@ -150,7 +160,8 @@ public object StableHloConverterFactory { @JvmStatic @kotlin.jvm.JvmOverloads public fun createFast( - policy: ConstantMaterializationPolicy = ConstantMaterializationPolicy.InlineAlways + policy: ConstantMaterializationPolicy = ConstantMaterializationPolicy.InlineAlways, + errorPolicy: ConversionErrorPolicy = ConversionErrorPolicy.STRICT ): StableHloConverter { val registry = StableHloOperationRegistry() val typeMapper = TypeMapper() @@ -168,7 +179,7 @@ public object StableHloConverterFactory { registry.register(ScalarOperationsConverter()) registry.register(ConstantOperationsConverter()) - return StableHloConverter(registry, typeMapper, null, policy) + return StableHloConverter(registry, typeMapper, null, policy, errorPolicy = errorPolicy) } /** @@ -182,8 +193,9 @@ public object StableHloConverterFactory { registry: StableHloOperationRegistry, typeMapper: TypeMapper = TypeMapper(), validator: MlirValidator? = MlirValidator(), - policy: ConstantMaterializationPolicy = ConstantMaterializationPolicy.InlineAlways + policy: ConstantMaterializationPolicy = ConstantMaterializationPolicy.InlineAlways, + errorPolicy: ConversionErrorPolicy = ConversionErrorPolicy.STRICT ): StableHloConverter { - return StableHloConverter(registry, typeMapper, validator, policy) + return StableHloConverter(registry, typeMapper, validator, policy, errorPolicy = errorPolicy) } } \ No newline at end of file diff --git a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/converters/GatherOperationsConverter.kt b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/converters/GatherOperationsConverter.kt index cbc05ab67..a9578e509 100644 --- a/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/converters/GatherOperationsConverter.kt +++ b/skainet-compile/skainet-compile-hlo/src/commonMain/kotlin/sk/ainet/compile/hlo/converters/GatherOperationsConverter.kt @@ -46,7 +46,10 @@ import sk.ainet.lang.graph.GraphNode public class GatherOperationsConverter : StableHloOperationConverter { override val supportedOperations: Set = setOf( - "gather", "embedding", "Embedding", "index_select" + // "indexSelect" is the name the KSP tracing wrapper actually emits + // (TensorOps.indexSelect); "index_select" is the framework-style + // alias. Registry lookup is exact, so both must be listed (#1247). + "gather", "embedding", "Embedding", "index_select", "indexSelect" ) override fun convert( @@ -55,7 +58,9 @@ public class GatherOperationsConverter : StableHloOperationConverter { context: ConversionContext ): ConversionResult { return when (node.operation.name.lowercase()) { - "gather", "embedding", "index_select" -> convertGather(node, operands, context) + // "indexSelect".lowercase() is "indexselect", not "index_select" — + // both lowercased spellings must be matched here (#1247). + "gather", "embedding", "index_select", "indexselect" -> convertGather(node, operands, context) else -> ConversionResult.Unsupported( node.operation.name, "Operation not supported by GatherOperationsConverter" diff --git a/skainet-compile/skainet-compile-hlo/src/commonTest/kotlin/sk/ainet/compile/hlo/ConversionStrictnessTest.kt b/skainet-compile/skainet-compile-hlo/src/commonTest/kotlin/sk/ainet/compile/hlo/ConversionStrictnessTest.kt new file mode 100644 index 000000000..2ba67db5d --- /dev/null +++ b/skainet-compile/skainet-compile-hlo/src/commonTest/kotlin/sk/ainet/compile/hlo/ConversionStrictnessTest.kt @@ -0,0 +1,179 @@ +package sk.ainet.compile.hlo + +import sk.ainet.lang.graph.DefaultComputeGraph +import sk.ainet.lang.graph.GraphEdge +import sk.ainet.lang.graph.GraphNode +import sk.ainet.lang.tensor.Tensor +import sk.ainet.lang.tensor.ops.Operation +import sk.ainet.lang.tensor.ops.TensorSpec +import sk.ainet.lang.tensor.ops.ValidationResult +import sk.ainet.lang.types.DType +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Covers the #1247 strictness contract: conversion failures must abort with + * a diagnosable exception instead of degrading into MLIR comments plus an + * empty `return` that exits 0. [ConversionErrorPolicy.LENIENT] preserves the + * historical comment-and-continue behavior for callers that inspect + * partially-converted modules. + */ +class ConversionStrictnessTest { + + // --- resolveOperands --------------------------------------------------- + + @Test + fun strict_resolveOperands_throws_named_exception_for_unresolved_producer() { + val (graph, _, consumer) = twoNodeGraph() + val context = ConversionContext(TypeMapper(), graph) + // The producer was never converted: no SSA name registered for it. + val e = assertFailsWith { + context.resolveOperands(consumer) + } + assertEquals("consumer", e.nodeId) + assertEquals("relu", e.opName) + assertEquals(0, e.inputPort) + assertEquals("producer", e.sourceNodeId) + } + + @Test + fun lenient_resolveOperands_reproduces_silent_drop_and_positional_shift() { + // Two producers into ports 0 and 1; only port 1's producer resolved. + val graph = DefaultComputeGraph() + val p0 = inputNode("p0") + val p1 = inputNode("p1") + val consumer = opNode("consumer", "gather", inputs = 2) + graph.addNode(p0) + graph.addNode(p1) + graph.addNode(consumer) + graph.addEdge(GraphEdge("e0", p0, consumer, 0, 0, p0.outputs[0])) + graph.addEdge(GraphEdge("e1", p1, consumer, 0, 1, p1.outputs[0])) + + val context = ConversionContext( + TypeMapper(), graph, + errorPolicy = ConversionErrorPolicy.LENIENT + ) + context.setValueName("p1", "%arg1") + + // Historical defect, now an explicit contract of LENIENT: the + // unresolved port-0 operand is dropped and port 1 slides into slot 0. + assertEquals(listOf("%arg1"), context.resolveOperands(consumer)) + } + + // --- converter-level strictness ---------------------------------------- + + @Test + fun strict_convert_throws_on_unknown_op_with_node_and_registry_diagnostics() { + val graph = singleOpGraph(opName = "definitelyNotAnOp") + val converter = StableHloConverterFactory.createExtended() + val e = assertFailsWith { + converter.convert(graph, "strict_unknown") + } + val message = e.message ?: "" + assertTrue("definitelyNotAnOp" in message, "exception must name the failing op: $message") + assertTrue("op1" in message, "exception must name the failing node: $message") + assertTrue("Registry known names" in message, "exception must carry the registry key set: $message") + } + + @Test + fun lenient_convert_keeps_comment_fallback_and_exits_normally() { + val graph = singleOpGraph(opName = "definitelyNotAnOp") + val converter = StableHloConverterFactory.createExtended( + errorPolicy = ConversionErrorPolicy.LENIENT + ) + val module = converter.convert(graph, "lenient_unknown") + assertTrue(module.content.contains("Error processing node op1")) + assertTrue(module.content.contains("Known names:")) + } + + @Test + fun strict_convert_throws_on_conversion_failure_result() { + // A gather wired with only one operand: resolveOperands succeeds (the + // one producer is an input) but the converter returns Failure — which + // must now throw instead of emitting a comment. + val graph = DefaultComputeGraph() + val weight = inputNode("W", shape = listOf(8, 4)) + val gather = opNode("embed1", "gather", inputs = 1) + graph.addNode(weight) + graph.addNode(gather) + graph.addEdge(GraphEdge("e1", weight, gather, 0, 0, weight.outputs[0])) + + val converter = StableHloConverterFactory.createExtended() + val e = assertFailsWith { + converter.convert(graph, "strict_arity") + } + val message = e.message ?: "" + assertTrue("embed1" in message, "exception must name the failing node: $message") + assertTrue("2 operands" in message, "exception must carry the converter's error: $message") + } + + // --- createBasic registry parity (#1247: gemma3n harness uses createBasic) --- + + @Test + fun createBasic_routes_neural_net_ops() { + // Previously createBasic lacked NeuralNetOperationsConverter, so a + // traced model with norms could not lower. "No converter found" (a + // registry miss) must not appear for rmsNorm; a Failure from the + // converter itself would surface differently and is acceptable here. + val graph = singleOpGraph(opName = "rmsNorm") + val converter = StableHloConverterFactory.createBasic( + errorPolicy = ConversionErrorPolicy.LENIENT + ) + val module = converter.convert(graph, "basic_rmsnorm") + assertFalse( + module.content.contains("No converter found for operation: rmsNorm"), + "createBasic must register NeuralNetOperationsConverter:\n${module.content}" + ) + } + + // --- fixtures ---------------------------------------------------------- + + private fun twoNodeGraph(): Triple { + val graph = DefaultComputeGraph() + val producer = opNode("producer", "mysteryOp", inputs = 0) + val consumer = opNode("consumer", "relu", inputs = 1) + graph.addNode(producer) + graph.addNode(consumer) + graph.addEdge(GraphEdge("e1", producer, consumer, 0, 0, producer.outputs[0])) + return Triple(graph, producer, consumer) + } + + private fun singleOpGraph(opName: String): DefaultComputeGraph { + val graph = DefaultComputeGraph() + val input = inputNode("in1") + val op = opNode("op1", opName, inputs = 1) + graph.addNode(input) + graph.addNode(op) + graph.addEdge(GraphEdge("e1", input, op, 0, 0, input.outputs[0])) + return graph + } + + private fun inputNode(id: String, shape: List = listOf(2, 2)): GraphNode = GraphNode( + id = id, + operation = fixtureOp("input", "input"), + inputs = emptyList(), + outputs = listOf(TensorSpec(id, shape, "FP32")) + ) + + private fun opNode(id: String, name: String, inputs: Int): GraphNode = GraphNode( + id = id, + operation = fixtureOp(name, "compute"), + inputs = List(inputs) { TensorSpec("in$it", listOf(2, 2), "FP32") }, + outputs = listOf(TensorSpec("$id-out", listOf(2, 2), "FP32")) + ) + + private fun fixtureOp(opName: String, opType: String): Operation = object : Operation { + override val name: String = opName + override val type: String = opType + override val parameters: Map = emptyMap() + override fun execute(inputs: List>): List> = + throw UnsupportedOperationException("test fixture only") + override fun validateInputs(inputs: List): ValidationResult = ValidationResult.Valid + override fun inferOutputs(inputs: List): List = emptyList() + override fun clone(newParameters: Map): Operation = this + override fun serialize(): Map = mapOf("name" to opName, "type" to opType) + } +} diff --git a/skainet-compile/skainet-compile-hlo/src/commonTest/kotlin/sk/ainet/compile/hlo/GatherConverterTest.kt b/skainet-compile/skainet-compile-hlo/src/commonTest/kotlin/sk/ainet/compile/hlo/GatherConverterTest.kt index 7aff69e5c..cc84f96cf 100644 --- a/skainet-compile/skainet-compile-hlo/src/commonTest/kotlin/sk/ainet/compile/hlo/GatherConverterTest.kt +++ b/skainet-compile/skainet-compile-hlo/src/commonTest/kotlin/sk/ainet/compile/hlo/GatherConverterTest.kt @@ -54,6 +54,16 @@ class GatherConverterTest { assertFalse(module.content.contains("Unsupported operation")) } + @Test + fun indexSelect_camel_case_alias_routes_to_same_lowering() { + // The KSP tracing wrapper emits "indexSelect", not "index_select"; + // registry lookup is exact, so the camelCase spelling must be + // routable too (#1247). + val module = buildEmbeddingModule(opName = "indexSelect") + assertTrue(module.content.contains("stablehlo.gather")) + assertFalse(module.content.contains("Unsupported operation")) + } + @Test fun embedding_lowering_carries_canonical_dim_numbers_and_slice_sizes() { val module = buildEmbeddingModule(opName = "embedding")