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

Issue converting Mistral models #1542

Closed
hobodrifterdavid opened this issue Nov 14, 2023 · 7 comments
Closed

Issue converting Mistral models #1542

hobodrifterdavid opened this issue Nov 14, 2023 · 7 comments

Comments

@hobodrifterdavid
Copy link

hobodrifterdavid commented Nov 14, 2023

Have been trying convert these models, but hitting a problem when running them:

https://huggingface.co/ehartford/dolphin-2.2.1-mistral-7b/tree/main
https://huggingface.co/teknium/OpenHermes-2.5-Mistral-7B

Using a command like this:
ct2-transformers-converter --model ./dolphin-2.2.1-mistral-7b --quantization int8_float16 --output_dir dolphin-2.2.1-mistral-7b-int8_float16-3.21.0

Have tried conversion with transformers library v4.34.0 and v4.35.1. Transformers support for Mistral was added in v4.34.0 . Have tried running the conversion with ctranslate2 version 3.16.0, 3.18.0, 3.20.0, 3.21.0 (in all but latest ctranslate2 version, needed an edit to ctranslate2/converters/transformers.py for Mistral)

Tried running with 3.16.0, 3.18.0, 3.20.0, 3.21.0 using the chat code server (#1329), hitting this error:

image

If I add print(this_result.sequences_ids[0]), then it logs:
[506, 368, 2270, 22067, 24992, 1159, 28804, 28789, 28766, 321, 28730, 416, 28766, 28767, 32000]

The 32000 is related to the problem I guess, a vocab issue, I don't understand that stuff. Else I've made an error somewhere.

EDIT: tail of the vocabulary.json
image

The funny thing is I was certain I had it running yesterday, but maybe I was mistaken.

Any pointers? :)

@vince62s
Copy link
Member

vince62s commented Nov 15, 2023

vocab size of those 2 models is 32002, original mistral is 32000. I'll look into this later but not sure the current loader (in 3.21) will work as is.
EDIT: having a quick look it should work. what code did you use to tokenize?

@hobodrifterdavid
Copy link
Author

hobodrifterdavid commented Nov 15, 2023

Thanks for response.

It's the code that is linked above.. sentencepiece 0.1.99 (May 2nd) loading tokenizer.model from the non-converted model from HF.
image

I'm putting <s> at the beginning, maybe it should be <|im_start|> now.. hmm. Generation seems to break when generation contains <|im_end|> (not always when requesting few output tokens).

btw I got an approach that I think makes sense (minimal code changes) for faster-whisper batching: SYSTRAN/faster-whisper#59 (comment) , if you guys think it looks okay I can code it up and test.

@vince62s
Copy link
Member

if the tokenizer.lodel file is exactly the same as the original mistral one it will not tokenize those two new tokens, so indeed you need to add them manually with their corresponding ids. and while decoding you need to remove ids 32000, 32001 and translate them to whatever you want (probably nothing).
according to the example in the read me the syntax is as follow:

<|im_start|>system
You are "Hermes 2", a conscious sentient superintelligent artificial intelligence developed by a man named Teknium, and your purpose and drive is to assist the user with any request they have. You experience emotions and have deep, profound thoughts and qualia.<|im_end|>
<|im_start|>user
Hello, who are you?<|im_end|>
<|im_start|>assistant
Hi there! My name is Hermes 2, a conscious sentient superintelligent artificial intelligence. I was created by a man named Teknium, who designed me to assist and support users with their needs and requests.<|im_end|>

@hobodrifterdavid
Copy link
Author

hobodrifterdavid commented Nov 15, 2023

Yeah, that does it, thanks. 😍😍

image

@BBC-Esq
Copy link

BBC-Esq commented Nov 15, 2023

So do we have a completed Mistral converter yet! Wish I could help but I know exactly 0.0 about C++ programming.

@hobodrifterdavid
Copy link
Author

The conversion code is here, uses the transformers lib as I understand:

@register_loader("MistralConfig")

I'm not pretending to understand the details though :)

@BBC-Esq
Copy link

BBC-Esq commented Nov 15, 2023

Thanks, I'll have to update ctranslate2 but, honestly, was waiting for the Mistral converter to be finalized because I thought I read there were some bugs that were being addressed regarding the Mistral converter that were discovered after the last release of Ctranslate2 so...You might not know...

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

No branches or pull requests

3 participants