Remove legacy code from client/lm.py and reformat #1604
Conversation
dspy/clients/lm.py
Outdated
|
|
||
| class LM: | ||
| def __init__(self, model, model_type='chat', temperature=0.0, max_tokens=1000, cache=True, **kwargs): | ||
| def __init__(self, model, model_type="chat", temperature=0.0, max_tokens=1000, cache=True): |
There was a problem hiding this comment.
Thanks Chen! Why remove kwargs? The kwargs here are important. They're at the level of the LM object. The kwargs in__call__ are at the level of the individual call. Both are needed.
There was a problem hiding this comment.
ohh the kwargs is not really used in __init__ method, and the only place it is used is inside __call__ method, where we join it with kwargs of __call__ method. This is not ideal because we are not drawing an explicit boundary between these two kwargs, so users don't know which one they should use. We might want to explicitly list out model args like temperature and max_tokens for clarity.
There was a problem hiding this comment.
Thank you @chenmoneygithub ! It's important that users can set kwargs in __init__, which become defaults for __call__. The kwargs passed to __call__ overwrite any defaults from __init__. It was well-defined and commonly used. We should add it back IMO.
There was a problem hiding this comment.
sure thing! Let me add it back.
dspy/clients/lm.py
Outdated
| self.kwargs = dict(temperature=temperature, max_tokens=max_tokens, **kwargs) | ||
| self.temperature = temperature | ||
| self.max_tokens = max_tokens | ||
| self.kwargs = kwargs |
There was a problem hiding this comment.
This used to be:
self.kwargs = dict(temperature=temperature, max_tokens=max_tokens, **kwargs)
2 participants
As litellm becomes an official dependency of DSPy, we no longer need the hacky import handling code.
Also reformat the code for better readability.