Skip to content

Commit 28d4238

Browse files
coderxiangjkbradley
authored andcommitted
[SPARK-7452] [MLLIB] fix bug in topBykey and update test
the toArray function of the BoundedPriorityQueue does not necessarily preserve order. Add a counter-example as the test, which would fail the original impl. Author: Shuo Xiang <[email protected]> Closes #5990 from coderxiang/topbykey-test and squashes the following commits: 98804c9 [Shuo Xiang] fix bug in topBykey and update test (cherry picked from commit 92f8f80) Signed-off-by: Joseph K. Bradley <[email protected]>
1 parent 05454fd commit 28d4238

File tree

2 files changed

+6
-5
lines changed

2 files changed

+6
-5
lines changed

mllib/src/main/scala/org/apache/spark/mllib/rdd/MLPairRDDFunctions.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class MLPairRDDFunctions[K: ClassTag, V: ClassTag](self: RDD[(K, V)]) extends Se
4646
combOp = (queue1, queue2) => {
4747
queue1 ++= queue2
4848
}
49-
).mapValues(_.toArray.reverse) // This is an min-heap, so we reverse the order.
49+
).mapValues(_.toArray.sorted(ord.reverse)) // This is an min-heap, so we reverse the order.
5050
}
5151
}
5252

mllib/src/test/scala/org/apache/spark/mllib/rdd/MLPairRDDFunctionsSuite.scala

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,14 @@ import org.apache.spark.mllib.rdd.MLPairRDDFunctions._
2424

2525
class MLPairRDDFunctionsSuite extends FunSuite with MLlibTestSparkContext {
2626
test("topByKey") {
27-
val topMap = sc.parallelize(Array((1, 1), (1, 2), (3, 2), (3, 7), (5, 1), (3, 5)), 2)
28-
.topByKey(2)
27+
val topMap = sc.parallelize(Array((1, 7), (1, 3), (1, 6), (1, 1), (1, 2), (3, 2), (3, 7), (5,
28+
1), (3, 5)), 2)
29+
.topByKey(5)
2930
.collectAsMap()
3031

3132
assert(topMap.size === 3)
32-
assert(topMap(1) === Array(2, 1))
33-
assert(topMap(3) === Array(7, 5))
33+
assert(topMap(1) === Array(7, 6, 3, 2, 1))
34+
assert(topMap(3) === Array(7, 5, 2))
3435
assert(topMap(5) === Array(1))
3536
}
3637
}

0 commit comments

Comments
 (0)