-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[FEATURE] Simplify some arguments of the evaluation #1737
Comments
@ChuyangKe can you review this? |
Perhaps another way is to use |
i can work on this issue, Can someone point-me to the correct file? I think the changes has to be cascaded and checked as well as multiple modules might be using these metrics? |
@AdityaSoni190319 can you implement the solution proposed by Le (yueguoguo)? metrics are here https://github.com/microsoft/recommenders/blob/main/recommenders/evaluation/python_evaluation.py |
@miguelgfierro I have tried to patch this fix and had run the tests for the given methods as well. LMK your feedback or any other changes needed. |
Description
I found that for metrics like
map_at_k
,ndcg_at_k
,precision_at_k
,recall_at_k
, thecol_rating
argument seem unnecessary if therelevancy_method
is"top_k"
. However, the current implementation will report an error if this argument is not passed in:Expected behavior with the suggested feature
I think it's a bit confusing to pass in unnecessary arguments.
A simple but dirty fix would be to change the default arguments
DEFAULT_RATING_COL
toDEFAULT_USER_COL
.Other Comments
The text was updated successfully, but these errors were encountered: