Scoring function in Phrases model is hardcoded

[hristo-vrigazov]

The Phrases model is based on word counting and bigram counting and it can process sentences by a given scoring function, which can be supplied via the construtor of Phrases (the parameter scoring). However, the field for scoring function is used only in the export_phrases method. Have a look here:
https://github.com/RaRe-Technologies/gensim/blob/develop/gensim/models/phrases.py#L269
and here:
https://github.com/RaRe-Technologies/gensim/blob/develop/gensim/models/phrases.py#L284

count_a = float(vocab[word_a])
count_b = float(vocab[word_b])
count_ab = float(vocab[bigram_word])
score = scoring_function(count_a, count_b, count_ab)

However, in the __getitem__ method, the scoring uses the default scoring always https://github.com/RaRe-Technologies/gensim/blob/develop/gensim/models/phrases.py#L334 :

pa = float(vocab[word_a])
pb = float(vocab[word_b])
pab = float(vocab[bigram_word])
score = (pab - min_count) / pa / pb * len(vocab)

This looks like a bug to me (we are always using the default scoring, even if we explicitly stated npmi in the constructor). Is it okay if I open a pull request fixing this one?

[gojomo]

@michaelwsherman thoughts?

[menshikh-iv]

This problem fixed in #1573 from @michaelwsherman
Before merge #1573 I think we should merge #1568 -> resolve conflicts in #1573, wdyt @gojomo?

[gojomo]

I have no opinion on which should merge first.

[michaelwsherman]

I'm of the opinion that #1573 should be merged first :), but I'll wait for #1568 -- just be aware that it could easily be a few weeks after #1568 until I merge the code--looks like it will be a meaty merge. If it's important that the merge happen quickly then maybe #1573 should merge first since #1568 looks more active right now. But happy to make whatever work if y'all don't mind the wait.

[menshikh-iv]

Ok, let's merge #1573 first, thanks for clarification @michaelwsherman.

[menshikh-iv]

Resolved in #1573