Add in-layer TF Tokenizer to BPE tokenizers
Author: piEsposito
Pull request: no
Labels: name: WIP

### Feature request

As what we have with `TFBertTokenizer`, but with models that use Byte Pair Encoding (e.g. `TFT5Tokenizer`, `TFClipTokenizer`) etc...

They were implemented in `keras-nlp` (https://github.com/keras-team/keras-nlp/pull/389) and we can now bring them here. 

### Motivation

With that feature we will be able to serve almost every model with TF Serving, which will make it much easier to serve models, as we won't have to write handlers and custom servers.

Having TF BPE Tokenizers is (I think) the last barrier to make `transformers` fully TF Serving-compliant.

### Your contribution

I can submit a PR, but there are a huge lot of models for which we would need to do that, so I expect a large number of subtasks if you decide to go for it.

Also, as `keras-nlp` implemented it (https://github.com/keras-team/keras-nlp/pull/389), should we copy-paste the code for each tokenizer or import from `keras-nlp`, while keeping the reference to their repo?

Comments:
It seems like I have to tag @n1t0, @LysandreJik because this is about the tokenizers. 

cc @Rocketknight1 

An alternative would be adding a `as_keras_layer` (or something) method to `PreTrainedTokenizer` and create the TF BPE Tokenizer from the vocab and merges from the tokenizer. What do you think?

This is great! We'd been blocked by the lack of a BPE tokenizer in TF-text or Keras-NLP, as it's an extremely common tokenizer class for us. We're definitely going to explore this as soon as we can find some time.

@Rocketknight1 I can contribute and submit a PR, I would just like some guidance of your on where to implement such Tokenizer (and whether should we import it from Keras-NLP or copy-paste + adapt it). 

As they haven't released version of the package in a while, maybe creating a base BPE Tokenizer on a `tf_tokenization_utils` file and then having the specifics of `prepare_for_tokenization` be implemented for each model. 

What do you think? I would just like to know if you folks are OK with some copy pasted code from `keras-nlp`.


Hi @piEsposito - good questions all round. Right now we only have one TF tokenizer in the library, the BERT tokenizer in `tf_tokenization_bert.py`. I think a good plan would be to add a single BPE tokenizer for a couple of popular models that use BPE (e.g. RoBERTa, GPT or DeBERTa). After that, we should be able to see how much code is shared and how much is model-specific and then refactor out a shared method for future tokenizers. WDYT?

Also, code copy-pasted from `keras-nlp` is fine as long as the licence allows it, but we also don't mind having `keras-nlp` as a dependency, since in-graph tokenizers will already have `tensorflow-text` as a dependency anyway.

@Rocketknight1 I will try doing it for T5 or GPT.

About having `keras-nlp` as a dependency: I've opened an issue there https://github.com/keras-team/keras-nlp/issues/442 asking for them to release to Pypi a version with BPE Tokenizer, and in the meantime will try to implement it in a way that works with T5, then copy paste only if this is needed.

How does that sound?

I should have something in that sense on the next week if you approve the idea.

Sounds perfect to me!

Hi! This is Chen from KerasNLP, we can release a branch specially for the BPE if you need it soon, but before that there is a concern I want to raise:

I made the TF BPE with many regex hacks because tf_text uses Google re2, which does not fully match the python re. Although I tested on multiple datasets (multiple languages as well) and it worked well, I am still not 100% confident it provides the exactly same result as openAI BPE. So please make sure you have a good testing coverage before using it in production, thanks!

Thanks @chenmoneygithub! We have quite a lot of models with BPE tokenizers that we could probably test against.

Awesome! We will go ahead and make a release for BPE tokenizer then. Will update this thread when that is finished.

We have made a release containing the BPE tokenizer: https://pypi.org/project/keras-nlp/

Please let us know if you find any issues, thank you!

cc @piEsposito to the above comment! ^

@Rocketknight1 thank you let me get started!

Yeah I'm exploring it and guess what it is not as easy as I thought haha. 

@piEsposito That was my experience too - are you having trouble even getting the results to match for a single model?

@Rocketknight1 I'm having it too, which is kinda fun, because the tokens are total mismatches, but when I decode them back they are still the same as the input. I think we will have to go deep on the internals to check for the differences. 

Ugh, of course - there are multiple valid tokenizations for the same string. I'm not enough of a tokenizers expert to know the exact algorithms used and if they differ between the many BPE models we have.

@Rocketknight1 I could make them match for GPT2, should open a PR this week. Sorry for the delay, this thing was an order of magnitude harder to do that I was estimating.

Don't apologize at all - this is something we were struggling with too!

Thanks for understanding, I'll try to get at least a draft today. 

@Rocketknight1 after some delay I could figure out a way to make it work, even with generation. I've requested your review and, as we agreed, kept the implementation minimal for us to get a sense of the effort needed to create the in-layer TF Tokenizer for the models that use BPE.

@Rocketknight1 all right, that was fun. Let's do it for CLIP now and figure out how we put the `<w/>` logic inside the keras-nlp bpe tokenizer.

Awesome, good luck!

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.

Hey don't stale us buddy. We're just putting out some fires and enjoying the holidays. 

What is the current status of this? I saw the GPT2 Tokeniser was added - any progress on any others? I am happy to contribute, though will need help wth some of the details.

Hi @calvingiles, we're not working on others internally but we'd be happy for you to try adding one! Basically, if you want to attempt this, I'd suggest picking a model, then seeing if you can replicate its tokenizer with a Tensorflow Text tokenizer class like `BytePairTokenizer`. If you can, then it's quite easy - take a look at the `tokenization_gpt2_tf.py` file in `transformers`. Most of that can be copied for a new model you want to add. There are probably several models that use standard BytePair tokenizers where that file could be copied with barely any changes to enable TF tokenization for them - we're happy to support you if you want to try looking around and making PRs for them!