Add regex loading from tokenizer.json and code refinement #863
Conversation
wenbingl
commented
Dec 19, 2024
•
wenbingl
commented
- using regex pattern if it exist in tokenizer.json
- add tokenizer test case like AMD/OLMa through pp_api
| } | ||
|
|
||
| if (iter_type->get<std::string>() != "Sequence") { | ||
| return {kOrtxErrorNotImplemented, "Unsupported pretokenizer type!"}; |
There was a problem hiding this comment.
something in the test seems off, or maybe my understanding is wrong - i saw in test_pp_api.py you use the "amd/AMD-OLMo-1B-SFT-DPO" model, and if you look in the tokenizer.json for it, pre_tokenizer type is "ByteLevel", not "Sequence" (Note: you have support for "ByteLevel" for "pretokenizers" below, (which I guess you are expecting within the "pre_tokenizer") but not for "pre_tokenizer" itself); so it would fail here right? (but in the CI it is passing)
So - maybe we should add "ByteLevel" to the supported types for "pre_tokenizer" as well here, but also first identify why it is not failing the test currently, perhaps the type is not being extracted right or it is conflating "pretokenizers" and "pre_tokenizer".
| continue; | ||
| } | ||
|
|
||
| auto regex_str = iter_pattern->find("Regex"); |
There was a problem hiding this comment.
i am seeing some examples of lowercase "regex" in tokenizer.json as well - perhaps we make the case insensitive here?
|
Can we add an example to test the regex string option as well? HF "amd/AMD-OLMo-1B-SFT-DPO" has a "pre_tokenizer" value with type ""ByteLevel" and "use_regex": true, but maybe we can find one with a "regex" string that we can then use to demonstrate the custom regex functionality. |
