[AutoTokenizer] Allow creation of tokenizers by tokenizer type #13668
Conversation
patrickvonplaten
requested review from
LysandreJik,
sgugger and
SaulLu
and removed request for
LysandreJik
There was a problem hiding this comment.
Thanks a lot for the addition @patrickvonplaten 🙌
I just left 2 small questions in comments
| # If we have the tokenizer_type we can leverage it | ||
| if tokenizer_type is not None: | ||
| tokenizer_class = None | ||
| tokenizer_class_tuple = TOKENIZER_MAPPING_NAMES.get(tokenizer_type, None) | ||
|
|
||
| if tokenizer_class_tuple is None: | ||
| raise ValueError( | ||
| f"Passed `tokenizer_type` {tokenizer_type} does not exist. `tokenizer_type` should be one of " | ||
| f"{', '.join(c for c in TOKENIZER_MAPPING_NAMES.keys())}." | ||
| ) | ||
|
|
||
| tokenizer_class_name, tokenizer_fast_class_name = tokenizer_class_tuple | ||
|
|
||
| if use_fast and tokenizer_fast_class_name is not None: | ||
| tokenizer_class = tokenizer_class_from_name(tokenizer_fast_class_name) | ||
|
|
||
| if tokenizer_class is None: | ||
| tokenizer_class = tokenizer_class_from_name(tokenizer_class_name) | ||
|
|
||
| if tokenizer_class is None: | ||
| raise ValueError(f"Tokenizer class {tokenizer_class_name} is not currently imported.") | ||
|
|
||
| return tokenizer_class.from_pretrained(pretrained_model_name_or_path, *inputs, **kwargs) |
There was a problem hiding this comment.
I was wondering if all these added lines could not be put before the line 432. It seems to me that tokenizer_config and config_tokenizer_class are not used in this if and it would "save" loading the tokenizer_config.
There was a problem hiding this comment.
Agree! Changed it
| tokenizer_class = tokenizer_class_from_name(tokenizer_class_name) | ||
|
|
||
| if tokenizer_class is None: | ||
| raise ValueError(f"Tokenizer class {tokenizer_class_name} is not currently imported.") |
There was a problem hiding this comment.
nit: I'm not sure I figured out why this error message isn't exactly (about the existence of the tokenizer) like the one on the line 478.
There was a problem hiding this comment.
The tokenizer_class_name has to exist as it's mapped from the tokenizer_type so the only reason this could fail is if the class cannot be important due to some missing packages like tokenizers or sentencepiece
What does this PR do?
This PR enables the
Case #4as discussed here: #13623 (comment)Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.