diff --git a/cognitive/src/test/scala/com/microsoft/azure/synapse/ml/core/utils/utils/ModelEqualitySuite.scala b/cognitive/src/test/scala/com/microsoft/azure/synapse/ml/core/utils/utils/ModelEqualitySuite.scala index b2bc5ed750f..4b021ed2707 100644 --- a/cognitive/src/test/scala/com/microsoft/azure/synapse/ml/core/utils/utils/ModelEqualitySuite.scala +++ b/cognitive/src/test/scala/com/microsoft/azure/synapse/ml/core/utils/utils/ModelEqualitySuite.scala @@ -12,6 +12,33 @@ import com.microsoft.azure.synapse.ml.stages.DropColumns class ModelEqualitySuite extends TestBase { spark + test("jaccardSimilarity grades partial overlap") { + // Sets of whole strings would collapse this to a 1.0/0.0 equality check. + assert(ModelEquality.jaccardSimilarity("abcd", "abcd") === 1.0) + assert(ModelEquality.jaccardSimilarity("abcd", "wxyz") === 0.0) + val partial = ModelEquality.jaccardSimilarity("the quick brown fox", "the quick brown cat") + assert(partial > 0.0 && partial < 1.0) + assert(partial > ModelEquality.jaccardSimilarity("the quick brown fox", "entirely unlike")) + } + + test("jaccardSimilarity is case insensitive and symmetric") { + assert(ModelEquality.jaccardSimilarity("Hello World", "hello world") === 1.0) + assert(ModelEquality.jaccardSimilarity("kitten", "sitting") + === ModelEquality.jaccardSimilarity("sitting", "kitten")) + assert(ModelEquality.jaccardSimilarity("", "") === 1.0) + } + + test("jaccardSimilarity handles strings shorter than one bigram") { + // sliding(2) keeps the short final window, so a 1-char string yields Set(char) + // rather than the empty set. Distinct 1-char strings must therefore score 0.0, + // not fall into the both-empty shortcut. + assert(ModelEquality.jaccardSimilarity("a", "b") === 0.0) + assert(ModelEquality.jaccardSimilarity("a", "a") === 1.0) + assert(ModelEquality.jaccardSimilarity("", "a") === 0.0) + assert(ModelEquality.jaccardSimilarity("a", "") === 0.0) + assert(ModelEquality.jaccardSimilarity("", "") === 1.0) + } + test("Complex param equality") { val m1 = new TextSentiment().setLocation("eastus") val m2 = new TextSentiment().setLocation("eastus") diff --git a/core/src/main/scala/com/microsoft/azure/synapse/ml/automl/HyperparamBuilder.scala b/core/src/main/scala/com/microsoft/azure/synapse/ml/automl/HyperparamBuilder.scala index 475466bf658..31e9f20b13d 100644 --- a/core/src/main/scala/com/microsoft/azure/synapse/ml/automl/HyperparamBuilder.scala +++ b/core/src/main/scala/com/microsoft/azure/synapse/ml/automl/HyperparamBuilder.scala @@ -5,6 +5,7 @@ package com.microsoft.azure.synapse.ml.automl import org.apache.spark.ml.param._ +import scala.annotation.tailrec import scala.collection.JavaConverters._ import scala.collection.mutable import scala.util.Random @@ -29,8 +30,40 @@ class LongRangeHyperParam(min: Long, max: Long, seed: Long = 0) extends RangeHyperParam[Long](min, max, seed) { def getNext(): Long = { + require(max >= min, s"LongRangeHyperParam requires max >= min, got min=$min, max=$max") val range = max - min - (random.nextLong() * range) + min + // nextLong() spans the full 64-bit range, so multiplying it by the range overflows and + // escapes [min, max) entirely. Reduce into the range first, as nextInt(range) does. + if (range == 0) { + min + } else if (range < 0) { + // The span is wider than Long.MaxValue so it cannot be represented as a Long. Draw from + // the whole 64-bit range instead; more than half of it lands in bounds, so this is cheap. + @tailrec def drawInBounds(): Long = { + val v = random.nextLong() + if (v >= min && v < max) v else drawInBounds() + } + drawInBounds() + } else { + boundedNextLong(range) + min + } + } + + /** Uniform draw from [0, bound), rejecting the partial final block so the result carries no + * modulo bias. Mirrors what java.util.Random.nextInt(bound) already does for Int. + */ + private def boundedNextLong(bound: Long): Long = { + val m = bound - 1 + if ((bound & m) == 0L) { + random.nextLong() & m // bound is a power of two, so the low bits are already uniform + } else { + @tailrec def draw(): Long = { + val u = random.nextLong() >>> 1 + val r = u % bound + if (u + m - r < 0L) draw() else r + } + draw() + } } } @@ -38,7 +71,7 @@ class LongRangeHyperParam(min: Long, max: Long, seed: Long = 0) class FloatRangeHyperParam(min: Float, max: Float, seed: Long = 0) extends RangeHyperParam[Float](min, max, seed) { - val doubleRange = new DoubleRangeHyperParam(min.toDouble, max.toDouble) + val doubleRange = new DoubleRangeHyperParam(min.toDouble, max.toDouble, seed) def getNext(): Float = { doubleRange.getNext().toFloat } diff --git a/core/src/main/scala/com/microsoft/azure/synapse/ml/core/utils/ModelEquality.scala b/core/src/main/scala/com/microsoft/azure/synapse/ml/core/utils/ModelEquality.scala index 797e82d122d..66ad00917ec 100644 --- a/core/src/main/scala/com/microsoft/azure/synapse/ml/core/utils/ModelEquality.scala +++ b/core/src/main/scala/com/microsoft/azure/synapse/ml/core/utils/ModelEquality.scala @@ -14,10 +14,13 @@ trait ParamEquality[T] extends Param[T] { object ModelEquality { + /** Similarity of two strings as the Jaccard index over their character bigrams. */ def jaccardSimilarity(s1: String, s2: String): Double = { - val a = Set(s1) - val b = Set(s2) - a.intersect(b).size.toDouble / (a | b).size.toDouble + // Sets of the whole strings would only ever yield 1.0 or 0.0, making this a strict + // equality check rather than a similarity measure. + val a = s1.toLowerCase.sliding(2).toSet + val b = s2.toLowerCase.sliding(2).toSet + if (a.isEmpty && b.isEmpty) 1.0 else a.intersect(b).size.toDouble / (a | b).size.toDouble } def assertEqual(m1: Params, m2: Params): Unit = { diff --git a/core/src/main/scala/com/microsoft/azure/synapse/ml/stages/UnicodeNormalize.scala b/core/src/main/scala/com/microsoft/azure/synapse/ml/stages/UnicodeNormalize.scala index 5ef14b37642..6ef6df50c4d 100644 --- a/core/src/main/scala/com/microsoft/azure/synapse/ml/stages/UnicodeNormalize.scala +++ b/core/src/main/scala/com/microsoft/azure/synapse/ml/stages/UnicodeNormalize.scala @@ -32,8 +32,9 @@ class UnicodeNormalize(val uid: String) extends Transformer /** @group setParam */ def setForm(value: String): this.type = { - // check input value - Normalizer.Form.valueOf(getForm) + // Validate the incoming value, not the value already set. Validating getForm let an invalid + // form be stored and only surface later, inside the UDF on the executors. + Normalizer.Form.valueOf(value) set("form", value) } diff --git a/core/src/test/scala/com/microsoft/azure/synapse/ml/automl/HyperparamRangeSuite.scala b/core/src/test/scala/com/microsoft/azure/synapse/ml/automl/HyperparamRangeSuite.scala new file mode 100644 index 00000000000..3e150dfaf74 --- /dev/null +++ b/core/src/test/scala/com/microsoft/azure/synapse/ml/automl/HyperparamRangeSuite.scala @@ -0,0 +1,99 @@ +// Copyright (C) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. See LICENSE in project root for information. + +package com.microsoft.azure.synapse.ml.automl + +import com.microsoft.azure.synapse.ml.core.test.base.TestBase + +// scalastyle:off magic.number +/** Covers the bounds and seeding contract shared by every RangeHyperParam. */ +class HyperparamRangeSuite extends TestBase { + + private val draws = 500 + + test("LongRangeHyperParam stays within its range") { + val hp = new LongRangeHyperParam(0L, 100L, seed = 42) + val values = (1 to draws).map(_ => hp.getNext()) + assert(values.forall(v => v >= 0L && v < 100L), s"out of range: ${values.filter(v => v < 0L || v >= 100L)}") + assert(values.toSet.size > 1) + } + + test("LongRangeHyperParam stays within a range too large to fit in an Int") { + val min = -4000000000L + val max = 4000000000L + val hp = new LongRangeHyperParam(min, max, seed = 7) + val values = (1 to draws).map(_ => hp.getNext()) + assert(values.forall(v => v >= min && v < max)) + assert(values.exists(_ < 0L) && values.exists(_ > 0L)) + } + + test("LongRangeHyperParam rejects an inverted range instead of silently returning min") { + val hp = new LongRangeHyperParam(10L, 5L, seed = 1) + assertThrows[IllegalArgumentException](hp.getNext()) + } + + test("LongRangeHyperParam stays in bounds when the span overflows Long") { + // max - min wraps negative here, so the ordinary reduction path cannot be used. + val hp = new LongRangeHyperParam(Long.MinValue + 1, Long.MaxValue, seed = 7) + val draws = (1 to 200).map(_ => hp.getNext()) + assert(draws.forall(v => v >= Long.MinValue + 1 && v < Long.MaxValue)) + assert(draws.distinct.length > 1) + } + + test("LongRangeHyperParam with an empty range returns min") { + val hp = new LongRangeHyperParam(5L, 5L, seed = 42) + assert((1 to 10).forall(_ => hp.getNext() === 5L)) + } + + test("LongRangeHyperParam covers a power-of-two range without bias") { + // Power-of-two bounds take the mask branch; every bucket must still be reachable. + val hp = new LongRangeHyperParam(0L, 8L, seed = 3) + val counts = (1 to 4000).map(_ => hp.getNext()).groupBy(identity).map { case (k, v) => k -> v.size } + assert(counts.keySet === (0L until 8L).toSet) + assert(counts.values.forall(c => c > 300 && c < 700), s"uneven distribution: $counts") + } + + test("LongRangeHyperParam covers a non-power-of-two range without bias") { + // Non-power-of-two bounds take the rejection branch, which must terminate and stay uniform. + val hp = new LongRangeHyperParam(0L, 7L, seed = 3) + val counts = (1 to 4000).map(_ => hp.getNext()).groupBy(identity).map { case (k, v) => k -> v.size } + assert(counts.keySet === (0L until 7L).toSet) + assert(counts.values.forall(c => c > 350 && c < 800), s"uneven distribution: $counts") + } + + test("IntRangeHyperParam stays within its range") { + val hp = new IntRangeHyperParam(5, 15, seed = 42) + val values = (1 to draws).map(_ => hp.getNext()) + assert(values.forall(v => v >= 5 && v < 15)) + } + + test("DoubleRangeHyperParam stays within its range") { + val hp = new DoubleRangeHyperParam(0.0, 1.0, seed = 42) + assert((1 to draws).map(_ => hp.getNext()).forall(v => v >= 0.0 && v < 1.0)) + } + + test("FloatRangeHyperParam stays within its range") { + val hp = new FloatRangeHyperParam(0.0f, 1.0f, seed = 42) + assert((1 to draws).map(_ => hp.getNext()).forall(v => v >= 0.0f && v < 1.0f)) + } + + test("equal seeds reproduce equal sequences and differing seeds diverge") { + def draw(hp: Dist[_]): Seq[Any] = (1 to 20).map(_ => hp.getNext) + + assert(draw(new IntRangeHyperParam(0, 1000, 11)) === draw(new IntRangeHyperParam(0, 1000, 11))) + assert(draw(new LongRangeHyperParam(0L, 1000L, 11)) === draw(new LongRangeHyperParam(0L, 1000L, 11))) + assert(draw(new DoubleRangeHyperParam(0.0, 1.0, 11)) === draw(new DoubleRangeHyperParam(0.0, 1.0, 11))) + // FloatRangeHyperParam delegates to an inner DoubleRangeHyperParam; it must forward its seed. + assert(draw(new FloatRangeHyperParam(0.0f, 1.0f, 11)) === draw(new FloatRangeHyperParam(0.0f, 1.0f, 11))) + assert(draw(new FloatRangeHyperParam(0.0f, 1.0f, 11)) !== draw(new FloatRangeHyperParam(0.0f, 1.0f, 12))) + } + + test("HyperParamUtils.getRangeHyperParam honors the seed for every numeric type") { + Seq[(Any, Any)]((0, 1000), (0L, 1000L), (0.0, 1.0), (0.0f, 1.0f)).foreach { case (min, max) => + val a = HyperParamUtils.getRangeHyperParam(min, max, 99) + val b = HyperParamUtils.getRangeHyperParam(min, max, 99) + assert((1 to 20).map(_ => a.getNext) === (1 to 20).map(_ => b.getNext), s"seed ignored for $min/$max") + } + } +} +// scalastyle:on magic.number diff --git a/core/src/test/scala/com/microsoft/azure/synapse/ml/stages/UnicodeNormalizeSuite.scala b/core/src/test/scala/com/microsoft/azure/synapse/ml/stages/UnicodeNormalizeSuite.scala index e7d098ca6fb..664140c5c6b 100644 --- a/core/src/test/scala/com/microsoft/azure/synapse/ml/stages/UnicodeNormalizeSuite.scala +++ b/core/src/test/scala/com/microsoft/azure/synapse/ml/stages/UnicodeNormalizeSuite.scala @@ -5,6 +5,7 @@ package com.microsoft.azure.synapse.ml.stages import com.microsoft.azure.synapse.ml.core.test.base.TestBase import com.microsoft.azure.synapse.ml.core.test.fuzzing.{TestObject, TransformerFuzzing} +import org.apache.spark.ml.param.Param import org.apache.spark.ml.util.MLReadable import org.apache.spark.sql.DataFrame @@ -49,9 +50,25 @@ class UnicodeNormalizeSuite extends TestBase with TransformerFuzzing[UnicodeNorm test("Check for NFKD forms") { testForm("NFKD", expectedResultDecomposed) } + test("setForm rejects an invalid form at set time") { + // An unvalidated form used to be accepted here and only fail later inside the UDF, + // on the executors, where the cause is far harder to attribute. + assertThrows[IllegalArgumentException](new UnicodeNormalize().setForm("NOT_A_FORM")) + } + + test("setForm accepts every supported form") { + java.text.Normalizer.Form.values().foreach { f => + assert(new UnicodeNormalize().setForm(f.name).getForm === f.name) + } + } + def testObjects(): Seq[TestObject[UnicodeNormalize]] = List(new TestObject( new UnicodeNormalize().setInputCol("words").setOutputCol("out"), makeBasicDF())) + // The generic fuzzer probes String setters with "foo"; form only accepts Normalizer.Form names. + override def getterSetterParamExamples(pipelineStage: UnicodeNormalize): Map[Param[_], Any] = + Map[Param[_], Any]((pipelineStage.form, "NFC")) + override def reader: MLReadable[_] = UnicodeNormalize }