[ViderStar]

When N is larger than the number of items, the CPU matrix factorization models return N results anyway, and the extra slots are filled with itemid 0 and score 0:

import numpy as np, scipy.sparse as sp
from implicit.als import AlternatingLeastSquares

m = sp.random(30, 20, density=0.2, format="csr", random_state=0)
m.data[:] = 1
model = AlternatingLeastSquares(factors=4, iterations=2, random_state=0)
model.fit(m, show_progress=False)

ids, scores = model.recommend(np.arange(2), m[:2], N=25)
print(ids[0])  # [11 12  9 ... 16  8  0  0  0  0  0]

This breaks ranking_metrics_at_k when K is larger than the number of items. Every padded 0 counts as a hit for users that have item 0 in the test set, so precision can go above 1. For example, on a 300x200 random matrix, ranking_metrics_at_k(model, train, test, K=300) gives precision=1.49. Also, ranking_metrics_at_k always loops over K columns of ids with bounds checking disabled, so if a model returns fewer than K items, it reads past the end of the array.

Changes:

• implicit/cpu/topk.pyx: limit k to the number of items. This fixes recommend, similar_items and similar_users for the CPU models. The existing test_recommend already says that asking for too many items "should return only the available items" (recommend N top recommendations #22).
• implicit/evaluation.pyx: only loop over the items that the model actually returned.

I added test_ranking_metrics_k_larger_than_number_of_items for ALS, BPR and LMF. It checks that recommend with N=100 on 50 items returns 50 columns, and that the metrics with K=100 are the same as with K=50. It fails on main and passes with this change. The full test suite passes locally on macOS arm64 (167 passed, 54 skipped since I don't have CUDA). I didn't change the GPU code path since I can't test it.

[ViderStar]

@benfred gentle ping: could you take a look when you have time? The ranking metrics read past the end of the array in some cases, which can give precision > 1; the PR includes a test.

[benfred]

thanks!

Mostly looks good, some nitpicks below:

[benfred]

we should probably also fix this in the GPU code

implicit/implicit/gpu/knn.cu

Line 132 in 8a95dbe

void KnnQuery::topk_impl(const Matrix &items, const Matrix &query, int k,

[benfred]

maybe just num_recs = min(K, items) ? (The model shouldn't be returning fewer than this).