feat: add max_rel_per_label to support recall for labeled data#826
feat: add max_rel_per_label to support recall for labeled data#826JoanFM merged 11 commits intodocarray:mainfrom
Conversation
Signed-off-by: Michael Guenther <[email protected]>
Codecov ReportBase: 82.91% // Head: 72.73% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #826 +/- ##
===========================================
- Coverage 82.91% 72.73% -10.18%
===========================================
Files 138 138
Lines 7122 7137 +15
===========================================
- Hits 5905 5191 -714
- Misses 1217 1946 +729
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| caller_max_rel = kwargs.pop('max_rel', None) | ||
| for d, gd in zip(self, ground_truth): | ||
| max_rel = caller_max_rel or len(gd.matches) | ||
| if caller_max_rel: |
There was a problem hiding this comment.
I think you need to refactor the if else logic here a bit
for d, gd in zip(self, ground_truth):
if caller_max_rel:
max_rel = caller_max_rel
if ground_truth_type == 'labels':
if max_rel_per_label:
max_rel = max_rel_per_label.get(d.tags[label_tag], None)
if max_rel is None:
raise ValueError(
'`max_rel_per_label` misses the label ' + str(d.tags[label_tag])
)
else:
raise ValueError('max_rel is required or something')
else:
max_rel = len(gd.matches)There was a problem hiding this comment.
I think it is correct, that caller_max_rel is used if the user provides a max_rel attribute explicitly.
There was a problem hiding this comment.
This exception when max_rel is not set should also not be their because most of the metrics do not require max_rel, but setting it to None might be better than setting it to len(gd.matches). I will change this.
Signed-off-by: Michael Guenther <[email protected]>
d65eb05 to
29777b7
Compare
Signed-off-by: Michael Guenther <[email protected]>
docarray/array/mixins/evaluation.py
Outdated
| if ground_truth and label_tag in ground_truth[0].tags: | ||
| max_rel_per_label = dict(Counter([d.tags[label_tag] for d in ground_truth])) | ||
| elif not ground_truth and label_tag in query_data[0].tags: | ||
| max_rel_per_label = dict(Counter([d.tags[label_tag] for d in query_data])) | ||
| else: | ||
| max_rel_per_label = None |
There was a problem hiding this comment.
i don't understand, max_rel_per_label is a variable you passed into the function, then what is this max_rel_per_label?
There was a problem hiding this comment.
can you elaberate a bit the naming, max_rel_per_label? why not num_relevant_documents_per_label or something like that?
802d8da to
62e70f8
Compare
Signed-off-by: Michael Guenther <[email protected]>
Signed-off-by: Michael Guenther <[email protected]>
9dbabed to
22f0dc9
Compare
Signed-off-by: Michael Guenther <[email protected]>
Signed-off-by: Michael Guenther <[email protected]>
68852b9 to
5739b4f
Compare
JoanFM
left a comment
There was a problem hiding this comment.
I would like to see some changes in Documentation
Signed-off-by: Michael Guenther <[email protected]>
|
|
||
| In this case, the keyword argument `k` is passed to all metric functions, even though it does not fulfill any specific function for the calculation of the reciprocal rank. | ||
|
|
||
| ### The max-rel parameter |
There was a problem hiding this comment.
| ### The max-rel parameter | |
| ### The max_rel parameter |
| ### The max-rel parameter | ||
|
|
||
| Some metric functions shown in the table above require a `max_rel` parameter. | ||
| This parameter should be set to the number of relevant documents in the document collection. |
There was a problem hiding this comment.
| This parameter should be set to the number of relevant documents in the document collection. | |
| This parameter should be set to the number of relevant Documents in the Document collection. |
|
|
||
| Some metric functions shown in the table above require a `max_rel` parameter. | ||
| This parameter should be set to the number of relevant documents in the document collection. | ||
| Without the knowledge of this number, metrics like `recall_at_k` and `f1_score_at_k` can be calculated. |
| This parameter should be set to the number of relevant documents in the document collection. | ||
| Without the knowledge of this number, metrics like `recall_at_k` and `f1_score_at_k` can be calculated. | ||
|
|
||
| In the `evaluate` function, one can provide a keyword argument `max_rel`, which is then used for all queries. |
| {'recall_at_k': 1.0} | ||
| ``` | ||
|
|
||
| Since all relevant documents are in the matches, the recall is one. |
There was a problem hiding this comment.
Capitalize Documents throughout text
Signed-off-by: Michael Guenther <[email protected]>
Signed-off-by: Michael Guenther [email protected]
Goals:
For labeled datasets it is not trivial to calculate metrics like recall and F1 measure, which require the knowledge of the number of relevant documents in the document collection for each query since neither the whole set of relevant documents nor the number of documents with a specific label is provided to the
evaluatefunction.To enable the calculation of recall and F1 measure, this PR
max_rel_per_label: Dictto the evaluate function which provides the number of relevant documents for each label, i.e., the number of documents in the collection with this label.max_rel_per_labelin theembed_and_evaluate_function.Code Example: