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

add other ollama visual llms #191

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

Conversation

ketsapiwiq
Copy link

What does this PR do?

Adds bakllava, llama3-llava, llava:13b, etc.

Requirement/Documentation

https://ollama.com/library?q=llava

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected. Make sure before submmiting this PR you run tests with evaluate.py

@joshbickett
Copy link
Contributor

@ketsapiwiq this looks like a great PR. Sorry I never merged it. I'm revisiting this project. I've been focused on other priorities.

If you want to resolve the conflicts and confirm you still want this merged in then test and merge in after.

@ketsapiwiq
Copy link
Author

ketsapiwiq commented Jan 23, 2025

Hi, yes I can rebase it but my rudimentary check if "llava" in model doesn't work anymore as now a lot of models do vision without them being called "llava", see: https://ollama.com/search?q=vision

Edit: maybe just pull llama3.2-vision by default as it's one of the best currently

@westoque
Copy link

chiming in this PR since i was about to do the same work.

i think it's better if we just let ollama be the interface for the function by explicitly removing references to llava so it's easier to use other models and not require a code change. for ex, i'm currently using llama3.2-vision and i don't need to have to put that into the if statement just to be able to run it.

@ketsapiwiq
Copy link
Author

Here, so by default it'll try with ollama :)

@joshbickett
Copy link
Contributor

@ketsapiwiq, great is the PR ready now? I'll take a look this week

@ketsapiwiq
Copy link
Author

Yes! Thank you :)

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.

3 participants