Skip to content
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

feat: Adding LiteLLM Router support to dspy.LM with RoutedLM #1611

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

zhaohan-dong
Copy link
Contributor

No description provided.

@zhaohan-dong
Copy link
Contributor Author

Should probably add a dummy for testing if it looks good.

@okhat
Copy link
Collaborator

okhat commented Oct 12, 2024

Hey @zhaohan-dong ! Thanks. This is awesome. Most of the code appears to be nearly an exact duplicate of dspy.LM, so it makes me wonder if there's a cleaner way to avoid duplication?

@zhaohan-dong
Copy link
Contributor Author

😅 If I'm allowed to modify the LM class, I'd say this part from Line 38 can be extracted to another function:

        if self.model_type == "chat":
            completion = cached_litellm_completion if cache else litellm_completion
        else:
            completion = cached_litellm_text_completion if cache else litellm_text_completion

Like completion = extracted_function()
So the RoutedLM class can override only this extracted function.

Separately I'm thinking if there's any benefit of moving the router completion methods to standalone functions. But then there might be issue with the function arguments for caching.

@zhaohan-dong
Copy link
Contributor Author

@okhat I feel like letting the litellm inference functions conditionally use routers would actually make it cleaner. It is possible since the litellm.Router can be hashed for lru_cache.

Let me know what you think!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants