Bug: possible mistake in MMR calculation

[schackartk]

Hello,

If I am reading the code correctly, there is a mistake in the implementation of maximal marginal relevance (MMR) calculation.

Referring to the original publication https://doi.org/10.1145/290941.291025, the calculation is:

and the code as currently implemented:

mmr = (
    1 - diversity
) * candidate_similarities - diversity * target_similarities.reshape(-1, 1)
mmr_idx = candidates_idx[np.argmax(mmr)]

assuming:

• diversity is equal to 1-λ
• _Sim_1(Di,Q) corresponds to candidate similarities
• _max Sim_2(D_i,Dj) corresponds to target_similarities

and I am assuming the last point because of the code:

target_similarities = np.max(
        word_similarity[candidates_idx][:, keywords_idx], axis=1
    )

the code should be:

mmr = (1 - diversity) *
(candidate_similarities - diversity * target_similarities.reshape(-1, 1))
mmr_idx = candidates_idx[np.argmax(mmr)]

So it appears to me that diversity is not distributed to both similarity terms as in the original equation; there needs to be parens around the difference between the similarity terms

I would note that I have seen a similar lack of parentheses, which distribute the diversity term (λ), in other works, for example http://www.cs.bilkent.edu.tr/~canf/CS533/hwSpring14/eightMinPresentations/handoutMMR.pdf

[MaartenGr]

Thanks for sharing this! Coincidentally, I indeed used the following as the main source for calculating the diversity:

I would note that I have seen a similar lack of parentheses, which distribute the diversity term (λ), in other works, for example http://www.cs.bilkent.edu.tr/~canf/CS533/hwSpring14/eightMinPresentations/handoutMMR.pdf

Having said that, it might be worthwhile to test out the effect of changing the parentheses. I am quite curious to see how that would affect representation. Moreover, there are quite a number of other libraries that have MMR implemented, such as LangChain and vector database applications. I could check out what their prefered method of doing so is.