This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8348-004a524c27f7b30a2ed02154e203b6b9cda4fe7c in repository https://gitbox.apache.org/repos/asf/texera.git
commit 5127b78bc5dfd48a904894792c299ec72ba07ab9 Author: Kary Zheng <[email protected]> AuthorDate: Mon Sep 28 21:28:12 2026 +0000 feat(machine-learning): export the four advanced trainers (#8348) ### What changes were proposed in this PR? The four advanced trainers implement `StandaloneCodeGenerator`. Each reads its hyperparameters from a table the user fills in rather than from fields of its own, so the script has to resolve, for every row, which parameter was chosen and what converter its value passes through before it reaches the estimator. `JsonSchemaCustomizer` and its caller in `OperatorMetadataGenerator` come along because these four implement it. The rules a hyperparameter's value follows — the set it may take, the bound it must respect — are written into the schema at generation time, and without the call the schema carries none of them. The KNN, SVC and SVR parameter tables gain the accepted values and the bounds each parameter actually admits, read from scikit-learn's own constraints rather than from recall. ### Any related issues, documentation, discussions? Part of #8325, 18 of 27; that issue lists the set in order. It needs #8327 for the trait, so it does not compile until that lands, and the rows these operators add to the verification runner follow with the harness rather than as whole new files here. Closes #8010, the label column offered as one of its own inputs. Part of #7979, #7952 and #7936, whose remaining halves are in #8350. Closes #8420, the task this change is the whole of. ### How was this PR tested? Each operator asserts the block it emits in its own spec. Once the verification lands it is also run through the engine and through its generated script, on every configuration its schema offers, and the two answers compared; this branch is cut from main and does not carry that machinery, so those runs are not on this diff's CI. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]> --- .../KNNTrainer/SklearnAdvancedKNNParameters.java | 39 ++- .../SVCTrainer/SklearnAdvancedSVCParameters.java | 45 ++- .../SVRTrainer/SklearnAdvancedSVRParameters.java | 50 +++- .../sklearnAdvanced/base/HyperParameters.scala | 19 ++ .../base/SklearnAdvancedBaseDesc.scala | 306 ++++++++++++++++++++- .../JsonSchemaCustomizer.scala} | 39 +-- .../metadata/OperatorMetadataGenerator.scala | 5 + .../base/SklearnAdvancedBaseDescSpec.scala | 235 +++++++++++++++- .../base/SklearnMLOperatorDescriptorSpec.scala | 17 ++ 9 files changed, 693 insertions(+), 62 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNParameters.java b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNParameters.java index 9ab29dbe61..29cd8ec18e 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNParameters.java +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/KNNTrainer/SklearnAdvancedKNNParameters.java @@ -22,24 +22,35 @@ package org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.KNNTrai import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.base.ParamClass; public enum SklearnAdvancedKNNParameters implements ParamClass { - n_neighbors("n_neighbors", "int"), - p("p", "int"), - weights("weights", "str"), - algorithm("algorithm", "str"), - leaf_size("leaf_size", "int"), - // A metric is named, not measured: "minkowski" and the rest of the accepted - // set are words, so int() rejects every value scikit-learn would take. - metric("metric", "str"), + // Bounds are scikit-learn's own: a neighbour count and a leaf size start at one, and the + // Minkowski power is open at zero. + n_neighbors("n_neighbors", "int", "5") { @Override public String getMinimum() { return ">=1"; } }, + p("p", "int", "2") { @Override public String getMinimum() { return ">0"; } }, + weights("weights", "str", "", "uniform", "distance"), + algorithm("algorithm", "str", "", "auto", "ball_tree", "kd_tree", "brute"), + leaf_size("leaf_size", "int", "30") { @Override public String getMinimum() { return ">=1"; } }, + // A metric is named, not measured: "minkowski" and the rest are words, so the + // int() this used to declare rejected every value scikit-learn would take. The + // set is the one every `algorithm` above accepts -- the tree algorithms take + // fewer metrics than brute force, and naming a brute-only metric here would + // break the moment the sibling knob is moved off `auto`. + metric("metric", "str", "minkowski", "minkowski", "euclidean", "manhattan", + "chebyshev", "cityblock", "l1", "l2"), // The only one that is not a scalar. scikit-learn wants a mapping of extra // keyword arguments for the metric, so the user's text is read as JSON. - metric_params("metric_params", "json.loads"); + metric_params("metric_params", "json.loads", "{}"); private final String name; private final String type; + private final String sampleValue; + private final String[] allowedValues; - SklearnAdvancedKNNParameters(String name, String type) { + SklearnAdvancedKNNParameters( + String name, String type, String sampleValue, String... allowedValues) { this.name = name; this.type = type; + this.sampleValue = sampleValue; + this.allowedValues = allowedValues; } public String getType() { @@ -49,4 +60,12 @@ public enum SklearnAdvancedKNNParameters implements ParamClass { public String getName() { return this.name; } + + public String getSampleValue() { + return this.sampleValue; + } + + public String[] getAllowedValues() { + return this.allowedValues.clone(); + } } diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVCTrainer/SklearnAdvancedSVCParameters.java b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVCTrainer/SklearnAdvancedSVCParameters.java index c2e6b6df3f..b85f4f6864 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVCTrainer/SklearnAdvancedSVCParameters.java +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVCTrainer/SklearnAdvancedSVCParameters.java @@ -22,20 +22,41 @@ package org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.SVCTrai import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.base.ParamClass; public enum SklearnAdvancedSVCParameters implements ParamClass { - C("C", "float"), - kernel("kernel", "str"), - gamma("gamma", "float"), - degree("degree", "int"), - coef0("coef0", "float"), - tol("tol", "float"), - probability("probability", "(lambda value: value.lower() == \"true\")"); + // Bounds are scikit-learn's own, whose ranges are open at zero for the two below and + // closed for degree. + C("C", "float", "1.0") { @Override public String getMinimum() { return ">0"; } }, + kernel("kernel", "str", "", "rbf", "linear", "poly", "sigmoid", "precomputed"), + // gamma takes either of two words or a number, so no converter of a name covers it, and + // the pattern states what this one takes. Digits are [0-9] rather than \d so the browser + // and Python read it alike. It lets a negative through for the estimator to refuse, since + // excluding the sign would also exclude -0.0, which the estimator takes. + gamma( + "gamma", + "(lambda value: value.strip() if value.strip() in (\"scale\", \"auto\") else float(value))", + "scale") { + @Override + public String getPattern() { + return "^\\s*(?:scale|auto|[-+]?(?:(?:[0-9]+(?:_[0-9]+)*)?\\.(?:[0-9]+(?:_[0-9]+)*)" + + "|(?:[0-9]+(?:_[0-9]+)*)\\.?)(?:[eE][-+]?[0-9]+(?:_[0-9]+)*)?)\\s*$"; + } + }, + degree("degree", "int", "3") { @Override public String getMinimum() { return ">=0"; } }, + // coef0 is the one parameter here with no bound at either end. + coef0("coef0", "float", "0.0"), + tol("tol", "float", "0.001") { @Override public String getMinimum() { return ">0"; } }, + probability("probability", "(lambda value: value.lower() == \"true\")", "", "false", "true"); private final String name; private final String type; + private final String sampleValue; + private final String[] allowedValues; - SklearnAdvancedSVCParameters(String name, String type) { + SklearnAdvancedSVCParameters( + String name, String type, String sampleValue, String... allowedValues) { this.name = name; this.type = type; + this.sampleValue = sampleValue; + this.allowedValues = allowedValues; } public String getType() { @@ -45,4 +66,12 @@ public enum SklearnAdvancedSVCParameters implements ParamClass { public String getName() { return this.name; } + + public String getSampleValue() { + return this.sampleValue; + } + + public String[] getAllowedValues() { + return this.allowedValues.clone(); + } } diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVRTrainer/SklearnAdvancedSVRParameters.java b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVRTrainer/SklearnAdvancedSVRParameters.java index 110b0f4a68..c631b95ee4 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVRTrainer/SklearnAdvancedSVRParameters.java +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVRTrainer/SklearnAdvancedSVRParameters.java @@ -22,24 +22,42 @@ package org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.SVRTrai import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.base.ParamClass; public enum SklearnAdvancedSVRParameters implements ParamClass { - C("C", "float"), - kernel("kernel", "str"), - gamma("gamma", "float"), - degree("degree", "int"), - coef0("coef0", "float"), - tol("tol", "float"), - shrinking("shrinking", "(lambda value: value.lower() == \"true\")"), - verbose("verbose", "(lambda value: value.lower() == \"true\")"), - epsilon("epsilon", "float"), - cache_size("cache_size", "int"), - max_iter("max_iter", "int"); + // Bounds are scikit-learn's own; see SVC for the shared ones. + C("C", "float", "1.0") { @Override public String getMinimum() { return ">0"; } }, + kernel("kernel", "str", "", "rbf", "linear", "poly", "sigmoid", "precomputed"), + // Same converter and shape as SVC's gamma -- see there for why each is spelled this way. + gamma( + "gamma", + "(lambda value: value.strip() if value.strip() in (\"scale\", \"auto\") else float(value))", + "scale") { + @Override + public String getPattern() { + return "^\\s*(?:scale|auto|[-+]?(?:(?:[0-9]+(?:_[0-9]+)*)?\\.(?:[0-9]+(?:_[0-9]+)*)" + + "|(?:[0-9]+(?:_[0-9]+)*)\\.?)(?:[eE][-+]?[0-9]+(?:_[0-9]+)*)?)\\s*$"; + } + }, + degree("degree", "int", "3") { @Override public String getMinimum() { return ">=0"; } }, + coef0("coef0", "float", "0.0"), + tol("tol", "float", "0.001") { @Override public String getMinimum() { return ">0"; } }, + shrinking("shrinking", "(lambda value: value.lower() == \"true\")", "", "true", "false"), + verbose("verbose", "(lambda value: value.lower() == \"true\")", "", "false", "true"), + epsilon("epsilon", "float", "0.1") { @Override public String getMinimum() { return ">=0"; } }, + cache_size("cache_size", "int", "200") { @Override public String getMinimum() { return ">0"; } }, + // -1 is SVR's own value for no iteration limit, which is also why the bound is -1 and not + // zero: the sentinel has to remain reachable. + max_iter("max_iter", "int", "-1") { @Override public String getMinimum() { return ">=-1"; } }; private final String name; private final String type; + private final String sampleValue; + private final String[] allowedValues; - SklearnAdvancedSVRParameters(String name, String type) { + SklearnAdvancedSVRParameters( + String name, String type, String sampleValue, String... allowedValues) { this.name = name; this.type = type; + this.sampleValue = sampleValue; + this.allowedValues = allowedValues; } public String getType() { @@ -49,4 +67,12 @@ public enum SklearnAdvancedSVRParameters implements ParamClass { public String getName() { return this.name; } + + public String getSampleValue() { + return this.sampleValue; + } + + public String[] getAllowedValues() { + return this.allowedValues.clone(); + } } diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/HyperParameters.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/HyperParameters.scala index 13fdb9aa60..3a8a76e632 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/HyperParameters.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/HyperParameters.scala @@ -27,8 +27,27 @@ import org.apache.texera.amber.operator.metadata.annotations.{ HideAnnotation } +/** + * One row of a trainer's parameter table. `parametersSource` decides which of the two inputs + * the row uses, and the hide rules below show only that one, so exactly one of them is needed + * and neither can be required outright. + */ +@JsonSchemaInject(json = """ +{ + "allOf": [ + { + "if": { "properties": { "parametersSource": { "const": true } } }, + "then": { "required": ["attribute"] }, + "else": { "required": ["value"] } + } + ] +} +""") class HyperParameters[T] { + // Two rows naming one parameter emit its keyword argument twice, which the generated + // Python will not compile, so the form warns on the row rather than letting it be added. + @JsonSchemaInject(json = """{ "uniqueAmongRows": true }""") @JsonProperty(required = true) @JsonSchemaTitle("Parameter") @JsonPropertyDescription("Choose the name of the parameter") diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDesc.scala index ba04aa5cbf..8ee4ba5898 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDesc.scala @@ -20,25 +20,91 @@ package org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.base import com.fasterxml.jackson.annotation.{JsonIgnore, JsonProperty, JsonPropertyDescription} -import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle +import com.kjetland.jackson.jsonSchema.annotations.{JsonSchemaInject, JsonSchemaTitle} import org.apache.texera.amber.core.tuple.{Attribute, AttributeType, Schema} import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString import org.apache.texera.amber.core.workflow.{InputPort, OutputPort, PortIdentity} -import org.apache.texera.amber.operator.PythonOperatorDescriptor +import org.apache.texera.amber.operator.{PythonOperatorDescriptor, StandaloneCodeGenerator} import org.apache.texera.amber.operator.metadata.annotations.{ AutofillAttributeName, - AutofillAttributeNameList + AutofillAttributeNameList, + SampleColumn +} +import org.apache.texera.amber.operator.metadata.{ + JsonSchemaCustomizer, + OperatorGroupConstants, + OperatorInfo } -import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorInfo} import org.apache.texera.amber.pybuilder.PythonTemplateBuilder +import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.pyStringLiteral +import org.apache.texera.amber.util.JSONUtils.objectMapper +import com.fasterxml.jackson.databind.node.ObjectNode +import java.lang.reflect.{ParameterizedType, Type} +import scala.util.Try + +/** + * One hyperparameter a trainer offers. `getName` is the keyword argument passed to the + * estimator and `getType` the callable converting the user's text, so together they already + * decide which values the estimator can be given. The two below state that so the form can + * hold the user to it, rather than leaving it to be discovered when `fit` raises. + */ trait ParamClass { def getName: String def getType: String + + /** One value this parameter accepts, offered as an example rather than imposed as a + * default: what the estimator itself documents as its default is the obvious choice, but + * nothing here decides a hyperparameter on the user's behalf. Empty for a parameter with an + * accepted set, which already names every value worth offering, and empty where even the + * estimator's own default is a value `getType` cannot convert, since an example the + * operator would then reject is worse than none. + */ + def getSampleValue: String + + /** The values the estimator accepts, where it accepts a fixed set rather than a range, the + * estimator's own default first. Empty for a parameter taking any number, which its + * converter already constrains. + */ + def getAllowedValues: Array[String] + + /** The shape the value takes, for a parameter that accepts neither a fixed set nor a plain + * number but a choice between them. Empty for every parameter one of the other two + * describes, which is most of them. + */ + def getPattern: String = "" + + /** How low the value may go, written the way the estimator's own range reads: `">0"` where + * zero itself is refused and `">=1"` where the bound is included. Empty for a parameter + * bounded by nothing, which a number alone cannot be told from one whose bound nobody + * looked up. + */ + def getMinimum: String = "" } -abstract class SklearnMLOperatorDescriptor[T <: ParamClass] extends PythonOperatorDescriptor { +// Every selected column goes into `fit` untouched, so each has to be one that +// scikit-learn reads as a number. A boolean counts, fitted as 0/1. +// +// A timestamp does not, even though one on its own fits: it stays datetime64, +// and numpy has no type that holds a date beside a number, so the moment a +// timestamp is selected alongside any other column the fit raises +// DTypePromotionError. This rule is read one column at a time and cannot say +// "all of them or none of them", so admitting a timestamp here would admit the +// combination that breaks. +@JsonSchemaInject(json = """ +{ + "attributeTypeRules": { + "Selected Features": { + "enum": ["integer", "long", "double", "boolean"] + } + } +} +""") +abstract class SklearnMLOperatorDescriptor[T <: ParamClass] + extends PythonOperatorDescriptor + with StandaloneCodeGenerator + with JsonSchemaCustomizer { @JsonIgnore def getImportStatements: String @@ -49,6 +115,9 @@ abstract class SklearnMLOperatorDescriptor[T <: ParamClass] extends PythonOperat @JsonSchemaTitle("Parameter Setting") var paraList: List[HyperParameters[T]] = List() + // The label the estimator fits against. Test-only steering: without it the first + // column wins, which on a feature/label table is a feature. + @SampleColumn("a\"b\\c_species") @JsonProperty(required = true) @JsonSchemaTitle("Ground Truth Attribute Column") @JsonPropertyDescription("Ground truth attribute column") @@ -61,6 +130,149 @@ abstract class SklearnMLOperatorDescriptor[T <: ParamClass] extends PythonOperat @AutofillAttributeNameList var selectedFeatures: List[EncodableString] = _ + /** + * State what a `paraList` row's `value` may hold, which depends on the `parameter` chosen + * beside it and so cannot be annotated on a field shared by every parameter. + * + * The rules go under a key of Texera's own rather than as a JSON-Schema `allOf`, for the + * same reason `attributeTypeRules` does, and whose grammar they borrow: the form builder + * merges the members of an `allOf` into a single field, which would leave one control + * carrying every parameter's constraints at once. + */ + override def customizeJsonSchema(schema: ObjectNode): Unit = { + val rowSchema = hyperParameterRowSchema(schema) + if (rowSchema == null) return + val value = rowSchema.path("properties").path("value") + if (!value.isObject) return + + val branches = objectMapper.createArrayNode() + paramConstants.foreach { param => + val condition = objectMapper.createObjectNode() + condition + .putObject("parameter") + .putArray("valEnum") + .add(chosenValueOf(param)) + + val outcome = objectMapper.createObjectNode() + if (param.getAllowedValues.nonEmpty) { + // The accepted set says everything: it names each value a reader could pick and each + // value a sweep should try, so an example alongside it would only repeat one of them. + param.getAllowedValues.foreach(outcome.withArray("enum").add) + } else { + // A pattern is what a parameter offering a choice between a set and a number has + // instead, so it stands in for the type rather than joining it. + if (param.getPattern.nonEmpty) outcome.put("pattern", param.getPattern) + else + valueTypeOf(param).foreach { valueType => + outcome.put("type", valueType) + // A bound belongs to a value read as a number, so it rides with the type rather + // than standing on its own. + addMinimum(param.getMinimum, outcome) + } + if (param.getSampleValue.nonEmpty) outcome.withArray("examples").add(param.getSampleValue) + } + + if (!outcome.isEmpty) { + val branch = objectMapper.createObjectNode() + branch.set[ObjectNode]("if", condition) + branch.set[ObjectNode]("then", outcome) + branches.add(branch) + } + } + if (!branches.isEmpty) + value + .asInstanceOf[ObjectNode] + .putObject("valueRules") + .set[ObjectNode]("allOf", branches) + } + + /** What a chosen `parameter` holds in the config, which a rule's condition has to name to + * hold: the enum constant, since that is what Jackson writes and what the form compares + * against. Every parameter currently spells its constant the way it spells the keyword the + * emitted Python passes on, but the two are free to differ, and only one of them is what + * the config carries. + */ + private def chosenValueOf(param: ParamClass): String = + param match { + case constant: Enum[_] => constant.name + case _ => param.getName + } + + /** Puts a declared bound under the JSON Schema name for it, `>` and `>=` being the two forms + * an estimator's range takes at the low end. A bound spelled any other way is skipped + * rather than guessed at, since a wrong one turns away values that work. + */ + private def addMinimum(bound: String, outcome: ObjectNode): Unit = + if (bound.startsWith(">=")) numberOf(bound.drop(2)).foreach(outcome.put("minimum", _)) + else if (bound.startsWith(">")) + numberOf(bound.drop(1)).foreach(outcome.put("exclusiveMinimum", _)) + + private def numberOf(text: String): Option[Double] = Try(text.trim.toDouble).toOption + + /** How the form should read a value with no fixed set of its own: from the callable the + * parameter names, since that is what the emitted code puts the text through. A parameter + * converted by anything else is left unconstrained rather than guessed at. + */ + private def valueTypeOf(param: ParamClass): Option[String] = + param.getType match { + case "int" => Some("integer") + case "float" | "double" => Some("number") + case _ => None + } + + /** The `HyperParameters` definition this operator's `paraList` points at. Followed by its + * `$ref` rather than by name, which carries the parameter enum and so differs per operator. + */ + private def hyperParameterRowSchema(schema: ObjectNode): ObjectNode = { + val ref = schema.path("properties").path("paraList").path("items").path("$ref").asText("") + val name = ref.stripPrefix("#/definitions/") + if (name.isEmpty) return null + schema.path("definitions").path(name) match { + case row: ObjectNode => row + case _ => null + } + } + + /** The hyperparameters this operator offers, from the enum bound to `T`. The field itself + * cannot say: erasure leaves `paraList` holding a plain `HyperParameters`, so the binding + * survives only on the generic supertype. Empty where `T` is not an enum, which is only + * ever a test stub standing in for one. + */ + private def paramConstants: Seq[ParamClass] = { + var t: Type = getClass.getGenericSuperclass + while (t != null) t match { + case p: ParameterizedType => + val raw = p.getRawType.asInstanceOf[Class[_]] + if (raw == classOf[SklearnMLOperatorDescriptor[_]]) + return p.getActualTypeArguments()(0) match { + case bound: Class[_] => + val constants = bound.getEnumConstants.asInstanceOf[Array[AnyRef]] + if (constants == null) Seq.empty + else constants.toSeq.map(_.asInstanceOf[ParamClass]) + case _ => Seq.empty + } + t = raw.getGenericSuperclass + case c: Class[_] => t = c.getGenericSuperclass + case _ => t = null + } + Seq.empty + } + + /** + * Each row emits one keyword argument, so two rows naming one parameter emit that keyword + * twice and Python rejects the operator with a repeated-keyword SyntaxError. Refused here + * instead, where the workflow fails to compile with the parameter named. + */ + private def requireDistinctParameters(paraList: List[HyperParameters[T]]): Unit = { + val repeated = paraList.map(_.parameter.getName).groupBy(identity).collect { + case (name, rows) if rows.size > 1 => name + } + require( + repeated.isEmpty, + s"Each parameter can be set at most once. Set more than once: ${repeated.toSeq.sorted.mkString(", ")}." + ) + } + private def getLoopTimes(paraList: List[HyperParameters[T]]): PythonTemplateBuilder = { for (ele <- paraList) { if (ele.parametersSource) { @@ -71,6 +283,7 @@ abstract class SklearnMLOperatorDescriptor[T <: ParamClass] extends PythonOperat } def getParameter(paraList: List[HyperParameters[T]]): List[PythonTemplateBuilder] = { + requireDistinctParameters(paraList) var workflowParam = s""; var portParam = pyb""; var paramString = pyb"" @@ -92,6 +305,45 @@ abstract class SklearnMLOperatorDescriptor[T <: ParamClass] extends PythonOperat } + /** Standalone-safe parameter strings: the parameter-table column name is + * emitted as a quoted Python string literal (`table["col"]`). The shared + * [[getParameter]]/[[getLoopTimes]] rely on EncodableString auto-encoding for + * the native path; the standalone `.plain` path would otherwise drop the + * quotes (`table[col]`) and raise NameError. Returns (model-args, para_str). + */ + private def getParameterStandalone(paraList: List[HyperParameters[T]]): (String, String) = { + val workflowParam = new StringBuilder + val portParam = new StringBuilder + val paramString = new StringBuilder + for (ele <- paraList) { + val name = ele.parameter.getName + val typ = ele.parameter.getType + workflowParam.append(s"$name = {},") + if (ele.parametersSource) { + val attrLit = pyStringLiteral(ele.attribute) + portParam.append(s"""$typ(table[$attrLit].values[i]),""") + paramString.append(s"""$name = $typ(table[$attrLit].values[i]),""") + } else { + // The value is written by the user as text and converted by `typ` in the + // emitted code, so it has to arrive as a Python string literal. Numeric + // hyperparameters survived without this because `float(1.0)` happens to + // parse; a string one emitted `str(rbf)` and raised NameError. + val valueLit = pyStringLiteral(ele.value) + portParam.append(s"$typ ($valueLit),") + paramString.append(s"$name = $typ ($valueLit),") + } + } + (paramString.toString, s"""${pyStringLiteral(workflowParam.toString)}.format($portParam)""") + } + + private def getLoopTimesStandalone(paraList: List[HyperParameters[T]]): String = + paraList + .collectFirst { + case ele if ele.parametersSource => + s"""table[${pyStringLiteral(ele.attribute)}].values.shape[0]""" + } + .getOrElse("1") + override def generatePythonCode(): String = { val listFeatures = selectedFeatures.map(feature => pyb"""$feature""").mkString(",") val trainingName = getImportStatements.split(" ").last @@ -171,6 +423,17 @@ abstract class SklearnMLOperatorDescriptor[T <: ParamClass] extends PythonOperat override def getOutputSchemas( inputSchemas: Map[PortIdentity, Schema] ): Map[PortIdentity, Schema] = { + // The features go to `fit` as they are named, so the ground truth among them + // is the answer handed to the estimator as an input. Nothing fails when that + // happens, which is why it is refused here: the run finishes and the model + // scores far better than what it learned deserves. + if (Option(selectedFeatures).exists(_.contains(groundTruthAttribute))) { + throw new RuntimeException( + s""""$groundTruthAttribute" is the Ground Truth Attribute Column, so it cannot""" + + " also be a Selected Feature. Remove it from Selected Features, or fit against" + + " a different column." + ) + } val outputSchema = Schema( List( new Attribute("Model", AttributeType.BINARY), @@ -180,4 +443,37 @@ abstract class SklearnMLOperatorDescriptor[T <: ParamClass] extends PythonOperat Map(operatorInfo.outputPorts.head.id -> outputSchema) } + + override def generateStandaloneCode(): String = { + val listFeatures = selectedFeatures.map(pyStringLiteral).mkString(",") + val trainingName = getImportStatements.split(" ").last + val (trainingParamPlain, paramStringPlain) = getParameterStandalone(paraList) + val loopTimesPlain = getLoopTimesStandalone(paraList) + + // Imported for the same reason the operator path imports it: a hyperparameter + // declaring `json.loads` as its converter names it in the emitted code. + s"""import json + |import pandas as pd + |${getImportStatements} + | + |dataset = in1df + |table = in2df + |features = [$listFeatures] + |rows_read = len(dataset) + |dataset = dataset.dropna(subset=features + [${pyStringLiteral(groundTruthAttribute)}]) + |if len(dataset) < rows_read: + | print("Skipped", rows_read - len(dataset), "of", rows_read, "rows with missing values") + |y_train = dataset[${pyStringLiteral(groundTruthAttribute)}] + |X_train = dataset[features] + |loop_times = $loopTimesPlain + |model_list = [] + |para_list = [] + |for i in range(loop_times): + | model = $trainingName($trainingParamPlain) + | model.fit(X_train, y_train) + | model_list.append(model) + | para_str = $paramStringPlain + | para_list.append(para_str) + |out1df = pd.DataFrame({"Model": model_list, "Parameters": para_list})""".stripMargin + } } diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVCTrainer/SklearnAdvancedSVCParameters.java b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/JsonSchemaCustomizer.scala similarity index 50% copy from common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVCTrainer/SklearnAdvancedSVCParameters.java copy to common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/JsonSchemaCustomizer.scala index c2e6b6df3f..b44065ba96 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/SVCTrainer/SklearnAdvancedSVCParameters.java +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/JsonSchemaCustomizer.scala @@ -17,32 +17,21 @@ * under the License. */ -package org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.SVCTrainer; +package org.apache.texera.amber.operator.metadata -import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.base.ParamClass; +import com.fasterxml.jackson.databind.node.ObjectNode -public enum SklearnAdvancedSVCParameters implements ParamClass { - C("C", "float"), - kernel("kernel", "str"), - gamma("gamma", "float"), - degree("degree", "int"), - coef0("coef0", "float"), - tol("tol", "float"), - probability("probability", "(lambda value: value.lower() == \"true\")"); +/** + * An operator with part of its schema that annotations cannot express, because that part + * depends on the descriptor's own type argument rather than on any one field. + * + * [[OperatorMetadataGenerator.generateOperatorJsonSchema]] calls this once the annotated + * schema is built, so an implementor edits a finished document rather than producing one. + */ +trait JsonSchemaCustomizer { - private final String name; - private final String type; - - SklearnAdvancedSVCParameters(String name, String type) { - this.name = name; - this.type = type; - } - - public String getType() { - return this.type; - } - - public String getName() { - return this.name; - } + /** Add to `schema` what the annotations could not state. Called with the operator's whole + * schema, `definitions` included. + */ + def customizeJsonSchema(schema: ObjectNode): Unit } diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/OperatorMetadataGenerator.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/OperatorMetadataGenerator.scala index fdfcbcf27d..3086ad851c 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/OperatorMetadataGenerator.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/OperatorMetadataGenerator.scala @@ -140,6 +140,11 @@ object OperatorMetadataGenerator { jsonSchema.get("required").asInstanceOf[ArrayNode].remove(operatorTypeIndex) // remove "title" for the operator - frontend uses userFriendlyName to show operator title jsonSchema.remove("title") + // let an operator add the part of its schema that its annotations cannot state + opDescClass.getConstructor().newInstance() match { + case customizer: JsonSchemaCustomizer => customizer.customizeJsonSchema(jsonSchema) + case _ => + } jsonSchema } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDescSpec.scala index 9477539b00..1d8550c51c 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnAdvancedBaseDescSpec.scala @@ -19,18 +19,29 @@ package org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.base +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.node.ObjectNode import org.apache.texera.amber.core.tuple.AttributeType -import org.apache.texera.amber.operator.metadata.OperatorGroupConstants +import org.apache.texera.amber.util.JSONUtils.objectMapper +import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.KNNTrainer.SklearnAdvancedKNNClassifierTrainerOpDesc +import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.SVCTrainer.SklearnAdvancedSVCTrainerOpDesc +import org.apache.texera.amber.operator.machineLearning.sklearnAdvanced.SVRTrainer.SklearnAdvancedSVRTrainerOpDesc +import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorMetadataGenerator} import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import scala.jdk.CollectionConverters.IteratorHasAsScala + class SklearnAdvancedBaseDescSpec extends AnyFlatSpec with Matchers { - // A minimal ParamClass: the generated code only ever consults getName/getType. + // A minimal ParamClass: the generated code only ever consults getName/getType, the other + // two being read by the schema rather than by code generation. private class TestParam(name: String, typ: String) extends ParamClass { override def getName: String = name override def getType: String = typ + override def getSampleValue: String = "" + override def getAllowedValues: Array[String] = Array.empty } // A concrete descriptor supplying just the two abstract hooks; everything @@ -145,4 +156,224 @@ class SklearnAdvancedBaseDescSpec extends AnyFlatSpec with Matchers { paramString should include("n_neighbors = int(table[") paramString should include(".values[i]") } + + "SklearnMLOperatorDescriptor" should + "constrain the selected features to what the estimator can be fitted on" in { + // The columns reach `fit` untouched, so the accepted set is whatever scikit-learn reads + // as a number, a boolean included. Text and binary are left out, and so is timestamp: + // one on its own fits, but selected beside any other column it raises + // DTypePromotionError, and a rule read one column at a time cannot admit the first + // case without admitting the second. + val schema = OperatorMetadataGenerator.generateOperatorJsonSchema( + classOf[SklearnAdvancedKNNClassifierTrainerOpDesc] + ) + schema + .path("attributeTypeRules") + .path("Selected Features") + .path("enum") + .elements() + .asScala + .map(_.asText()) + .toSeq should contain theSameElementsAs Seq( + "integer", + "long", + "double", + "boolean" + ) + } + + // The rules are built from the enum bound to the descriptor's type argument, so they need a + // real operator rather than the stub above, whose TestParam is a class and has no constants. + private def valueRulesOf(opClass: Class[_ <: org.apache.texera.amber.operator.LogicalOp]) = { + val schema = OperatorMetadataGenerator.generateOperatorJsonSchema(opClass) + val row = schema + .path("definitions") + .path( + schema + .path("properties") + .path("paraList") + .path("items") + .path("$ref") + .asText() + .stripPrefix("#/definitions/") + ) + row.path("properties").path("value").path("valueRules").path("allOf") + } + + private def ruleFor(rules: JsonNode, parameter: String): JsonNode = + rules + .elements() + .asScala + .find( + _.path("if") + .path("parameter") + .path("valEnum") + .elements() + .asScala + .exists(_.asText() == parameter) + ) + .map(_.path("then")) + .getOrElse(fail(s"no rule for $parameter")) + + "SklearnMLOperatorDescriptor.customizeJsonSchema" should + "offer a chosen-from-a-set parameter exactly the values scikit-learn accepts" in { + val kernel = ruleFor(valueRulesOf(classOf[SklearnAdvancedSVCTrainerOpDesc]), "kernel") + // scikit-learn's own default leads, so a reader taking the first takes that one + kernel.path("enum").elements().asScala.map(_.asText()).toSeq shouldBe + Seq("rbf", "linear", "poly", "sigmoid", "precomputed") + // an accepted set replaces a type rather than joining it: the values are the constraint + kernel.has("type") shouldBe false + // and it replaces an example too, having already named every value worth offering + kernel.has("examples") shouldBe false + } + + it should "read a parameter with no set of its own from the converter it names" in { + val rules = valueRulesOf(classOf[SklearnAdvancedSVCTrainerOpDesc]) + ruleFor(rules, "C").path("type").asText() shouldBe "number" + ruleFor(rules, "degree").path("type").asText() shouldBe "integer" + } + + it should "carry the estimator's bound under whichever name matches its range" in { + val rules = valueRulesOf(classOf[SklearnAdvancedSVCTrainerOpDesc]) + // C's range is open at zero and degree's is closed, so the two take different names + ruleFor(rules, "C").path("exclusiveMinimum").asDouble() shouldBe 0.0 + ruleFor(rules, "C").has("minimum") shouldBe false + ruleFor(rules, "degree").path("minimum").asDouble() shouldBe 0.0 + ruleFor(rules, "degree").has("exclusiveMinimum") shouldBe false + // coef0 is bounded by nothing, so it gets neither rather than a made-up zero + ruleFor(rules, "coef0").has("minimum") shouldBe false + ruleFor(rules, "coef0").has("exclusiveMinimum") shouldBe false + } + + it should "keep a sentinel value reachable when it lies below the useful range" in { + // SVR's max_iter uses -1 for no limit, so the bound has to admit it + val rules = valueRulesOf(classOf[SklearnAdvancedSVRTrainerOpDesc]) + ruleFor(rules, "max_iter").path("minimum").asDouble() shouldBe -1.0 + } + + it should "describe a parameter choosing between a set and a number with a pattern" in { + // gamma takes either of two words or a number, which no type names, so the rule carries a + // pattern in place of one. Its example is the estimator's own default, a word. + val gamma = ruleFor(valueRulesOf(classOf[SklearnAdvancedSVCTrainerOpDesc]), "gamma") + gamma.has("type") shouldBe false + gamma.path("examples").path(0).asText() shouldBe "scale" + val pattern = gamma.path("pattern").asText() + Seq("scale", "auto", "0.1", "1e-3", " 1 ").foreach(v => v.matches(pattern) shouldBe true) + Seq("abc", "", "scaleauto", "1.2.3").foreach(v => v.matches(pattern) shouldBe false) + } + + it should "offer a boolean parameter the two words its converter takes" in { + // The condition names the parameter as the config spells it, which is the enum constant + // rather than the keyword the emitted Python passes on. The two agree for every parameter + // today, and a condition naming the wrong one would hold for nothing. + val shrinking = ruleFor(valueRulesOf(classOf[SklearnAdvancedSVRTrainerOpDesc]), "shrinking") + shrinking.path("enum").elements().asScala.map(_.asText()).toSeq shouldBe Seq("true", "false") + } + + it should "state a rule for every parameter whose converter says anything about it" in { + val rules = valueRulesOf(classOf[SklearnAdvancedKNNClassifierTrainerOpDesc]) + val covered = rules + .elements() + .asScala + .flatMap(_.path("if").path("parameter").path("valEnum").elements().asScala) + .map(_.asText()) + .toSet + // Every KNN parameter now says something a rule can carry: a converter that narrows + // the value, a set of accepted words, or an example. `metric_params` says the least + // of them — `json.loads` narrows nothing a JSON Schema type can express — but it + // still offers `{}` as the shape to start from. + covered shouldBe Set( + "n_neighbors", + "p", + "weights", + "algorithm", + "leaf_size", + "metric", + "metric_params" + ) + } + + it should "leave a schema it cannot find the row in alone" in { + // The three shapes that make the rules unwritable. None arises from a schema the generator + // produced, so the point is that each is passed over rather than thrown on. + val svc = new SklearnAdvancedSVCTrainerOpDesc + Seq( + objectMapper.createObjectNode(), + objectMapper.readTree("""{"properties": {"paraList": {"items": {}}}}"""), + objectMapper.readTree( + """{"properties": {"paraList": {"items": {"$ref": "#/definitions/Row"}}}, + "definitions": {"Row": {"properties": {"value": "not an object"}}}}""" + ) + ).foreach { schema => + val node = schema.asInstanceOf[ObjectNode] + noException should be thrownBy svc.customizeJsonSchema(node) + node.findValue("valueRules") shouldBe null + } + } + + it should "write no rules for a descriptor whose type argument is not an enum" in { + // TestParam is a class standing in for one, so there are no constants to read. A real + // operator always binds an enum; this is the path a test stub takes. + val schema = objectMapper + .readTree( + """{"properties": {"paraList": {"items": {"$ref": "#/definitions/Row"}}}, + "definitions": {"Row": {"properties": {"value": {"type": "string"}}}}}""" + ) + .asInstanceOf[ObjectNode] + new TestSklearnMLOp().customizeJsonSchema(schema) + schema.findValue("valueRules") shouldBe null + } + + "HyperParameters" should "require whichever of the two inputs the row actually uses" in { + val schema = + OperatorMetadataGenerator.generateOperatorJsonSchema(classOf[SklearnAdvancedSVCTrainerOpDesc]) + val row = schema + .path("definitions") + .path( + schema + .path("properties") + .path("paraList") + .path("items") + .path("$ref") + .asText() + .stripPrefix("#/definitions/") + ) + val branch = row.path("allOf").path(0) + branch + .path("if") + .path("properties") + .path("parametersSource") + .path("const") + .asBoolean() shouldBe true + branch.path("then").path("required").path(0).asText() shouldBe "attribute" + branch.path("else").path("required").path(0).asText() shouldBe "value" + // neither may be required outright: the hide rules show only one of them at a time + row + .path("required") + .elements() + .asScala + .map(_.asText()) + .toSeq should contain noneOf ("value", "attribute") + } + + it should "refuse two rows setting one parameter, whichever source they read" in { + val d = new TestSklearnMLOp + val paraList = List( + hyperParam("n_neighbors", "int", fromWorkflow = false, value = "5"), + hyperParam("n_neighbors", "int", fromWorkflow = true, attribute = "k_col") + ) + val thrown = the[IllegalArgumentException] thrownBy d.getParameter(paraList) + thrown.getMessage should include("n_neighbors") + } + + it should "accept rows setting different parameters" in { + val d = new TestSklearnMLOp + val paraList = List( + hyperParam("n_neighbors", "int", fromWorkflow = false, value = "5"), + hyperParam("leaf_size", "int", fromWorkflow = false, value = "30") + ) + val paramString = d.getParameter(paraList)(1).encode + paramString.filterNot(_.isWhitespace) should include("n_neighbors=int(") + paramString.filterNot(_.isWhitespace) should include("leaf_size=int(") + } } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnMLOperatorDescriptorSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnMLOperatorDescriptorSpec.scala index eac74a54d6..a4411b5697 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnMLOperatorDescriptorSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/sklearnAdvanced/base/SklearnMLOperatorDescriptorSpec.scala @@ -62,6 +62,23 @@ class SklearnMLOperatorDescriptorSpec extends AnyFlatSpec with Matchers { ) } + it should "reject the ground truth column named as a feature" in { + val op = newOp() + op.groundTruthAttribute = "species" + op.selectedFeatures = List("petal_length", "species") + val thrown = intercept[RuntimeException](op.getOutputSchemas(Map.empty)) + thrown.getMessage should include("species") + thrown.getMessage should include("Ground Truth Attribute Column") + thrown.getMessage should include("Selected Features") + } + + it should "let the features through while none of them is the ground truth" in { + val op = newOp() + op.groundTruthAttribute = "species" + op.selectedFeatures = List("petal_length", "petal_width") + op.getOutputSchemas(Map.empty).keySet shouldBe Set(op.operatorInfo.outputPorts.head.id) + } + "SklearnMLOperatorDescriptor" should "default paraList to empty, groundTruthAttribute to empty, and selectedFeatures to null" in { val op = newOp()
