Clean up FastText Cython code, fix division by zero #2382
Merged
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
The problem is bad initialization of L1 working space (infinity) (gensim) misha@cabron:~/git/gensim$ python bug.py INFO:gensim.models.word2vec:resetting layer weights INFO:gensim.models.word2vec:collecting all words and their counts WARNING:gensim.models.word2vec:Each 'sentences' item should be a list of words (usually unicode strings). First item here is instead plain <class 'str'>. INFO:gensim.models.word2vec:PROGRESS: at sentence #0, processed 0 words, keeping 0 word types INFO:gensim.models.word2vec:collected 53 word types from a corpus of 14201 raw words and 10 sentences INFO:gensim.models.word2vec:Loading a fresh vocabulary INFO:gensim.models.word2vec:effective_min_count=5 retains 39 unique words (73% of original 53, drops 14) INFO:gensim.models.word2vec:effective_min_count=5 leaves 14173 word corpus (99% of original 14201, drops 28) INFO:gensim.models.word2vec:deleting the raw counts dictionary of 53 items INFO:gensim.models.word2vec:sample=0.001 downsamples 26 most-common words INFO:gensim.models.word2vec:downsampling leaves estimated 2588 word corpus (18.3% of prior 14173) INFO:gensim.models.word2vec:constructing a huffman tree from 39 words INFO:gensim.models.word2vec:built huffman tree with maximum node depth 11 INFO:gensim.models.fasttext:estimated required memory for 39 words, 0 buckets and 100 dimensions: 75972 bytes INFO:gensim.models.word2vec:resetting layer weights INFO:gensim.models.base_any2vec:training model with 3 workers on 39 vocabulary and 100 features, using sg=1 hs=1 sample=0.001 negative=5 window=5 l1: -inf -inf inf inf -inf -inf inf inf -inf inf inf inf -inf -inf -inf -inf -inf inf inf inf -inf -inf -inf inf inf -inf -inf inf inf -inf inf inf -inf inf inf -inf -inf inf -inf -inf inf -inf inf inf inf inf -inf inf inf inf inf inf inf inf inf inf inf -inf inf inf -inf -inf -inf inf inf inf -inf -inf inf -inf -inf -inf -inf -inf -inf -inf inf inf inf -inf inf -inf -inf -inf -inf inf inf -inf inf inf -inf -inf inf -inf inf inf inf -inf inf inf syn1[3700]: 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 0.000000 -6 -nan 6 Segmentation fault (core dumped)
mpenkov
changed the title
mpenkov
changed the title
|
@piskvorky We have several options of proceeding with this PR.
WDYT? Any preferences? |
|
I don't understand the trade-offs well enough. Can I leave this decision with you @mpenkov ? |
|
I'm gonna go with option 3). It fixes the immediate bug, and avoids the risk of introducing new ones. Opened #2404 We'll merge the Cython improvements after the bugfix release. |
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.
The main goal of this PR is to fix #2377
I found the original Cython code hard to debug, so I improved it by moving long code blocks out to separate functions and introducing better variable names. Functionally, it's the same, but it's a bit easier to read, and it helped me uncover the zero division problem.