From 8129f6fa82b16f49430a202221291739e3dd0833 Mon Sep 17 00:00:00 2001 From: David Hall Date: Sat, 13 Jul 2019 11:50:47 -0700 Subject: [PATCH] fix Counters for 2.13 --- .../main/scala/breeze/linalg/Counter.scala | 2 +- .../breeze/linalg/operators/CounterOps.scala | 91 +++++++++++++++++-- .../breeze/util/ScalaVersion.scala | 5 + .../scala_2.13/breeze/util/ScalaVersion.scala | 5 + 4 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 math/src/main/scala_2.11_2.12/breeze/util/ScalaVersion.scala create mode 100644 math/src/main/scala_2.13/breeze/util/ScalaVersion.scala diff --git a/math/src/main/scala/breeze/linalg/Counter.scala b/math/src/main/scala/breeze/linalg/Counter.scala index d93bf14db..3173680ec 100644 --- a/math/src/main/scala/breeze/linalg/Counter.scala +++ b/math/src/main/scala/breeze/linalg/Counter.scala @@ -49,7 +49,7 @@ trait CounterLike[K, V, +M <: scala.collection.mutable.Map[K, V], +This <: Count def contains(k: K) = data.contains(k) override def apply(k: K) = { - data.get(k).getOrElse(default) + data.getOrElse(k, default) } def update(k: K, v: V): Unit = { data(k) = v } diff --git a/math/src/main/scala/breeze/linalg/operators/CounterOps.scala b/math/src/main/scala/breeze/linalg/operators/CounterOps.scala index d03ce412d..3a6ad65d3 100644 --- a/math/src/main/scala/breeze/linalg/operators/CounterOps.scala +++ b/math/src/main/scala/breeze/linalg/operators/CounterOps.scala @@ -5,6 +5,7 @@ import breeze.math.{Field, Ring, Semiring} import breeze.linalg.support.{CanZipMapKeyValues, CanTransformValues, CanZipMapValues, CanCopy} import breeze.generic.UFunc import breeze.linalg._ +import breeze.util.ScalaVersion trait CounterOps { implicit def canCopy[K1, V: Zero: Semiring]: CanCopy[Counter[K1, V]] = new CanCopy[Counter[K1, V]] { @@ -29,7 +30,14 @@ trait CounterOps { new OpAdd.InPlaceImpl2[Counter[K1, V], Counter[K1, V]] { val field = implicitly[Semiring[V]] def apply(a: Counter[K1, V], b: Counter[K1, V]): Unit = { - for ((k, v) <- b.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213 && (b eq a)) { + b.activeIterator.toSet.iterator + } else { + b.activeIterator + } + + for ((k, v) <- it) { a(k) = field.+(a(k), v) } } @@ -39,7 +47,14 @@ trait CounterOps { new scaleAdd.InPlaceImpl3[Counter[K1, V], V, Counter[K1, V]] { val field = implicitly[Semiring[V]] def apply(a: Counter[K1, V], s: V, b: Counter[K1, V]): Unit = { - for ((k, v) <- b.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213 && (b eq a)) { + b.activeIterator.toSet.iterator + } else { + b.activeIterator + } + + for ((k, v) <- it) { a(k) = field.+(a(k), field.*(s, v)) } } @@ -53,7 +68,14 @@ trait CounterOps { new OpAdd.InPlaceImpl2[Counter[K1, V], V] { val field = implicitly[Semiring[V]] def apply(a: Counter[K1, V], b: V): Unit = { - for ((k, v) <- a.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213) { + a.activeIterator.toSet.iterator + } else { + a.activeIterator + } + + for ((k, v) <- it) { a(k) = field.+(v, b) } } @@ -67,7 +89,14 @@ trait CounterOps { new OpSub.InPlaceImpl2[Counter[K1, V], Counter[K1, V]] { val field = implicitly[Ring[V]] def apply(a: Counter[K1, V], b: Counter[K1, V]): Unit = { - for ((k, v) <- b.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213 && (b eq a)) { + b.activeIterator.toSet.iterator + } else { + b.activeIterator + } + + for ((k, v) <- it) { a(k) = field.-(a(k), v) } } @@ -96,7 +125,14 @@ trait CounterOps { new OpMulScalar.InPlaceImpl2[Counter[K1, V], Counter[K2, V]] { val field = implicitly[Semiring[V]] def apply(a: Counter[K1, V], b: Counter[K2, V]): Unit = { - for ((k, v) <- a.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213) { + a.activeIterator.toSet.iterator + } else { + a.activeIterator + } + + for ((k, v) <- it) { a(k) = field.*(v, b(k)) } } @@ -121,7 +157,14 @@ trait CounterOps { new OpMulScalar.InPlaceImpl2[Counter[K1, V], V] { val field = implicitly[Semiring[V]] def apply(a: Counter[K1, V], b: V): Unit = { - for ((k, v) <- a.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213) { + a.activeIterator.toSet.iterator + } else { + a.activeIterator + } + + for ((k, v) <- it) { a(k) = field.*(v, b) } } @@ -131,7 +174,14 @@ trait CounterOps { new OpMulMatrix.InPlaceImpl2[Counter[K1, V], V] { val field = implicitly[Semiring[V]] def apply(a: Counter[K1, V], b: V): Unit = { - for ((k, v) <- a.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213) { + a.activeIterator.toSet.iterator + } else { + a.activeIterator + } + + for ((k, v) <- it) { a(k) = field.*(v, b) } } @@ -169,7 +219,14 @@ trait CounterOps { new OpDiv.InPlaceImpl2[Counter[K1, V], Counter[K1, V]] { val field = implicitly[Field[V]] def apply(a: Counter[K1, V], b: Counter[K1, V]): Unit = { - for ((k, v) <- a.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213) { + a.activeIterator.toSet.iterator + } else { + a.activeIterator + } + + for ((k, v) <- it) { a(k) = field./(v, b(k)) } } @@ -210,7 +267,14 @@ trait CounterOps { new OpDiv.InPlaceImpl2[Counter[K1, V], V] { val field = implicitly[Field[V]] def apply(a: Counter[K1, V], b: V): Unit = { - for ((k, v) <- a.activeIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213) { + a.activeIterator.toSet.iterator + } else { + a.activeIterator + } + + for ((k, v) <- it) { a(k) = field./(v, b) } } @@ -229,7 +293,14 @@ trait CounterOps { implicit def canSetIntoVS[K1, V]: OpSet.InPlaceImpl2[Counter[K1, V], V] = { new OpSet.InPlaceImpl2[Counter[K1, V], V] { def apply(a: Counter[K1, V], b: V): Unit = { - for (k <- a.keysIterator) { + // scala 2.13 hashmaps invalidate iterators if you change values, even if the keys are already there + val it = if (ScalaVersion.is213) { + a.keysIterator.toSet.iterator + } else { + a.keysIterator + } + + for (k <- it) { a(k) = b } } diff --git a/math/src/main/scala_2.11_2.12/breeze/util/ScalaVersion.scala b/math/src/main/scala_2.11_2.12/breeze/util/ScalaVersion.scala new file mode 100644 index 000000000..a23605a17 --- /dev/null +++ b/math/src/main/scala_2.11_2.12/breeze/util/ScalaVersion.scala @@ -0,0 +1,5 @@ +package breeze.util + +private[breeze] object ScalaVersion { + def is213: Boolean = false +} \ No newline at end of file diff --git a/math/src/main/scala_2.13/breeze/util/ScalaVersion.scala b/math/src/main/scala_2.13/breeze/util/ScalaVersion.scala new file mode 100644 index 000000000..a49ae6f58 --- /dev/null +++ b/math/src/main/scala_2.13/breeze/util/ScalaVersion.scala @@ -0,0 +1,5 @@ +package breeze.util + +private[breeze] object ScalaVersion { + def is213: Boolean = true +} \ No newline at end of file