You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by wangmiao1981 <gi...@git.apache.org> on 2017/01/06 20:44:59 UTC
[GitHub] spark pull request #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel retu...
GitHub user wangmiao1981 opened a pull request:
https://github.com/apache/spark/pull/16491
[SPARK-19110][ML][MLLIB]:DistributedLDAModel returns different logPrior for original and loaded model
## What changes were proposed in this pull request?
While adding DistributedLDAModel training summary for SparkR, I found that the logPrior for original and loaded model is different.
For example, in the test("read/write DistributedLDAModel"), I add the test:
val logPrior = model.asInstanceOf[DistributedLDAModel].logPrior
val logPrior2 = model2.asInstanceOf[DistributedLDAModel].logPrior
assert(logPrior === logPrior2)
The test fails:
-4.394180878889078 did not equal -4.294290536919573
The reason is that `graph.vertices.aggregate(0.0)(seqOp, _ + _)` only returns the value of a single vertex instead of the aggregation of all vertices. Therefore, when the loaded model does the aggregation in a different order, it returns different `logPrior`.
Please refer to #16464 for details.
## How was this patch tested?
Add a new unit test for testing logPrior.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/wangmiao1981/spark ldabug
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16491.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #16491
----
commit 31d9bce7c1de016fa23f1783e41aa8b394a68cc4
Author: wm624@hotmail.com <wm...@hotmail.com>
Date: 2017-01-06T20:40:54Z
fix the bug of logPrior
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel retu...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/16491#discussion_r95034707
--- Diff: mllib/src/test/scala/org/apache/spark/ml/clustering/LDASuite.scala ---
@@ -260,6 +260,14 @@ class LDASuite extends SparkFunSuite with MLlibTestSparkContext with DefaultRead
Vectors.dense(model2.topicsMatrix.toArray) absTol 1e-6)
assert(Vectors.dense(model.getDocConcentration) ~==
Vectors.dense(model2.getDocConcentration) absTol 1e-6)
+ val logPrior = model.asInstanceOf[DistributedLDAModel].logPrior
+ val logPrior2 = model2.asInstanceOf[DistributedLDAModel].logPrior
+ val trainingLogLikelihood =
+ model.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ val trainingLogLikelihood2 =
+ model2.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ assert(logPrior ~== logPrior2 absTol 1e-6)
+ assert(trainingLogLikelihood ~== trainingLogLikelihood2 absTol 1e-6)
--- End diff --
should we check trainingLogLikelihood and logPrior are not changing for LocalLDAModel?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel retu...
Posted by wangmiao1981 <gi...@git.apache.org>.
Github user wangmiao1981 commented on a diff in the pull request:
https://github.com/apache/spark/pull/16491#discussion_r95045854
--- Diff: mllib/src/test/scala/org/apache/spark/ml/clustering/LDASuite.scala ---
@@ -260,6 +260,14 @@ class LDASuite extends SparkFunSuite with MLlibTestSparkContext with DefaultRead
Vectors.dense(model2.topicsMatrix.toArray) absTol 1e-6)
assert(Vectors.dense(model.getDocConcentration) ~==
Vectors.dense(model2.getDocConcentration) absTol 1e-6)
+ val logPrior = model.asInstanceOf[DistributedLDAModel].logPrior
+ val logPrior2 = model2.asInstanceOf[DistributedLDAModel].logPrior
+ val trainingLogLikelihood =
+ model.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ val trainingLogLikelihood2 =
+ model2.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ assert(logPrior ~== logPrior2 absTol 1e-6)
+ assert(trainingLogLikelihood ~== trainingLogLikelihood2 absTol 1e-6)
--- End diff --
`LocalLDAModel` doesn't extend `DistributedLDAModel` and vice versa. I am not clear how to check `trainingLogLikelihood ` and `logPrior` in `LocalLDAModel`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel retu...
Posted by wangmiao1981 <gi...@git.apache.org>.
Github user wangmiao1981 commented on a diff in the pull request:
https://github.com/apache/spark/pull/16491#discussion_r95036829
--- Diff: mllib/src/test/scala/org/apache/spark/ml/clustering/LDASuite.scala ---
@@ -260,6 +260,14 @@ class LDASuite extends SparkFunSuite with MLlibTestSparkContext with DefaultRead
Vectors.dense(model2.topicsMatrix.toArray) absTol 1e-6)
assert(Vectors.dense(model.getDocConcentration) ~==
Vectors.dense(model2.getDocConcentration) absTol 1e-6)
+ val logPrior = model.asInstanceOf[DistributedLDAModel].logPrior
+ val logPrior2 = model2.asInstanceOf[DistributedLDAModel].logPrior
+ val trainingLogLikelihood =
+ model.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ val trainingLogLikelihood2 =
+ model2.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ assert(logPrior ~== logPrior2 absTol 1e-6)
+ assert(trainingLogLikelihood ~== trainingLogLikelihood2 absTol 1e-6)
--- End diff --
`logLikelihood` and `logPrior` are only for distributed model.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16491
**[Test build #70992 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70992/testReport)** for PR 16491 at commit [`a29d078`](https://github.com/apache/spark/commit/a29d07861b1c7ed6355474b8afb9d7ffc590158f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel retu...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/16491#discussion_r95047025
--- Diff: mllib/src/test/scala/org/apache/spark/ml/clustering/LDASuite.scala ---
@@ -260,6 +260,14 @@ class LDASuite extends SparkFunSuite with MLlibTestSparkContext with DefaultRead
Vectors.dense(model2.topicsMatrix.toArray) absTol 1e-6)
assert(Vectors.dense(model.getDocConcentration) ~==
Vectors.dense(model2.getDocConcentration) absTol 1e-6)
+ val logPrior = model.asInstanceOf[DistributedLDAModel].logPrior
+ val logPrior2 = model2.asInstanceOf[DistributedLDAModel].logPrior
+ val trainingLogLikelihood =
+ model.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ val trainingLogLikelihood2 =
+ model2.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ assert(logPrior ~== logPrior2 absTol 1e-6)
+ assert(trainingLogLikelihood ~== trainingLogLikelihood2 absTol 1e-6)
--- End diff --
Ok, I guess I remember this wrong because of the other PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel retu...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/16491#discussion_r95038873
--- Diff: mllib/src/test/scala/org/apache/spark/ml/clustering/LDASuite.scala ---
@@ -260,6 +260,14 @@ class LDASuite extends SparkFunSuite with MLlibTestSparkContext with DefaultRead
Vectors.dense(model2.topicsMatrix.toArray) absTol 1e-6)
assert(Vectors.dense(model.getDocConcentration) ~==
Vectors.dense(model2.getDocConcentration) absTol 1e-6)
+ val logPrior = model.asInstanceOf[DistributedLDAModel].logPrior
+ val logPrior2 = model2.asInstanceOf[DistributedLDAModel].logPrior
+ val trainingLogLikelihood =
+ model.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ val trainingLogLikelihood2 =
+ model2.asInstanceOf[DistributedLDAModel].trainingLogLikelihood
+ assert(logPrior ~== logPrior2 absTol 1e-6)
+ assert(trainingLogLikelihood ~== trainingLogLikelihood2 absTol 1e-6)
--- End diff --
right - I mean that they are not persisted & loaded into an unexpected but valid value (!= Double.NaN)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16491
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70991/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16491
**[Test build #70992 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70992/testReport)** for PR 16491 at commit [`a29d078`](https://github.com/apache/spark/commit/a29d07861b1c7ed6355474b8afb9d7ffc590158f).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by wangmiao1981 <gi...@git.apache.org>.
Github user wangmiao1981 commented on the issue:
https://github.com/apache/spark/pull/16491
Jenkins, retest this please.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16491
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:
https://github.com/apache/spark/pull/16491
Yikes, thanks for fixing this!
LGTM
Merging with master
I'll also try to merge it with branch-2.1, branch-2.0, branch-1.6 but will say if I run into issues.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16491
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel retu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/16491
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/16491
@jkbradley @yanboliang please have a look
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16491: [SPARK-19110][ML][MLLIB]:DistributedLDAModel returns dif...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16491
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70992/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org