Make evaluator invariant of input request type order #215
Conversation
| # ===== Unpack the request ===== | ||
| prediction_list.sort( | ||
| key=lambda x: x.request_index | ||
| ) # When we use Loglikelihood for several tokens we have all the options here |
There was a problem hiding this comment.
This seems to be unnecessary as the lm responses are reordered back to their original order in lm's methods. I removed it in this PR because for example when the responses for a document is like [LoglikelihoodReturn(index=0), LoglikelihoodReturn(index=1), GreedyUntilReturn(index=0)] (which occurs now because RequestType.LOGLIKELIHOOD resides before RequestType.GREEDY_UNTIL in RequestType), this statement wrongly changes it to [LoglikelihoodReturn(index=0), GreedyUntilReturn(index=0)], LoglikelihoodReturn(index=1). If you think we should keep it, we could add the request type to the sort key.
There was a problem hiding this comment.
The list sorted here will only contain request of the same type. It is used for loglikelihood requests. For example, for one loglikelihood request with 4 choices we would have 4 different request. We sort them here so that we have the same ordering from the task doc.
There was a problem hiding this comment.
But by looking at the upper for loop, we find that all responses of of all types associated with a single task and single example go to this list.
There was a problem hiding this comment.
oh yeah i forgot that we could have both greedy and multichoice returns for one task.
THis could be an issue when computing results. Did you check that removing the sorting was indeed fixing this issue ?
There was a problem hiding this comment.
Yes I did. It could be verified by the example I put in #193
Hi there!
To make
evaluator.py::evaluate(...,request_dicts,...)invariant of request type order inrequest_dicts.As
task.process_results()expects the responses to be in a specific order,evaluate()must prepare them in that very order, but currently it depends on the order in itsrequest_dictsargs. This PR proposes to useRequestTypeas the ordering reference to which bothtask.process_results()andevaluate()submit.Fixes #193