Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make check of document & embedding count optional in FAISS and Pinecone #2677

Merged
merged 2 commits into from
Jul 4, 2022

Conversation

julian-risch
Copy link
Member

@julian-risch julian-risch commented Jun 17, 2022

Proposed changes:

  • The _validate_index_sync() method in FAISS and Pinecone hinders users from initializing a document store that already contains some documents but no embeddings yet. This situation can happen though if a large number of documents is written to the document store in multiple separate sessions and only after all documents are written, the update_embeddings step is called. This PR makes running the validation optional by adding a parameter to the document store's init method that allows choosing whether to run it or not. Default is True.
  • Pinecone has a _validate_index_sync() but it was never called. This PR calls it as the final step of the document store's init method

Status (please check what you already did):

@julian-risch julian-risch changed the title make validation optional & add method call in pinecone init Make check of document & embedding count optional in FAISS and Pinecone Jun 17, 2022
@julian-risch julian-risch merged commit 1c1faa4 into master Jul 4, 2022
@julian-risch julian-risch deleted the validate-index-sync branch July 4, 2022 08:12
Krak91 pushed a commit to Krak91/haystack that referenced this pull request Jul 26, 2022
…ne (deepset-ai#2677)

* make validation optional & add method call in pinecone init

* Update Documentation & Code Style

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants