Incorporate Retrieval Scores into RM3 #453
Conversation
…ing terrier-org#407 will not change their behaviour
pyterrier/rewrite.py
Outdated
| occurrences = [0] * len(docids) | ||
|
|
||
| elif "docno" in topics_and_res.columns: | ||
| docnos = topics_and_res[topics_and_res["qid"] == qid]["docno"].values | ||
| docids = [] | ||
| scores = [] | ||
| docnos_to_scores = {i:0.0 for i in docnos} | ||
| if self.requires_scores: | ||
| docnos_to_scores = {i['docno']: i['score'] for _, i in topics_and_res[topics_and_res["qid"] == qid].iterrows()} |
There was a problem hiding this comment.
do we need to make a dictionary here? why not extract a list like was done above?
There was a problem hiding this comment.
A bit of an aside, but the topics_and_res[topics_and_res["qid"] == qid] selector is also performed a few lines above, and it's pretty inefficient. Can we cache the result to avoid doing it twice?
There was a problem hiding this comment.
Indeed, we could indeed cache the results.
And I also see now that we do not need a dict, I confused the skipped part, I was thinking that the if condition for skipped documents would have a continue statement, but now I see that this is not the case. I will modify the code.
There was a problem hiding this comment.
I modified the code accordingly.
|
Great, thanks @mam10eks for the high quality explanations. I have removed some unit test checks (Bo1 etc dont need the additional PRF jar), and added a single comment (do we need a dict?) |
cmacdonald
changed the title
|
I made some edits to ensure it didnt crash if some docno could not be resolved. @seanmacavaney are you happy? I'll merge this forward to the java branch after its committed to master. |
|
lgtm! I think we can remove the test jankiness once in the java branch. |
|
Cool :) |
Backport #453 to Java branch
This pull request (currently in draft) is aimed to address issue 407.