-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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: Add support for Mistral API models #2053
Conversation
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
Signed-off-by: Olyxz16 <cedric.sazos@tutanota.com>
Signed-off-by: Olyxz16 <cedric.sazos@tutanota.com>
@cebtenzzre look good now? |
Signed-off-by: Olyxz16 <cedric.sazos@tutanota.com>
should be good, please review |
Signed-off-by: Olyxz16 <cedric.sazos@tutanota.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has conflicts and needs that need to be resolved and then we'll review again.
In general, I'm really happy to have someone working on updating these remote Models and consolidating the code for them / keeping them up2date as the remote API's change. Would be cool to have Claude support as well! |
Signed-off-by: Olyxz16 <cedric.sazos@tutanota.com> # Conflicts: # gpt4all-chat/chatapi.cpp # gpt4all-chat/modellist.cpp
it's merged ! |
I plan on changing the way we handle them and use the same system as the other models |
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to figure out the naming convention we want and then change accordingly, but otherwise I'm happy with this. How about you @cebtenzzre ?
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
So what should I do next ? |
Going to have @cebtenzzre look at this tomorrow but after we come up with a solution for the prefix file names and we figure out the issues below, then we're hopefully merging this week |
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
NOTE: This very closely related PR that I'm closing due to conflicts... but putting it here for posterity: #1692 |
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
I do agree with @cebtenzzre about the naming convention of the remote model files. But the issue remains : where do we first store the name of the remote model, since it's different from the display name and the filename ? |
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
I fixed the backward compatibility issue by checking for remaining .txt file when loading the models. I then transform them into the new .rmodel format. The process then continues like before |
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com>
…x-1991 Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com> # Conflicts: # gpt4all-chat/chatapi.cpp # gpt4all-chat/chatllm.cpp
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
@Olyxz16 Heh sorry, manyoso suggested that I make the style changes for you but apparently you were already working on it, so we duplicated some work. Looks like it came out OK in the merge. |
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
Yeah I saw your commit after my changes, no worries you changed more things than I did anyway |
looks like all we're missing is another conflict resolution and then this is ready to merge? |
Signed-off-by: Cédric Sazos <cedric.sazos@tutanota.com> # Conflicts: # gpt4all-backend/llama.cpp-mainline # gpt4all-chat/chatllm.h # gpt4all-chat/modellist.cpp
Just merged, there was conflicts on the backend I didn't know how to solve, so I just chose the branch responsible for the changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hold on, the merge isn't quite right.
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
I'm still seeing, "This branch cannot be rebased due to conflicts" |
Probably because rebasing merges isn't trivial. I just squashed it to keep things clean - one can click the PR link to see the history of this specific change. |
we need a check for the prefix of the old "chatgpt-" when converting the .rmodel and probably 'nomic-' too? |
It looks like Nomic Embed is not even converted and still uses the old name. |
See #2119 I'm leaving the nomic one now as I think there is more that needs to be changed and I'll want to test to make sure it works |
Describe your changes
Added models mistral-tiny, mistral-small and mistral-medium from Mistral API.
Issue ticket number and link
Fixes #1991
Checklist before requesting a review
Demo
Notes
Mistral API : https://docs.mistral.ai/api/