Skip to content

chore: add DenseRetriever abstraction#3252

Merged
ZanSara merged 53 commits intomainfrom
dense_retriever
Sep 21, 2022
Merged

chore: add DenseRetriever abstraction#3252
ZanSara merged 53 commits intomainfrom
dense_retriever

Conversation

@tstadel
Copy link
Member

@tstadel tstadel commented Sep 20, 2022

Related Issues

Proposed Changes:

  • introduce DenseRetriever abstraction to facilitate usage of retrievers in document store's update_embeddings
  • streamline validation of embedding shape across all document stores (BaseDocumentStore_validate_embeddings_shape)

Breaking Changes:

DensePassageRetriever and EmbeddingRetriever: embed_queries's arg changed from texts to queries

How did you test it?

  • no new tests necessary

Notes for the reviewer

Checklist

@tstadel tstadel requested review from a team as code owners September 20, 2022 17:50
@tstadel tstadel requested review from vblagoje and removed request for a team September 20, 2022 17:50
@ZanSara ZanSara requested review from ZanSara and removed request for vblagoje September 21, 2022 07:45
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super cool! I'm really happy for this change 🤩 There is one "big" question we might want to address, the other are smaller notes. Thank you for taking care of this!

Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thank you 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BaseRetriever deepcopies all documents at indexing

2 participants