Skip to content

Faiss integration #351

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

Merged
merged 6 commits into from
Jun 11, 2024
Merged

Faiss integration #351

merged 6 commits into from
Jun 11, 2024

Conversation

VinciGit00
Copy link
Collaborator

No description provided.

@VinciGit00 VinciGit00 requested a review from PeriniM June 6, 2024 19:42
@@ -98,7 +99,19 @@ def execute(self, state: dict) -> dict:
)
embeddings = self.embedder_model

retriever = FAISS.from_documents(chunked_docs, embeddings).as_retriever()
folder_name = "cache"
Copy link
Collaborator

@DiTo97 DiTo97 Jun 6, 2024

Choose a reason for hiding this comment

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

that's the only thing I'd change.

as it stands, each RAG node with the cache config would overwrite the very same folder, effectively nullifying the cache, unless a RAG node on the very same input state is invoked subsequently.

I would allow for more flexibility by either 1): allowing the user to specify the cache folder in the config for that specific RAG node or 2) automatically compute a unique identifier for that specific input state (e.g., the filename, the endpoint or a some hashing function on the documents) so that the user may choose what to keep and what not to keep in the cache

@PeriniM PeriniM changed the base branch from pre/beta to dev June 11, 2024 21:15
@PeriniM PeriniM merged commit 589da1d into dev Jun 11, 2024
0 of 2 checks passed
@PeriniM PeriniM deleted the faiss_integration branch June 11, 2024 21:15
Copy link

🎉 This PR is included in version 1.7.0-beta.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants