Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix .rank() method for multiple models #615

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

hieuddo
Copy link
Member

@hieuddo hieuddo commented Apr 22, 2024

Description

The new Recommender.rank() function adds k as required value, which breaks some models that do not use k in ranking evaluation (e.g., ComparER, EFM, LRPPM).

I put k into **kwargs and use k = kwargs.get("k", -1) instead.
Another option is to add k=None to incompatible models.

These two sound like backward compatibility. Should make old models forward-compatible instead.

I added and updated topK ranking for mentioned models.

@hieuddo hieuddo requested review from tqtg and lthoang April 22, 2024 08:42
@tqtg
Copy link
Member

tqtg commented Apr 22, 2024

@lthoang @hieuddo one question for these models, is it possible to move the part of computing known_item_scores at the beginning of the rank function to the score function?

@lthoang
Copy link
Member

lthoang commented Apr 23, 2024

@lthoang @hieuddo one question for these models, is it possible to move the part of computing known_item_scores at the beginning of the rank function to the score function?

@tqtg Could you please elaborate more? Based on my understanding, these models compute ranking scores by weighting the tradeoff between predicted ratings and top-k aspects via the following formula:

$$RankingScore_{ij} = \alpha \cdot Rating_{ij} + (1-\alpha) \cdot AverageTopKAspects_{ij}$$

We can replace $Rating_{ij}$ with score(i, j). However, we may not need to recompute the score in $ComparER_{sub}$ model as ratings are already computed along with aspect scores.

@tqtg
Copy link
Member

tqtg commented Apr 23, 2024

I was thinking whether we can reuse the rank() function as implemented by default for all models in Cornac. It seems that these models trying to do rating prediction and top-K recommendation with different scoring mechanisms so we need to overwrite the rank() function.

@tqtg
Copy link
Member

tqtg commented Apr 23, 2024

@hieuddo the changes look fine to me. @lthoang please help double check the detail. Thank you both.

Copy link
Member

@lthoang lthoang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hieuddo hieuddo merged commit ae7ba86 into PreferredAI:master Apr 24, 2024
24 checks passed
@hieuddo hieuddo added the bug Something isn't working label Apr 24, 2024
@hieuddo hieuddo deleted the fix-recommender-rank branch May 20, 2024 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants