dongjoon-hyun commented on code in PR #58812:
URL: https://github.com/apache/spark/pull/58812#discussion_r4030400098


##########
mllib/src/main/scala/org/apache/spark/ml/feature/Instance.scala:
##########
@@ -41,10 +41,14 @@ private[spark] case class InstanceBlock(
     labels: Array[Double],
     weights: Array[Double],
     matrix: Matrix) {
-  require(labels.length == matrix.numRows)
-  require(matrix.isTransposed)
+  require(labels.length == matrix.numRows,
+    s"The number of labels (${labels.length}) must match the number of matrix 
rows " +
+    s"(${matrix.numRows}).")
+  require(matrix.isTransposed, "The matrix must be transposed (stored in 
row-major order).")

Review Comment:
   nit. `(stored in row-major order)` is accurate for `DenseMatrix`, but for 
`SparseMatrix`, `isTransposed` means CSR format. Since `blokify` can produce 
sparse matrices, what about `(row-major for dense, CSR for sparse)` or simply 
dropping the parenthetical?



##########
mllib/src/main/scala/org/apache/spark/ml/feature/Instance.scala:
##########
@@ -164,7 +168,9 @@ private[spark] object InstanceBlock {
         while (instanceIterator.hasNext && blockMemUsage < maxMemUsage) {
           val instance = instanceIterator.next()
           if (numCols < 0L) numCols = instance.features.size
-          require(numCols == instance.features.size)
+          require(numCols == instance.features.size,
+            "All instances must have the same number of features, but got " +
+            s"${instance.features.size} != $numCols.")

Review Comment:
   nit. `X != Y` doesn't tell which one is the expected value. What about the 
following?
   ```scala
   s"All instances must have the same number of features: expected $numCols " +
   s"but got ${instance.features.size}."
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to