[MRG] Fixes incorrect vectors learned during online training for FastText models
#1756
Conversation
…_vocab` are different
…igning to `syn0`
manneshiva
changed the title
FastText modelsFastText models
| @@ -419,6 +419,8 @@ def online_sanity(self, model): | |||
| self.assertTrue(all(['terrorism' not in l for l in others])) | |||
| model.build_vocab(others) | |||
| model.train(others, total_examples=model.corpus_count, epochs=model.iter) | |||
| # checks that `syn0` is different from `syn0_vocab` | |||
| self.assertFalse(np.all(np.equal(model.wv.syn0, model.wv.syn0_vocab))) | |||
There was a problem hiding this comment.
This is a rather weird check. Nobody really cares if syn0 or syn0_vocab are different. What we do care about is that syn0_vocab is incorrect because it gets overwritten by get_vocab_word_vecs(). A more meaningful check would be either asserting certain properties of syn0_vocab that should hold or checking that syn0_vocab is the same before and after calling get_vocab_word_vecs().
There was a problem hiding this comment.
What @janpom said + check out np.allclose :)
There was a problem hiding this comment.
I will add another test case but let us also retain the np.equal check for syn0 and syn0_vocab.
|
Typically, we shouldn't use |
This PR addresses #1752. This bug was caused by incorrect assignment of vectors to
syn0after training aFastTextmodel.