Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Colbert local mode support both as retriever and reranker. #797
Colbert local mode support both as retriever and reranker. #797
Changes from 17 commits
9632e5e
e415f39
a4b3844
321a768
6cd1d56
eeafacb
1639bd2
ec062b6
987d923
9ff5b28
825a272
c25e9c4
ab5b12e
63dd534
f6a9293
197a2c2
4698b00
81d142f
b73753c
0ec1ded
567d5c4
685df2a
fa2bc20
509b36c
34328fd
146ec7b
f0437e3
9cb522b
ec4b9b3
326ce01
b5913fc
c60fadc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
can we instead handle this through logging and through a Depecration message?
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.
@arnavsinghvi11, can you explain what you mean by logging?
Do I have to create a python logging object file and then log these? Sorry if this is a trivial question
I have added the deprecation warning for now
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.
dspy/dspy/evaluate/evaluate.py
Line 56 in d09d984
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.
The dspy logger object is not available in the dsp folder, hence I followed logging as done here for anthropic LM. Is there a better way to log this?
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.
logging here too
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.
as with above -
dspy/dspy/evaluate/evaluate.py
Line 56 in d09d984
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.
as with above-
The dspy logger object is not available in the dsp folder, hence I followed logging as done here for anthropic LM. Is there a better way to log this?
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 we have maintain the forward pass call from before and abstract the repetitive code from below within the forward pass?
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.
Sorry I was not able to understand this. @arnavsinghvi11
Do you want to have a common utility function for both
Retrieve
andRetrieveTheRerank
to process the returned documents?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.
Hi @Athe-kunal , yeah it seemed like there is some repetitive code in both forward passes that can be abstracted out for the different retriever types. let me know if this change makes sense
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.
Hi @arnavsinghvi11
I have abstracted the repetitive code part. However there are some nuances in the multi-query retriever, hence I didn't make a helper function for it. But for a single query, I have added a helper function
single_query_passage
. Please let me know if I need to make some other changes.