xiangwang1223 / neural_graph_collaborative_filtering

Neural Graph Collaborative Filtering, SIGIR2019
MIT License
781 stars 261 forks source link

Not consistent with original NDCG #34

Open newlei opened 4 years ago

newlei commented 4 years ago

Hi, through Wikipedia and related papers, I find that NDCG=DCG/IDCG. The maximum possible DCG is called Ideal DCG (IDCG), in other words, IDCG is the DCG value of the best ranking function on a dataset. so for a specific user in test data, the best ranking is unique, then the IDCG is unchanging. However, in your code, the IDCG is changing. The calculation of the dcg_max(IDCG) depends on parameters r, but r is not the best ranking and will change according to the predicted of the model.

    r = []
    for i in K_max_item_score:
        if i in user_pos_test:
            r.append(1)
        else:
            r.append(0)
    auc = get_auc(item_score, user_pos_test)
    return r, auc

and

metrics.ndcg_at_k(r, K) 
 def ndcg_at_k(r, k, method=1): 
    dcg_max = dcg_at_k(sorted(r, reverse=True), k, method)
    #------------------------------#
    #here , the value of r  will changes with the model, so the  dcg_max will change.
    # But, it is not consistent with the original IDCG.
    if not dcg_max:
        return 0.
    return dcg_at_k(r, k, method) / dcg_max
proto-n commented 4 years ago

Agreed. Simple example why this is not the right thing to do: with the current version, one hit in the 1. rank ([1,0,0]) is a perfect score, while one hit in 1. rank and one hit in 3. rank ([1,0,1]) is less than perfect . This is obviously wrong, since the latter should be strictly better.

Correctly it would read something like dcg_max = dcg_at_k([1 for i in range(k)], k, method), i.e. we compare to the best achievable score. Unless of course the user's test set can be smaller than k in which case it's dcg_max = dcg_at_k([int(i<len(user_test)) for i in range(k)], k, method)