entropy-research / Devon

Devon: An open-source pair programmer
GNU Affero General Public License v3.0
1.91k stars 133 forks source link

Ollama Support: litellm dependency issue with uvicorn #33

Closed antoninoLorenzo closed 1 week ago

antoninoLorenzo commented 2 weeks ago

Context I am working to add Ollama support for Devon, so I started working on the prompts in my fork inside the ollama-support branch; at the moment of writing there is the OllamaModel class implemented as follows inside devon_agent/agents/model.py:

import os
import litellm
import logging
from litellm import completion
from dataclasses import dataclass
from typing import Optional

# ...
class OllamaModel:

    def __init__(self, args: ModelArguments):
        self.args = args
        self.api_model = args.model_name  # fixed
        self.model_metadata = {
            "max_tokens": 4096,
        }

        self.api_key = "ollama"

    def query(self, messages: list[dict[str, str]], system_message: str = "") -> str:
        model_completion = completion(
            messages=[{"role": "system", "content": system_message}] + messages,
            max_tokens=self.model_metadata["max_tokens"],
            model=self.api_model,
            temperature=self.args.temperature,
            stop=["</COMMAND>"],
            api_base="http://localhost:11434"
        )

        response = model_completion.choices[0].message.content.rstrip("</COMMAND>")
        return response + "</COMMAND>"

Issue To run litellm the following command should be runned:

litellm --model ollama_chat/model_name

It relies on litellm[proxy] library, to install it the following command should be runned:

poetry add litellm[proxy]

The problem is that it causes a dependency error with poetry, as follows:

$ poetry add litellm[proxy]

Updating dependencies

Because no versions of litellm match >1.37.16,<2.0.0
 and litellm[proxy] (1.37.16) depends on uvicorn (>=0.22.0,<0.23.0), litellm[proxy] (>=1.37.16,<2.0.0) requires uvicorn (>=0.22.0,<0.23.0).
So, because devon-agent depends on both uvicorn (^0.29.0) and litellm[proxy] (^1.37.16), version solving failed.

Possible Solution I think that the problem could be solved by removing litellm dependency in models.py and directly using ollama for python. If there are other solutions I would appreciate some feedback, however until a solution is found I am unable to work one ollama support.

cyborgmarina commented 2 weeks ago

Isn't ollama OpenAI compatible? Just out of curiosity, is litellm proxy really necessary here?

ollama/docs/openai.md at main · ollama/ollama · GitHub

Could you provide a link for a WIP Pull Request or point me to some branch you're working on? Maybe I can help!

Mihir1003 commented 2 weeks ago

Bumping down the version of uvicorn is fine, which should take care of the dependency conflict. I think having liteLLM could vert useful long term to add support for cloud providers like groq, together, etc

antoninoLorenzo commented 2 weeks ago

@cyborgmarina I was thinking exactly about not using litellm.

I am working on the ollama-support branch on my own fork given that I am not one of the repository main contributors, so I was implementing the prompts at devon_agent/agents/default/ollama_prompts.py; you could either go in my fork and see if you can solve it or fork this repository and do it by yourself. However personally I would wait for approval if you are considering removing litellm.

@Mihir1003 also got a point however, using litellm could be future-proof. I usually prefer having less dependencies when I can, but at this point it is a decision of the owners.

Mihir1003 commented 2 weeks ago

One thing we can do to reduce bloated dependencies in the future is use python package extras. We could support devon_agent[litellm], however for now we want to add support for groq and other providers in the short run.

Once dependencies start getting bloated, we will divide them into extras. Also yes let me install ollama in the main repo so you can fork from there. Thanks!

Mihir1003 commented 2 weeks ago

https://github.com/BerriAI/liteLLM-proxy LiteLLM proxy seems to be deprecated

akiradev0x commented 2 weeks ago

Ok so litellm proxy is deprecated, @antoninoLorenzo you should just be able to do the completion as normal with the api_base set to the ollama base url one as it is right now. For reference, here is the litellm page on ollama: https://docs.litellm.ai/docs/providers/ollama

Feel free follow up here if thats not working.

Longer term though, having litellm as a wrapper over different apis will make supporting new apis easier. More than willing to rip it out later as it shouldn't be too hard.

Mihir1003 commented 1 week ago

So i got ollama to work with litellm without the proxy. All I had to do was set completion (in LiteLLM) to this.

        model_completion = completion(
                messages=[{"role": "system", "content": system_message}] + messages,
                max_tokens=self.model_metadata["max_tokens"],
                model="ollama/phi3",
                temperature=self.args.temperature,
                stop=["</COMMAND>"],
                api_base="http://localhost:11434"
            )
Mihir1003 commented 1 week ago

@antoninoLorenzo hopefully that helps

cyborgmarina commented 1 week ago

how would we proceed with the cli? I was thinking it would be nice to be able to run devon configure <modelname> system_prompt.txt (optional) last_prompt(optional) and infers correct class from it, like ollama/phi3 so we could basically use any ollama or litellm supported provider such as openrouter.

antoninoLorenzo commented 1 week ago

@Mihir1003 @killind-dev Thanks for the support, I made the required changes to OllamaModel class and tested the system prompt within ollama_prompts.py; it seems to work correctly, however to ensure correct behaviour I will implement a test, for now I am going to put it in devon_agent/agents/tests, is it ok?

akiradev0x commented 1 week ago

Sounds good to me! @antoninoLorenzo

akiradev0x commented 1 week ago

@cyborgmarina yeah that seems correct

akiradev0x commented 1 week ago

Ollama support merged. closing