You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by nzw0301 <gi...@git.apache.org> on 2017/09/27 18:24:37 UTC
[GitHub] spark pull request #19372: [MLLIB] Fix update equation of learning rate in W...
GitHub user nzw0301 opened a pull request:
https://github.com/apache/spark/pull/19372
[MLLIB] Fix update equation of learning rate in Word2Vec.scala
## What changes were proposed in this pull request?
Current equation of learning rate is incorrect when `numIterations` > `1`.
This PR is based on [original C code](https://github.com/tmikolov/word2vec/blob/master/word2vec.c#L393).
cc: @mengxr
## How was this patch tested?
manual tests
I modified [this example code](https://spark.apache.org/docs/2.1.1/mllib-feature-extraction.html#example).
### `numIteration=1`
#### Code
```scala
import org.apache.spark.mllib.feature.{Word2Vec, Word2VecModel}
val input = sc.textFile("data/mllib/sample_lda_data.txt").map(line => line.split(" ").toSeq)
val word2vec = new Word2Vec()
val model = word2vec.fit(input)
val synonyms = model.findSynonyms("1", 5)
for((synonym, cosineSimilarity) <- synonyms) {
println(s"$synonym $cosineSimilarity")
}
```
#### Result
```
0 0.3267880082130432
2 0.21420614421367645
3 0.19923636317253113
9 0.1063166931271553
4 0.0397246889770031
```
### `numIteration=5`
#### Code
```scala
import org.apache.spark.mllib.feature.{Word2Vec, Word2VecModel}
val input = sc.textFile("data/mllib/sample_lda_data.txt").map(line => line.split(" ").toSeq)
val word2vec = new Word2Vec()
word2vec.setNumIterations(5)
val model = word2vec.fit(input)
val synonyms = model.findSynonyms("1", 5)
for((synonym, cosineSimilarity) <- synonyms) {
println(s"$synonym $cosineSimilarity")
}
```
#### Result
```
2 0.9803512096405029
0 0.9774332642555237
3 0.9450059533119202
4 0.9394038319587708
9 -0.7876168489456177
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nzw0301/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19372.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 #19372
----
commit e2a7d393e141405f658a68f99bc4a1f53816db95
Author: Kento NOZAWA <k_...@klis.tsukuba.ac.jp>
Date: 2017-09-27T17:04:03Z
Update equation of lr
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by nzw0301 <gi...@git.apache.org>.
Github user nzw0301 commented on the issue:
https://github.com/apache/spark/pull/19372
I updated the results of word2vec example based on this PR in the first comment.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [MLLIB] Fix update equation of learning rate in Word2Vec...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/19372
You should make a JIRA as it's a non-trivial behavior change.
I agree that the intent seems to be to follow the original code. I also see that it was intentionally not used. I think the author would have to weigh in.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19372
**[Test build #3939 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3939/testReport)** for PR 19372 at commit [`90735a9`](https://github.com/apache/spark/commit/90735a9da11cfbff4ac5133d2f01c6719626f306).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by LowikC <gi...@git.apache.org>.
Github user LowikC commented on the issue:
https://github.com/apache/spark/pull/19372
Looks good to me.
Maybe @nzw0301 could split the formula for readability?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by nzw0301 <gi...@git.apache.org>.
Github user nzw0301 commented on the issue:
https://github.com/apache/spark/pull/19372
Thank you for your reviews, @LowikC.
Like this?
```scala
val totalWordsCounts = numIterations * trainWordsCount + 1
val numWordsProcessedInPreviousIterations = (k - 1) * trainWordsCount
alpha = learningRate *
(1 - (numPartitions * wordCount.toDouble + numWordsProcessedInPreviousIterations) /
totalWordsCounts)
if (alpha < learningRate * 0.0001) alpha = learningRate * 0.0001
logInfo("wordCount = " + (wordCount + numWordsProcessedInPreviousIterations) +
", alpha = " + alpha)
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19372: [SPARK-22156][MLLIB] Fix update equation of learn...
Posted by nzw0301 <gi...@git.apache.org>.
Github user nzw0301 commented on a diff in the pull request:
https://github.com/apache/spark/pull/19372#discussion_r142332793
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/feature/Word2Vec.scala ---
@@ -368,11 +371,12 @@ class Word2Vec extends Serializable with Logging {
var wc = wordCount
if (wordCount - lastWordCount > 10000) {
lwc = wordCount
- // TODO: discount by iteration?
- alpha =
- learningRate * (1 - numPartitions * wordCount.toDouble / (trainWordsCount + 1))
+ alpha = learningRate *
+ (1 - (numPartitions * wordCount.toDouble + numWordsProcessedInPreviousIterations) /
+ totalWordsCounts)
if (alpha < learningRate * 0.0001) alpha = learningRate * 0.0001
- logInfo("wordCount = " + wordCount + ", alpha = " + alpha)
+ logInfo("wordCount = " + (wordCount + numWordsProcessedInPreviousIterations) +
--- End diff --
@srowen Done.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19372
**[Test build #3943 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3943/testReport)** for PR 19372 at commit [`2ea3f18`](https://github.com/apache/spark/commit/2ea3f18bb002c38f387e01c31c6fb3ce2cb37231).
* This patch passes all tests.
* This patch **does not merge cleanly**.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19372
**[Test build #3943 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3943/testReport)** for PR 19372 at commit [`2ea3f18`](https://github.com/apache/spark/commit/2ea3f18bb002c38f387e01c31c6fb3ce2cb37231).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19372: [SPARK-22156][MLLIB] Fix update equation of learn...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/19372#discussion_r142328125
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/feature/Word2Vec.scala ---
@@ -368,11 +371,12 @@ class Word2Vec extends Serializable with Logging {
var wc = wordCount
if (wordCount - lastWordCount > 10000) {
lwc = wordCount
- // TODO: discount by iteration?
- alpha =
- learningRate * (1 - numPartitions * wordCount.toDouble / (trainWordsCount + 1))
+ alpha = learningRate *
+ (1 - (numPartitions * wordCount.toDouble + numWordsProcessedInPreviousIterations) /
+ totalWordsCounts)
if (alpha < learningRate * 0.0001) alpha = learningRate * 0.0001
- logInfo("wordCount = " + wordCount + ", alpha = " + alpha)
+ logInfo("wordCount = " + (wordCount + numWordsProcessedInPreviousIterations) +
--- End diff --
If you update this again, you can use string interpolation: `logInfo(s"wordCount = ${wordCount + ...}, alpha = $alpha")`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/19372
Merged to master
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/19372
What do you think @LowikC ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19372: [SPARK-22156][MLLIB] Fix update equation of learn...
Posted by LowikC <gi...@git.apache.org>.
Github user LowikC commented on a diff in the pull request:
https://github.com/apache/spark/pull/19372#discussion_r141607418
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/feature/Word2Vec.scala ---
@@ -368,9 +368,9 @@ class Word2Vec extends Serializable with Logging {
var wc = wordCount
if (wordCount - lastWordCount > 10000) {
lwc = wordCount
- alpha =
- learningRate *
- (1 - numPartitions * wordCount.toDouble / (numIterations * trainWordsCount + 1))
+ alpha = learningRate *
+ (1 - numPartitions * wordCount.toDouble + (k - 1) * trainWordsCount /
--- End diff --
you need `numPartitions * wordCount.toDouble + (k - 1) * trainWordsCount` between parentheses
`alpha = learningRate * (1 - (numPartitions * wordCount.toDouble + (k - 1) * trainWordsCount) / (numIterations * trainWordsCount + 1))`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by LowikC <gi...@git.apache.org>.
Github user LowikC commented on the issue:
https://github.com/apache/spark/pull/19372
I think the PR is incorrect:
- the original C code decreases the learning rate linearly from `starting_alpha` to 0, across all iterations
new_alpha = `starting_alpha` * (1 - progress), progress = `word_count_actual` / (numIterations * numWordsPerIteration + 1)
Note that `word_count_actual` counts the number of words processed so far, in all iterations.
- in the PR, word_count_actual is called `wordCount`, but `wordCount` is reset at the beginning of each iteration (see https://github.com/nzw0301/spark/blob/e2a7d393e141405f658a68f99bc4a1f53816db95/mllib/src/main/scala/org/apache/spark/mllib/feature/Word2Vec.scala#L365).
- the current Spark code is also different: it decreases the learning rate linearly from `learningRate` to 0, in one iteration. At the beginning of the next iteration, the value is again `learningRate`
- the "right" formula (to keep the behavior of the original C code) should be:
```
val numWordsToProcess = numIterations * trainWordsCount + 1
val numWordsProcessedInPreviousIterations = (k - 1) * trainWordsCount
val numWordsProcessedInCurrentIteration = numPartitions * wordCount.toDouble
val progress = (numWordsProcessedInPreviousIterations + numWordsProcessedInCurrentIteration) / numWordsToProcess
alpha = learningRate * (1 - progress)
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19372: [SPARK-22156][MLLIB] Fix update equation of learn...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19372
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by nzw0301 <gi...@git.apache.org>.
Github user nzw0301 commented on the issue:
https://github.com/apache/spark/pull/19372
Thank you for your comment, @LowikC.
You are right, my PR code is incorrect.
Correct update formula is
```scala
alpha = learningRate *
(1 - numPartitions * wordCount.toDouble + (k - 1) * trainWordsCount /
(numIterations * trainWordsCount + 1))
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19372: [SPARK-22156][MLLIB] Fix update equation of learn...
Posted by nzw0301 <gi...@git.apache.org>.
Github user nzw0301 commented on a diff in the pull request:
https://github.com/apache/spark/pull/19372#discussion_r141609260
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/feature/Word2Vec.scala ---
@@ -368,9 +368,9 @@ class Word2Vec extends Serializable with Logging {
var wc = wordCount
if (wordCount - lastWordCount > 10000) {
lwc = wordCount
- alpha =
- learningRate *
- (1 - numPartitions * wordCount.toDouble / (numIterations * trainWordsCount + 1))
+ alpha = learningRate *
+ (1 - numPartitions * wordCount.toDouble + (k - 1) * trainWordsCount /
--- End diff --
oh... Thanks! I fixed it
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19372
**[Test build #3939 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3939/testReport)** for PR 19372 at commit [`90735a9`](https://github.com/apache/spark/commit/90735a9da11cfbff4ac5133d2f01c6719626f306).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [MLLIB] Fix update equation of learning rate in Word2Vec...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19372
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [MLLIB] Fix update equation of learning rate in Word2Vec...
Posted by nzw0301 <gi...@git.apache.org>.
Github user nzw0301 commented on the issue:
https://github.com/apache/spark/pull/19372
Thank you for your comment, @srowen.
I'll create an isuen on JIRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19372: [SPARK-22156][MLLIB] Fix update equation of learning rat...
Posted by nzw0301 <gi...@git.apache.org>.
Github user nzw0301 commented on the issue:
https://github.com/apache/spark/pull/19372
Thank you for your kindful reviews!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org