add check for ascending order

apache · Jun 1, 2015 · 6e4f8ca · 6e4f8ca
1 parent 9956365
commit 6e4f8ca
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/mllib/src/main/scala/org/apache/spark/mllib/util/MLUtils.scala b/mllib/src/main/scala/org/apache/spark/mllib/util/MLUtils.scala
@@ -82,8 +82,19 @@ object MLUtils {
           val value = indexAndValue(1).toDouble
           (index, value)
         }.unzip
-        require(indices.size == 0 || indices(0) >= 0,
-          "indices should be one-based in LIBSVM format")
+
+        // check if indices is one-based and in ascending order
+        var previous = -1
+        var i = 0
+        val indicesLength = indices.size
+        while (i < indicesLength) {
+          if (indices(i) <= previous) {
+            throw new IllegalArgumentException("indices should be one-based and in ascending order")
+          }
+          previous = indices(i)
+          i += 1
+        }
+
         (label, indices.toArray, values.toArray)
       }
 

diff --git a/mllib/src/test/scala/org/apache/spark/mllib/util/MLUtilsSuite.scala b/mllib/src/test/scala/org/apache/spark/mllib/util/MLUtilsSuite.scala
@@ -110,7 +110,7 @@ class MLUtilsSuite extends FunSuite with MLlibTestSparkContext {
     Utils.deleteRecursively(tempDir)
   }
 
-  test("loadLibSVMFile throws SparkException when passing a zero-based vector") {
+  test("loadLibSVMFile throws IllegalArgumentException when indices is zero-based") {
     val lines =
       """
         |0
@@ -122,7 +122,24 @@ class MLUtilsSuite extends FunSuite with MLlibTestSparkContext {
     val path = tempDir.toURI.toString
 
     intercept[SparkException] {
-      val pointsWithoutNumFeatures = loadLibSVMFile(sc, path).collect()
+      loadLibSVMFile(sc, path).collect()
+    }
+    Utils.deleteRecursively(tempDir)
+  }
+
+  test("loadLibSVMFile throws IllegalArgumentException when indices is not in ascending order") {
+    val lines =
+      """
+        |0
+        |0 3:4.0 2:5.0 6:6.0
+      """.stripMargin
+    val tempDir = Utils.createTempDir()
+    val file = new File(tempDir.getPath, "part-00000")
+    Files.write(lines, file, Charsets.US_ASCII)
+    val path = tempDir.toURI.toString
+
+    intercept[SparkException] {
+      loadLibSVMFile(sc, path).collect()
     }
     Utils.deleteRecursively(tempDir)
   }