Repository navigation
Conversation
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
|
[For maintainers] Suggested jobs to run (before merge) run-slow: auto |
|
CI Dashboard: View test results in Grafana |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
View the CircleCI Test Summary for this PR: https://huggingface.co/spaces/transformers-community/circle-ci-viz?pr=46622&sha=b11320 |
ArthurZucker
left a comment
There was a problem hiding this comment.
Thanks for the great description !
Let's just fix the CIs which are related pytest tests/models/lighton_ocr/test_processor_lighton_ocr.py::LightOnOcrProcessorTest::test_save_load_pretrained_additional_features seems to go through
src/transformers/tokenization_mistral_common.py:1513: in from_pretrained
tokenizer_path = download_tokenizer_from_hf_hub(
is weird as it has qwen2 tokenizer....
|
the reproducer works with / without mistral-common in #46091 |
|
should be good to close as we merged this instead #46667 |
Context
Some Mistral models are released with both a
tokenizersformat tokenizer (tokenizer.json) and amistral-commonformat tokenizer (tekken.json).If we examine
TOKENIZER_MAPPING_NAMESinsrc/transformers/models/auto/tokenization_auto.pywe see the following pattern repeated for all Mistral models:transformers/src/transformers/models/auto/tokenization_auto.py
Lines 204 to 209 in 08a7ef0
From these we can reasonably assume that if
mistral-commonis:AutoTokenizershould useTokenizersBackend(i.e.tokenizer.json)AutoTokenizershould useMistralCommonBackend(i.e.tekken.json)Therefore, for checkpoints which have both
tokenizer.jsonandtekken.json, the decision of which to use comes down to whethermistral-commonis installed.Next, if we look at:
transformers/src/transformers/models/auto/tokenization_auto.py
Lines 718 to 746 in 08a7ef0
We can see that if:
tokenizer_config.jsonexists and contains atokenizer_classfieldTOKENIZER_MAPPING_NAMEScontains an entry for this model typemistral-commonis:registered_class_nameis alwaysTokenizersBackendfor Mistral modelstokenizer_config_classThis last line is a bug and leads to incorrect tokenization.
Reproducer
Using the following file as a reproducer:
We see the following two outputs with and without
mistral-common:The case where
mistral-commonis installed resolves the incorrect class and produces incorrect detokenization.Solution
The solution is to treat
MistralCommonBackendmore similarly toTokenizersBackend. IfMistralCommonBackendis the registered class, try to use it and fall back toTokenizersBackendif there wasn't actually atekken.jsonfile.This behaviour:
TOKENIZER_MAPPING_NAMESLlamaTokenizerto load; which is the same behaviour as whenmistral-commonis not installedRunning the reproducer again with and without
mistral-commonwe see:which is the expected behaviour.