-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix(pinecone_rm): refactored to use cloud_embed and fix pinecone init #1027
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
Conversation
| ERRORS, | ||
| max_time=15, | ||
| ) | ||
| def get_embeddings(self, queries: List[str]) -> List[List[float]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could the caching functionality included in this PR be used here?
I realize we probably need an abstracted Retriever class to handle this but for now, feel free to check if this suffices
| { | ||
| "data": { | ||
| "text/plain": [ | ||
| "[upserted_count: 100,\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe better to remove the output for this cell :)
| "name": "stdout", | ||
| "output_type": "stream", | ||
| "text": [ | ||
| "Bootstrapped 0 full traces after 20 examples in round 0.\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems like the bootstrapping in this case couldn't find any valid examples. I'm curious if it is because of the retriever and what passages are being retrieved - worth investigating if it is due to the changes in this PR. We typically expect this example to work with most LMs/RMs but it could just be these 20 were not valid outputs. Just worth checking IMO before merging this example if that is the case
|
Hi @csellis , thanks for the PR! left a few comments to be addressed before merging. Also could you run |
|
Hi @csellis , I updated the ruff fix but accidentally merged this. Could you reopen this with the updated changes as per the comments? Thanks. |
source is from https://github.com/stanfordnlp/dspy/pull/342/files