Adding LiteLLM support #78
Conversation
simonguozirui
left a comment
simonguozirui
left a comment
There was a problem hiding this comment.
Looks great! let's think about a few details re: local serving and default settings, and make sure the E2E pipeline still works. So much cleaner, great work @pythonomar22 and then we can merge it
another thing to do in the future is to leverage the litellm / openai batch call API, but that is for a future PR.
src/utils.py
Outdated
| return outputs | ||
| messages = prompt | ||
|
|
||
| if system_prompt and (not messages or messages[0].get("role") != "system"): |
There was a problem hiding this comment.
should we put this ahead before building the message list with the prompt
There was a problem hiding this comment.
yeah, updated to check if the prompt already contains a system message before building the messages list.
| self.max_tokens = 4096 | ||
| self.temperature = 0.0 | ||
| self.server_type = None | ||
| self.model_name = None |
There was a problem hiding this comment.
so are we not specifying a default, this could also works, and just expect them to use a preset here?
There was a problem hiding this comment.
updated to reflect slack thread - but yeah
|
Thanks for cleaning this up @pythonomar22 and we finally have a clean llm interface rather than the hacky per provider client condition statements that I wrote. |
* adding litellm support for all except sambanova * adding reasoning config support * addressing some comments * fixing modal litellm * setting defaults * litellm lgtm --------- Co-authored-by: Omar Abul-Hassan <omarah@matx1.stanford.edu> Co-authored-by: Simon Guo <simonguo@stanford.edu>
Added LiteLLM support. Tested locally and passes. Have not added support for Sambanova.