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

[Enhancement] Allow an override of which model is used by Ollama #6

Open
dansiegel opened this issue Nov 2, 2024 · 4 comments · May be fixed by #8
Open

[Enhancement] Allow an override of which model is used by Ollama #6

dansiegel opened this issue Nov 2, 2024 · 4 comments · May be fixed by #8

Comments

@dansiegel
Copy link

Description

Currently the Ollama configuration is setup to always use the llama3 model. The problem with this is that new models are coming out all the time, for instance llama 3.2 is currently available and there are newer models which may provide better results for software development such as deepseek-coder-v2 or codegemma.

public static string AiModel = "llama3";

builder.Services.Add(new ServiceDescriptor(typeof(IOllamaApiClient), new OllamaApiClient(connectionString, Constants.AiModel)));

Proposal

Add an extension point so that when there is an Ollama ConnectionString we check for a Model Name like Ollama_Model with a backup that could still be provided by the constants file.

@QuantumNightmare
Copy link
Contributor

Thanks for posting this - we have also had this in mind and would like to support this extensibility. Somewhat related - we donated the Ollama component we'd built for this to the Aspire Community Toolkit. At the very least, we'd want this extensibility to be compatible with that. Ideally, you set up the Ollama component and specify what model you want it to be loaded with (there can be multiple, but there is this idea of having a single "Selected" model), and then this Raygun component will automatically use that. I have heard mention of the selected model name going to be added to the connection string coming from that Ollama component, but looks like it hasn't been done just yet. Let me know any other thoughts you have on this in the meantime.

@QuantumNightmare
Copy link
Contributor

Actually, looks like this has been done on the Ollama component side - but rather than being in the connection string of the main resource, you can use the resource created via the AddModel method which includes the Model in the connection string. https://github.com/CommunityToolkit/Aspire/pull/143/files#diff-347e09e74adc4f343e7c851220afb064dadd3ee73f0d38f8d42a40b40c3b2f08R22

We'll just need to look at adding support for this. Let us know if you think that'll suit what you're looking for here.

@dansiegel
Copy link
Author

dansiegel commented Nov 4, 2024

So I would say a few things on this:

  1. I do agree specifying it in the connection string is better.
  2. Whatever the Raygun host is using for the Ollama Client should be treated as a fallback in the event that the connection string does not have a model specified.
  3. That fallback model that is used here should still be configurable within this Aspire component.
  4. If a model is explicitly specified here, it should override any model in the connection string.

@QuantumNightmare
Copy link
Contributor

Thanks. I don't agree with your 4th point at this stage, but can discuss further.
I think that it's useful to tell the Raygun host component what model to use just to support the case that you've decided not to use an Ollama component that supports announcing a model. I think it's quite clear during setup that if you initialize the Ollama component and then add a particular model (maybe multiple) and then pass one of those models to the Raygun component, you're wanting it to use that model. I think any model you can configure on the Raygun component should be clear that it's a "default" or "fallback". Perhaps there could be a second "ModelOverride" property if we really wanted a way to define a model that overrides what's in the connection string. I probably wouldn't introduce this as part of this work initially though.

@dansiegel dansiegel linked a pull request Nov 18, 2024 that will close this issue
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 a pull request may close this issue.

2 participants