Skip to content

Conversation

@lee19
Copy link
Contributor

@lee19 lee19 commented Jun 23, 2015

I'm sorry that I made #6949 closed by mistake.
I pushed codes again.

And, I added a test code.

There is a bug that U.numCols() = self.nCols in IndexedRowMatrix.computeSVD()
It should have been U.numCols() = k = svd.U.numCols()

self = U * sigma * V.transpose
(m x n) = (m x n) * (k x k) * (k x n) //ASIS
-->
(m x n) = (m x k) * (k x k) * (k x n) //TOBE

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will need to write: svd.U.numCols().toInt

@jkbradley
Copy link
Member

@lee19 Thanks for the PR! Those 2 issues should be quick to fix. But please run the tests locally before updating to make sure the update works.

@srowen
Copy link
Member

srowen commented Jun 26, 2015

OK to test

@srowen
Copy link
Member

srowen commented Jun 26, 2015

LGTM

@SparkQA
Copy link

SparkQA commented Jun 26, 2015

Test build #963 has finished for PR 6953 at commit 4b9803b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@lee19
Copy link
Contributor Author

lee19 commented Jun 26, 2015

I'm very sorry for build errors. 😳 I should have spent time on it at least.
And, I'm sorry again for the failure of unit test this time.
I can't run unit tests locally, yet, because
spark/mlib$ ../build/mvn test is being stuck at LogisticRegressionClusterSuite because of OutOfMemoryError.
Let me tell you after I finish it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid an extra job, we can keep nRows.

@srowen
Copy link
Member

srowen commented Jun 30, 2015

@lee19 can you update this per the comment about nRows? I am not sure why your test failed since it looks correct, as does your change. Yes maybe you can run just that test locally to figure out what's going on.

@lee19
Copy link
Contributor Author

lee19 commented Jun 30, 2015

I just confirmed spark$ build/mvn -pl mllib -Pyarn -Phadoop.version=2.4 -DwildcardSuites=org.apache.spark.mllib.linalg.distributed.IndexedRowMatrixSuite test passed locally.
I don't know, neither, why the test build 963 of SparkQA failed few days ago.
@mengxr Thanks for the suggestion!

@SparkQA
Copy link

SparkQA commented Jun 30, 2015

Test build #985 has finished for PR 6953 at commit c1812a0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

asfgit pushed a commit that referenced this pull request Jun 30, 2015
…).U.numCols = k

I'm sorry that I made #6949 closed by mistake.
I pushed codes again.

And, I added a test code.

>
There is a bug that `U.numCols() = self.nCols` in `IndexedRowMatrix.computeSVD()`
It should have been `U.numCols() = k = svd.U.numCols()`

>
```
self = U * sigma * V.transpose
(m x n) = (m x n) * (k x k) * (k x n) //ASIS
-->
(m x n) = (m x k) * (k x k) * (k x n) //TOBE
```

Author: lee19 <[email protected]>

Closes #6953 from lee19/MLlibBugfix and squashes the following commits:

c1812a0 [lee19] [SPARK-8563] [MLlib] Used nRows instead of numRows() to reduce a burden.
4b9803b [lee19] [SPARK-8563] [MLlib] Fixed a build error.
c2ccd89 [lee19] Added a unit test that validates matrix sizes of svd for [SPARK-8563][MLlib]
8373424 [lee19] [SPARK-8563][MLlib] Fixed a bug so that IndexedRowMatrix.computeSVD().U.numCols = k

(cherry picked from commit e725262)
Signed-off-by: Xiangrui Meng <[email protected]>
asfgit pushed a commit that referenced this pull request Jun 30, 2015
…).U.numCols = k

I'm sorry that I made #6949 closed by mistake.
I pushed codes again.

And, I added a test code.

>
There is a bug that `U.numCols() = self.nCols` in `IndexedRowMatrix.computeSVD()`
It should have been `U.numCols() = k = svd.U.numCols()`

>
```
self = U * sigma * V.transpose
(m x n) = (m x n) * (k x k) * (k x n) //ASIS
-->
(m x n) = (m x k) * (k x k) * (k x n) //TOBE
```

Author: lee19 <[email protected]>

Closes #6953 from lee19/MLlibBugfix and squashes the following commits:

c1812a0 [lee19] [SPARK-8563] [MLlib] Used nRows instead of numRows() to reduce a burden.
4b9803b [lee19] [SPARK-8563] [MLlib] Fixed a build error.
c2ccd89 [lee19] Added a unit test that validates matrix sizes of svd for [SPARK-8563][MLlib]
8373424 [lee19] [SPARK-8563][MLlib] Fixed a bug so that IndexedRowMatrix.computeSVD().U.numCols = k

(cherry picked from commit e725262)
Signed-off-by: Xiangrui Meng <[email protected]>
@asfgit asfgit closed this in e725262 Jun 30, 2015
asfgit pushed a commit that referenced this pull request Jun 30, 2015
…).U.numCols = k

I'm sorry that I made #6949 closed by mistake.
I pushed codes again.

And, I added a test code.

>
There is a bug that `U.numCols() = self.nCols` in `IndexedRowMatrix.computeSVD()`
It should have been `U.numCols() = k = svd.U.numCols()`

>
```
self = U * sigma * V.transpose
(m x n) = (m x n) * (k x k) * (k x n) //ASIS
-->
(m x n) = (m x k) * (k x k) * (k x n) //TOBE
```

Author: lee19 <[email protected]>

Closes #6953 from lee19/MLlibBugfix and squashes the following commits:

c1812a0 [lee19] [SPARK-8563] [MLlib] Used nRows instead of numRows() to reduce a burden.
4b9803b [lee19] [SPARK-8563] [MLlib] Fixed a build error.
c2ccd89 [lee19] Added a unit test that validates matrix sizes of svd for [SPARK-8563][MLlib]
8373424 [lee19] [SPARK-8563][MLlib] Fixed a bug so that IndexedRowMatrix.computeSVD().U.numCols = k

(cherry picked from commit e725262)
Signed-off-by: Xiangrui Meng <[email protected]>
asfgit pushed a commit that referenced this pull request Jun 30, 2015
…).U.numCols = k

I'm sorry that I made #6949 closed by mistake.
I pushed codes again.

And, I added a test code.

>
There is a bug that `U.numCols() = self.nCols` in `IndexedRowMatrix.computeSVD()`
It should have been `U.numCols() = k = svd.U.numCols()`

>
```
self = U * sigma * V.transpose
(m x n) = (m x n) * (k x k) * (k x n) //ASIS
-->
(m x n) = (m x k) * (k x k) * (k x n) //TOBE
```

Author: lee19 <[email protected]>

Closes #6953 from lee19/MLlibBugfix and squashes the following commits:

c1812a0 [lee19] [SPARK-8563] [MLlib] Used nRows instead of numRows() to reduce a burden.
4b9803b [lee19] [SPARK-8563] [MLlib] Fixed a build error.
c2ccd89 [lee19] Added a unit test that validates matrix sizes of svd for [SPARK-8563][MLlib]
8373424 [lee19] [SPARK-8563][MLlib] Fixed a bug so that IndexedRowMatrix.computeSVD().U.numCols = k

(cherry picked from commit e725262)
Signed-off-by: Xiangrui Meng <[email protected]>
asfgit pushed a commit that referenced this pull request Jun 30, 2015
…).U.numCols = k

I'm sorry that I made #6949 closed by mistake.
I pushed codes again.

And, I added a test code.

>
There is a bug that `U.numCols() = self.nCols` in `IndexedRowMatrix.computeSVD()`
It should have been `U.numCols() = k = svd.U.numCols()`

>
```
self = U * sigma * V.transpose
(m x n) = (m x n) * (k x k) * (k x n) //ASIS
-->
(m x n) = (m x k) * (k x k) * (k x n) //TOBE
```

Author: lee19 <[email protected]>

Closes #6953 from lee19/MLlibBugfix and squashes the following commits:

c1812a0 [lee19] [SPARK-8563] [MLlib] Used nRows instead of numRows() to reduce a burden.
4b9803b [lee19] [SPARK-8563] [MLlib] Fixed a build error.
c2ccd89 [lee19] Added a unit test that validates matrix sizes of svd for [SPARK-8563][MLlib]
8373424 [lee19] [SPARK-8563][MLlib] Fixed a bug so that IndexedRowMatrix.computeSVD().U.numCols = k

(cherry picked from commit e725262)
Signed-off-by: Xiangrui Meng <[email protected]>
@mengxr
Copy link
Contributor

mengxr commented Jun 30, 2015

LGTM. Merged into master and all branches since 1.0. Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants