NDCG=0.0 instead of nan for IDCG=0.0

[lironT74]

Hi,
I believe this update is necessary, since if IDCG is 0 then NDCG should be 0, and not nan.

[guidozuc]

thanks @lironT74. This is a quite particular case.
Mathematically, to compute nDCG, the DCG value is divided by the IDCG value. Then, if IDCG=0, x/0 = nan, i.e. dividing a number by zero is undefined.
In practice, if IDCG=0, it means there are no relevant documents for that query in the qrels. Thus, one should exclude the query from the dataset or the evaluation: whatever you do on that query, will be impossible to evaluate (as you have no relevant documents for that query).

[ishnid]

I can see both sides :-)

I would suggest that the project should aim to follow whatever behaviour the official trec_eval has in situations like these.

[lironT74]

@guidozuc I agree, but as @ishnid mentioned, trec_eval outputs 0.0 for those cases.

[joaopalotti]

Hi all,

Thanks for your comments, @lironT74, @ishnid and @guidozuc.
This issue here is somewhat close to #24. For the input that we got from there, we have recently modified some of the metrics to behave closer to trec_eval for particular corner cases like the one that you mentioned.

Unfortunately, your PR does not fix the problem as a whole. For example, get_ndcg with per_query==True would return a dataframe with all known queries (even the ones that are not in the run) and doing something like returned_df.mean() would result in a value that is different than running get_ndcg with per_query==False. I have taken this into account in my latest try to fix this issue (please see this commit here).
But while it fixes the problem for NDCG, we still need to propagate this fix to other metrics.
Would you be up for this challenge, @lironT74?

[lironT74]

@joaopalotti I see. It seems that the fix is changing other metrics which might suffer from this issue the (almost exact) same way you did for NDCG + the fillna per query fix, correct?

I can work on it in my spare time, but I am not sure which additional metrics need to be changed.

[joaopalotti]

Thanks @lironT74, your suggestion was taken and included in the last commit!