Skip to content

Conversation

@vriesdemichael
Copy link
Contributor

This fixes one of the problems in #5040

The add_bos_token and add_eos_token are config params to determine whether you should add special tokens to the sequence automatically: https://huggingface.co/docs/transformers/en/model_doc/llama2#transformers.LlamaTokenizer.add_bos_token describes the params.

Regardless of add_bos_token and add_eos_token the special tokens should be added in the metadata, as they can still be used in the chat_template.

@slaren
Copy link
Member

slaren commented Feb 14, 2024

Should the list of special_token_types also be updated to include all the token types listed in #5040 (comment)? Otherwise only these will be exported:

https://github.com/ggerganov/llama.cpp/blob/f428652b2ab852fe84f39d6877a8f38d63fa4df7/gguf-py/gguf/vocab.py#L29-L32

@vriesdemichael
Copy link
Contributor Author

@vriesdemichael vriesdemichael force-pushed the fix/5040-eos-bos-token-missing-in-gguf branch 2 times, most recently from b656423 to 10f9651 Compare February 15, 2024 09:12
@ggerganov ggerganov requested a review from slaren February 15, 2024 09:14
@ggerganov
Copy link
Member

Lint check needs fix too

@vriesdemichael vriesdemichael force-pushed the fix/5040-eos-bos-token-missing-in-gguf branch from 10f9651 to 3b5dc11 Compare February 15, 2024 09:36
@slaren slaren merged commit 7312247 into ggml-org:master Feb 15, 2024
jordankanter pushed a commit to jordankanter/llama.cpp that referenced this pull request Mar 13, 2024
…oken is set to false (ggml-org#5487)

* fix(gguf-py): special tokens are no longer skipped when add_<token>_token is set to false

* fix(gguf-py): added missing cls and mask token ids to the gguf metadata
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 1, 2024
…oken is set to false (ggml-org#5487)

* fix(gguf-py): special tokens are no longer skipped when add_<token>_token is set to false

* fix(gguf-py): added missing cls and mask token ids to the gguf metadata
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