[SPARK-19922][ML] small speedups to findSynonyms #17263
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently generating synonyms using a model with 3m words is painfully slow. These efficiencies have sped things up by more than 17%. Address a few issues in the findSynonyms logic: 1) no need to zero out the cosineVec array each time, since default value for float arrays is 0.0f. This should offer some nice speedups 2) use floats throughout. The conversion to Doubles before doing the priorityQueue is totally superflous, since all the computations are done using floats anyway 3) convert the slow for(i <- cosVec.indices), which combines a scala closure with a Range, to an ugly but faster while loop
|
Test build #74393 has finished for PR 17263 at commit
|
|
Test build #74394 has finished for PR 17263 at commit
|
srowen
reviewed
Mar 12, 2017
| @@ -570,7 +570,7 @@ class Word2VecModel private[spark] ( | |||
| require(num > 0, "Number of similar words should > 0") | |||
|
|
|||
| val fVector = vector.toArray.map(_.toFloat) | |||
| val cosineVec = Array.fill[Float](numWords)(0) | |||
| val cosineVec = new Array[Float](numWords) // default value is 0.0f | |||
There was a problem hiding this comment.
It's fine, it doesn't need a comment
| if (norm == 0.0) { | ||
| cosVec(ind) = 0.0 | ||
| cosineVec(i) = 0 |
There was a problem hiding this comment.
I'd still write 0.0f for clarity but it's no big deal. I guess we can write norm == 0.0f too
also, remove comment about default value, it's not needed
|
Test build #74407 has finished for PR 17263 at commit
|
srowen
approved these changes
Mar 13, 2017
|
Merged to master |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently generating synonyms using a large model (I've tested with 3m words) is very slow. These efficiencies have sped things up for us by ~17%
I wasn't sure if such small changes were worthy of a jira, but the guidelines seemed to suggest that that is the preferred approach
What changes were proposed in this pull request?
Address a few small issues in the findSynonyms logic:
Array.fillto zero out thecosineVecarray. The default float value in Scala and Java is 0.0f, so explicitly setting the values to zero is not neededpriorityQueueis totally superfluous, since all the similarity computations are done using Floats anyway. Creating a second large array just serves to put extra strain on the GCfor(i <- cosVec.indices)to an ugly, but faster,whileloopThese efficiencies are really only apparent when working with a large model
How was this patch tested?
Existing unit tests + some in-house tests to time the difference
cc @jkbradley @MLnick @srowen