-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
WIP - dspy.RM/retrieve refactor #1739
base: main
Are you sure you want to change the base?
Conversation
dspy/retrieve/embedder.py
Outdated
from typing import Callable, List, Optional | ||
|
||
|
||
class Embedder: |
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.
#1735 will go here once merged @chenmoneygithub
def __init__(self, url: str = "http://0.0.0.0", port: Optional[Union[str, int]] = None, post_requests: bool = False): | ||
super().__init__(embedder=None) | ||
self.post_requests = post_requests | ||
self.url = f"{url}:{port}" if port else url |
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.
Is this port required? If so let's raise an ValueError for clear action.
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.
just an optional i believe. our examples typically use 'http://20.102.90.50:2017/wiki17_abstracts'
where the port can be specified in the URL
return res.json()["topk"][:k] | ||
|
||
|
||
@functools.cache |
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.
why do we need this _wrapped
function? seems it's not adding extra logic except from some cache, call we move the annotations to colbertv2_post_request_v2
?
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.
yea i think we can simplify it but will defer to @okhat as this was existing behavior from colbertv2.py
refactoring for cleaner retriever interface in DSPy