dongjoon-hyun opened a new pull request, #59560: URL: https://github.com/apache/spark/pull/59560
### What changes were proposed in this pull request? This PR aims to make `DenseMatrix.copy` and `SparseMatrix.copy` preserve `isTransposed` in both `org.apache.spark.ml.linalg` and `org.apache.spark.mllib.linalg`. `SparseMatrix.copy` still shares `colPtrs` and `rowIndices` with the original matrix and clones only `values`, as before. ### Why are the changes needed? When `isTransposed` is true, `DenseMatrix.values` is stored in row-major order, and `SparseMatrix` stores its entries in CSR format. However, `copy` drops `isTransposed`, so the copy reads the same arrays in column-major order (CSC format for `SparseMatrix`). As a result, the copy of a transposed matrix is generally not equal to the original matrix. - For a square matrix, the copy is the transpose of the original matrix. - For a non-square `DenseMatrix`, the copy has the same shape, but its entries are rearranged. - For a non-square `SparseMatrix`, `copy` fails with `IllegalArgumentException` from the `colPtrs` length check. This is a correctness issue because, in the first two cases, `copy` silently returns a different matrix without any error. For example, the `coefficientMatrix` of a multinomial `LogisticRegressionModel` can be a transposed `DenseMatrix`, so its `copy` returns a matrix with rearranged coefficients. This has been the case since Apache Spark [v1.3.0](https://github.com/apache/spark/releases/tag/v1.3.0) (2015-03-13) for `mllib` (SPARK-5321) and [v2.0.0](https://github.com/apache/spark/releases/tag/v2.0.0) (2016-07-26) for `ml` (SPARK-14549). SPARK-7668 fixed the same issue in `map`, but not in `copy`. ```scala import org.apache.spark.ml.linalg.{DenseMatrix, SparseMatrix} val dm = new DenseMatrix(2, 3, Array(1.0, 2.0, 3.0, 4.0, 5.0, 6.0)).transpose dm.copy == dm // Before: false // After: true val sm = new SparseMatrix(2, 3, Array(0, 1, 2, 3), Array(0, 1, 0), Array(1.0, 2.0, 3.0)).transpose sm.copy == sm // Before: java.lang.IllegalArgumentException: requirement failed: Expecting 3 colPtrs when numCols = 2 but got 4 // After: true ``` ### Does this PR introduce _any_ user-facing change? Yes. `copy` of a transposed `DenseMatrix` or `SparseMatrix` returns a matrix that is equal to the original matrix and has the same `isTransposed`. Previously, it returned a different matrix or failed with `IllegalArgumentException`. There is no change for non-transposed matrices. ### How was this patch tested? Pass the CIs with the newly added test cases in `ml.linalg.MatricesSuite` and `mllib.linalg.MatricesSuite`. They cover transposed square and non-square `DenseMatrix` and `SparseMatrix`. Without this fix, both new test cases fail because the copy of a transposed 2 x 2 `DenseMatrix` is not equal to the original matrix. ``` $ build/sbt 'mllib-local/testOnly *MatricesSuite' $ build/sbt 'mllib/testOnly org.apache.spark.mllib.linalg.MatricesSuite' ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 5.5 -- 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]
