-
Notifications
You must be signed in to change notification settings - Fork 399
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
Fix missing spaces in Tokenizer.convert_token_to_string
#280
Fix missing spaces in Tokenizer.convert_token_to_string
#280
Conversation
e26bff1
to
7c184ba
Compare
7340b66
to
f60a998
Compare
f60a998
to
6fc9fa4
Compare
This probably isn't a general enough fix, but it should be a workable stand-in for Llama models right now, so I'm going to merge this. @AL-377, still feel free to put in a PR for another approach, especially if you think it will cover more cases. |
OK,thanks for your effort!I have tried the codellama model and the result remains the white space,I will keep trying |
Which HF tokenizer class is it using? |
transformers.models.code_llama.tokenization_code_llama_fast.CodeLlamaTokenizerFast |
Thanks! We just need to add |
But actually I have tried the code: self.is_sentencepiece = isinstance(
self.tokenizer, (LlamaTokenizerFast, LlamaTokenizer,CodeLlamaTokenizer,CodeLlamaTokenizerFast)
)
My test.py is just the same as the example at https://github.com/outlines-dev/outlines#efficient-json-generation-following-a-pydantic-model |
Yeah, that looks like a different issue with the Llama models altogether. I'll check it out. |
That issue is due to tokens that correspond to empty strings (see #273 (comment)). The general issue of empty strings and our regex sampling logic might go beyond this Llama-specific tokenization issue, though. I address it directly in #272, which replaces and simplifies all the regex logic. |
Closes #273