Open tstadel opened 2 years ago
Hi @tstadel,
Thank you for bringing up this issue. I agree with your point. Could you provide an implementation that you have in mind for a common abstract base class and provide a pull request for that? I can check and merge it in the future.
Also, happy to see BEIR integrated within haystack.
Kind Regards, Nandan Thakur
Hey @NThakur20,
sure. I'll submit a PR in the next days.
Shouldn't the line:
Union[Type[DRES], Type[DRFS], Type[BM25], Type[SS]]
Be:
Union[DRES, DRFS, BM25, SS]
In the examples you use this code:
model = DRES(models.SentenceBERT("msmarco-roberta-base-ance-fristp"))
retriever = EvaluateRetrieval(model, score_function="dot")
In this case, model
is not of type Type[DRES]
, it is of type DRES
The current design of
EvaluateRetrieval
is already supporting multiple different retrievers. See https://github.com/beir-cellar/beir/blob/bb8a0582d8a82d6f4fddee8b668b1ebaa42985ef/beir/retrieval/evaluation.py#L14 However according to typings the supplied retrievers are limited to those implemented in BEIR. If we want to integrate BEIR in other tools (like e.g. haystack) a custom retriever is basically easy to implement (see https://github.com/deepset-ai/haystack/pull/2333). This is due to the fact that all retrievers share a common interface of course. However the typings onEvaluateRetrieval
do not account for this explicitly. A common abstract base class instead of a Tuple of all implemented retrievers would make this extensibility more comprehensive.