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

Map metric #71

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Map metric #71

wants to merge 2 commits into from

Conversation

beeva-ramiromanso
Copy link

From #58

Wanted to add the MAP metric to the evaluation.py script.

Same interface as mrr_score. If the training dataset is provided, scores of known interactions are set to very low values to discard them.

Next step: Tweak it to calculate MAP@K

Copy link
Owner

@maciejkula maciejkula left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

In addition to my inline comments, would you mind adding a test for the functionality as well?


prec = []
ranking = np.sort(st.rankdata(predictions)[row.indices])
for index, value in enumerate(ranking):
Copy link
Owner

Choose a reason for hiding this comment

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

Could probably do prec = np.arange(1, len(ranking) + 1) / ranking or something similar to make use of vectorized numpy operations?

prec.append((index + 1) / value)
ap.append(sum(prec) / len(ranking))

return np.mean(ap)
Copy link
Owner

Choose a reason for hiding this comment

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

The other metrics return raw arrays, leaving the averaging (or getting other descriptive statistics) down to the user. Could we do it this way here as well?

@beeva-ramiromanso
Copy link
Author

I'll try to create a small test for the metric. I have a controlled test with a set scoring instead of model.predict for the metric so I'll devise something.

As for the inline comments, sure, no problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants