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

Fix converter for internlm2 #8321

Merged
merged 3 commits into from
Jul 10, 2024
Merged

Conversation

RunningLeon
Copy link
Contributor

fix internlm2 gguf output eos token at the end of each conversation
output example:

image

@RunningLeon RunningLeon changed the title [WIP]: update internlm2 [WIP]: Fix converter for internlm2 Jul 5, 2024
@github-actions github-actions bot added the python python script changes label Jul 5, 2024
@RunningLeon RunningLeon changed the title [WIP]: Fix converter for internlm2 Fix converter for internlm2 Jul 5, 2024
Comment on lines +2148 to +2150
# take care of ununsed raw token
if piece.startswith('[UNUSED'):
toktype = SentencePieceTokenTypes.UNKNOWN
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, does it work without this if the checks for UNKNOWN below are replaced with checks for UNUSED?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, these raw tokens are in fact used and updated by added_tokens_decoder. And there are asserations in line 2180 and 2199 which is aligned with what's done in phi3. The tokens are all read as SentencePieceTokenTypes.NORMAL and the assertations would fail if remove or change to UNUSED type.

@ggerganov ggerganov merged commit e4dd31f into ggerganov:master Jul 10, 2024
9 checks passed
compilade added a commit that referenced this pull request Jul 10, 2024
This makes the changes from #8321 more consistent
with the other changes made here.
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 13, 2024
* update internlm2

* remove unused file

* fix lint
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 13, 2024
* update internlm2

* remove unused file

* fix lint
compilade added a commit that referenced this pull request Jul 14, 2024
* llama : fix mpt and olmo pre-tokenizer

* llama : pre-tokenize non-special user-defined tokens first

* llama : fix detection of control-like user-defined tokens

* convert_hf : identify which user-defined tokens are control tokens

Only used in _set_vocab_gpt2() for now.

* convert_hf : identify more added control tokens for SPM tokenziers

This makes Gemma and Gemma-2 tokenize pretty much EVERYTHING correctly,
including HTML tags and consecutive spaces,
but it unfortunately requires model re-conversion.

There seems to be a weird behavior of the HF tokenizer for Gemma,
which prefers to use the 16-space token over more lengthy space tokens,
while using the SentencePiece tokenizer does not do this.
(the implementation in llama.cpp has the same behavior as SentencePiece)

* llama : fix wrong pre-tokenization of byte tokens

* llama : fix Viking pre-tokenizer regex

The order was previously wrong, which caused errors in some tests.

* llama : fix command-r detokenization

* convert_hf : reduce usages of the UNKNOWN token type

* llama : add UNKNOWN tokens in the special tokens cache

* convert_hf : reduce usages of UNKNOWN for InternLM2

This makes the changes from #8321 more consistent
with the other changes made here.

* test-tokenizer-random : reduce potential confilcts with #8379

* test-tokenizer-random : add a failing edge case for falcon
Nexesenex pushed a commit to Nexesenex/croco.cpp that referenced this pull request Jul 15, 2024
)

* llama : fix mpt and olmo pre-tokenizer

* llama : pre-tokenize non-special user-defined tokens first

* llama : fix detection of control-like user-defined tokens

* convert_hf : identify which user-defined tokens are control tokens

Only used in _set_vocab_gpt2() for now.

* convert_hf : identify more added control tokens for SPM tokenziers

This makes Gemma and Gemma-2 tokenize pretty much EVERYTHING correctly,
including HTML tags and consecutive spaces,
but it unfortunately requires model re-conversion.

There seems to be a weird behavior of the HF tokenizer for Gemma,
which prefers to use the 16-space token over more lengthy space tokens,
while using the SentencePiece tokenizer does not do this.
(the implementation in llama.cpp has the same behavior as SentencePiece)

* llama : fix wrong pre-tokenization of byte tokens

* llama : fix Viking pre-tokenizer regex

The order was previously wrong, which caused errors in some tests.

* llama : fix command-r detokenization

* convert_hf : reduce usages of the UNKNOWN token type

* llama : add UNKNOWN tokens in the special tokens cache

* convert_hf : reduce usages of UNKNOWN for InternLM2

This makes the changes from ggerganov#8321 more consistent
with the other changes made here.

* test-tokenizer-random : reduce potential confilcts with ggerganov#8379

* test-tokenizer-random : add a failing edge case for falcon
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 27, 2024
)

* llama : fix mpt and olmo pre-tokenizer

* llama : pre-tokenize non-special user-defined tokens first

* llama : fix detection of control-like user-defined tokens

* convert_hf : identify which user-defined tokens are control tokens

Only used in _set_vocab_gpt2() for now.

* convert_hf : identify more added control tokens for SPM tokenziers

This makes Gemma and Gemma-2 tokenize pretty much EVERYTHING correctly,
including HTML tags and consecutive spaces,
but it unfortunately requires model re-conversion.

There seems to be a weird behavior of the HF tokenizer for Gemma,
which prefers to use the 16-space token over more lengthy space tokens,
while using the SentencePiece tokenizer does not do this.
(the implementation in llama.cpp has the same behavior as SentencePiece)

* llama : fix wrong pre-tokenization of byte tokens

* llama : fix Viking pre-tokenizer regex

The order was previously wrong, which caused errors in some tests.

* llama : fix command-r detokenization

* convert_hf : reduce usages of the UNKNOWN token type

* llama : add UNKNOWN tokens in the special tokens cache

* convert_hf : reduce usages of UNKNOWN for InternLM2

This makes the changes from ggerganov#8321 more consistent
with the other changes made here.

* test-tokenizer-random : reduce potential confilcts with ggerganov#8379

* test-tokenizer-random : add a failing edge case for falcon
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python python script changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants