fix: divide DocumentMAPEvaluator average precision by all relevant documents - #12255
Open
anxkhn wants to merge 2 commits into
Open
fix: divide DocumentMAPEvaluator average precision by all relevant documents#12255anxkhn wants to merge 2 commits into
anxkhn wants to merge 2 commits into
Conversation
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
|
@anxkhn is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
DocumentMAPEvaluatoroutput against the standard mean average precision definition.Proposed Changes:
DocumentMAPEvaluatorcomputed average precision assum(precision@k) / number_of_retrieved_relevant_documents. Average precision is defined assum(precision@k) / number_of_relevant_documents, where relevant documents that were never retrieved contribute a precision of zero (see the TREC measure definitions at https://trec.nist.gov/pubs/trec16/appendices/measures.pdf and thetrec_evalreference implementation, which accumulates precision and then divides by the total number of relevant documents).Because of the wrong denominator, a query with two relevant documents where only one was retrieved scored
1.0instead of0.5. In addition, the ground-truth values were never consumed once matched, so retrieving the same relevant document twice was credited twice and could push the score back up while a relevant document was still missing.This PR:
Documented behaviour of the class (the usage example in the docstring) is unaffected: its expected scores are unchanged. Scores for evaluations with missed or duplicated relevant documents will change, which is called out in the release note under
upgrade.How did you test it?
0.5for a query with two relevant documents and one distinct hit.test_run_with_complex_dataexpectations, which previously encoded the inflated values (1.0for a query where only one of two relevant documents was retrieved, and0.805…for a query where a duplicate retrieval was credited).Notes for the reviewer
This is a behaviour change for users who track absolute MAP values, so a release note with an
upgradesection is included. Happy to split the duplicate-retrieval part out if you would rather land the denominator fix on its own.Disclosure: this change was drafted with AI assistance and reviewed by a human before submission.
Checklist
fix: