Fix bug in bm25.py

[souravsingh]

Related to #1718

[piskvorky]

That looks like a serious / critical difference, thanks! @menshikh-iv can you please verify the new version is correct?

What functionality is affected? How did this bug manifest itself in practice?

[souravsingh]

Nothing is mentioned in the issue itself. The person filing the issue just mentions that it is a bug, and didn't say anything about how it affects the functionality.

[piskvorky]

Probably best to double check and verify. The summarization subpackage is still used very sparsely (I didn't even know it had BM25 in it).

[souravsingh]

The person who filed the issue mentioned the following-

Actually, I used the bm25 module independently of the
summarization module just for IR.

Basically, I wanted to quickly compare bm25 with tf-idf, pylucene's api
seemed to be too cumbersome, and I did not want to reinvent the wheel.
Gensim generally has pretty good coding standards that's why I decided to
use bm25 from gensim instead of the various other BM25 implementations on
github. But after implementation I noticed that the results from bm25 were
worse than standard tf-idf and that's why I looked at the code in more
detail. The data that I am working on comes from https://www.ldc.upenn.edu
so its hard to share it.

[piskvorky]

Yes, I saw.

It's better link to @se4u's answer, instead of copy-pasting.

[souravsingh]

So we confirm the behavior of the bug by running both tf-idf and bm25 correct? It might be a bit difficult since bm25 doesn't have any docstrings to help in understanding the behavior.

[menshikh-iv]

BM25 formula:

Current variant (in code) looks correct now.

[eliotpbrenner]

Can you please double check? I believe it should be len(self.corpus[index]), not len(document)
Thanks!

[se4u]

@eliotpbrenner yes you are right !

[menshikh-iv]

@eliotpbrenner 24a692b#commitcomment-26708993