Re-design "*2vec" implementations

[manneshiva]

This PR aims to better the current "*2vec" implementation design (Doc2Vec, Word2Vec, FastText, Sent2Vec, Poincare), making it more modular and ensuring maximum code re-use.
Link summarizing this PR.

[manneshiva]

The first commit contains the initial design to help facilitate discussion. It outlines present thoughts about the design and is in no way complete. @janpom Image below for your reference:

[gojomo]

Simply extracting/clarifying the existing implementation could offer value – but also is less-likely to clear the way for all-new algorithmic variations or optimizations. For example, a very generic "2Vec" model might not even have the idea of a 'vocabulary', or a step within itself that builds-a-vocabulary... as opposed to just taking corpuses or configuration prepped by other objects.

That is, this sort of incremental re-factoring starting from the current design might not offer the same reuse/efficiency/flexibility benefits for #1623 a from-scratch design could give.)

[manneshiva]

@gojomo Agreed. These first few commits were only meant to facilitate discussions in our meetings. The idea is to sketch the design in a top-down fashion -- first, fix the public APIs that are to be exposed to users and then refactor everything else accordingly. This will involve designing a few classes from scratch, at the same time, will try and make use of existing classes/functions by making them more generic. Currently, the idea is to try and maintain backward compatibility (in terms of end-user APIs) unless some design changes make it absolutely necessary to change them (APIs). The scope of this PR may not include every point in #1623 but is definitely a big step in that direction which will make it easier to incorporate many of the listed points in the future.
Current status: We have finalized on the public interfaces. We also discussed providing/incorporating callbacks functionality in a way similar to keras. You should see be able to see these commits soon. Your review/comments would be helpful.

[gojomo]

I'll keep an eye for reviewable updates. I would suggest starting from a long, weakly-prioritized list of potential new functionality, then cull that to the things that can be usefully merged. (That is, more bottom-up from specifics of novel functions/combinations.) Starting from top-level, end-user visible interfaces, and maintaining compatibility with the (somewhat clunky) older interfaces, risks severely-limiting what new breakthrough capabilities can be delivered. (If they were easy to fit into the existing API, they could be added incrementally, rather than via a new design.)

[janpom]

indented extra level

[menshikh-iv]

Looks good for me

[menshikh-iv]

What's a magic constant here?

[menshikh-iv]

This is always "words"? Probably "entities" is better for the general case.

[menshikh-iv]

return super(...)

[janpom]

Some thoughts about the parameter naming. Using a generic term such as entity (meaning word or sentence or document) is potentially confusing for the user. Also, I see little value in keeping the API exactly the same for all algorithms. Don't get me wrong. There's certainly value in keeping the API consistent so that once the user learns how to use Word2Vec, it will be easy for them to use Doc2Vec, but it's not necessary (and probably not even desirable) that the public API is exactly the same including parameter names.

For example, I consider the following:

doc2vec = Doc2Vec(data)
print(doc2vec.kv.similarity(document1=mydoc1, document2=mydoc2))

word2vec = Word2Vec(data)
print(word2vec.kv.similarity(word1="foo", word2="bar"))

better than:

doc2vec = Doc2Vec(data)
print(doc2vec.kv.similarity(entity1=mydoc1, entity2=mydoc2))

word2vec = Word2Vec(data)
print(word2vec.kv.similarity(entity1="foo", entity2="bar"))

Should the different parameter naming be a problem for code reuse, we can always keep the same parameter names in internal methods and define public methods as thin wrappers. For example:

class KeyedVectorsBase(object):
    def _similarity(self, entity1, entity2):
        # calculate and return similarity

class Word2VecKeyedVectors(KeyedVectorsBase):
    def similarity(self, word1, word2):
        return self._similarity(word1, word2)

class Doc2VecKeyedVectors(KeyedVectorsBase):
    def similarity(self, document1, document2):
        return self._similarity(document1, document2)

[janpom]

There was probably some misunderstanding in "functions preferred over methods". They are preferred but only where it makes sense. Where the function parameter is the model, it makes more sense to keep it as a method, especially if the function modifies the model data such as is the case here.

Out of the functions in this module, I would only keep _raw_word_count() as a function. The other ones sound more like methods to me.

Should the _raw_word_count() be the only function in this module, it's not even worth having the module, especially if the function is a one-liner.

Also, if the functions are to be called from other modules, they shouldn't be private (no leading underscore).