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

Adding in a rockset retriever class #1146

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

danpechi
Copy link

@danpechi danpechi commented Jun 13, 2024

  • the input can be finicky; it works with something like: str(openai_client.embeddings.create(model="text-embedding-ada-002", input=[question]).data[0].embedding)

  • this has some inflexibility wrt underlying schema, ie all results are coming from a singular vector db collection

  • note: rockset doesn't have a 'retrieve' function, it just leverages cosine_sim

  • this is mostly meant for singular questions, and not batch inference

self._rockset_collection_source_key]].values
passages.extend(data)

# Return type not changed, needs to be a Prediction object. But other code will break if we change it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment to be addressed? This can be a dspy.Prediction(passages=...).

@arnavsinghvi11
Copy link
Collaborator

Thanks @danpechi for the PR!

Couple of changes needed to merge:

  1. To support the import message here, can you add rockset as a dependency to the pyproject.toml? Feel free to follow how it's done for the other retrievers and then add rockset as a supported RM in the README (under Installation).

  2. Please run ruff check . --fix-only and push to pass the failing ruff test.

It would also be great if you could add documentation for the Rockset RM to provide context as done for the RMs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants