Shared tokenizer interface
Author: CatalinVoss
Pull request: yes
Labels: name: wontfix

Up to this point, `tokenize()` and `encode()` mean different places. In GPT-land, `tokenize` doesn't get us all the way to token IDs. Idteally, he tokenizers would share a common interface so that they can be plugged in and out of places just like the models.

I don't know if you want breaking changes, so I just created `encode()` and `decode()` as aliases on tokenizers that did not adhere to that spec already.

There is more cleanup to do. Since it seems that non-BERT models were added on later on, the BERT files should probably be renamed into `tokenizer_bert`, etc., but I left that in place to maintain compatibility. BERT is still missing `decode`.

Please advise and I can clean it up.

Comments:
Hi Catalin, thanks for that!
Yes backward compatibility is an important things to keep in mind here.
I think the changes look nice.
We should also:
- document them in the readme, and
- add associated tests in the relevant test files.

Just before this gets merged - I've noticed that the GPT(1) tokenizer ignores whether or not a string contains special tokens, and hence doesn't encode them properly.  
We've got around this by splitting `text` on whitespace inside the `tokenize` method and iterating, checking if a word is a special token or not, like so:
```
def tokenize(self, text):
        split_tokens = []
        for t in text.split(' '):
            if t in self.special_tokens:
                split_tokens.extend([t])
            else:
                #current tokenization stuff
```
Would be nice if this could be included :)

@andrewPoulton we can include this, but only for the whitespace tokenizer fallback of GPT-1. The original tokenizer (SpaCy) would split tokens like `<\w>` in pieces which is the reason it was not included originally.

Overall I must say I'm not a huge fan of the not-split-specific-tokens feature (the `never_split` option). We've added it due to popular request but it is very dependent on the underlying behavior of the tokenizer and the character content of special tokens (does it contains spaces, dashes...) and from the issues it looks like a common source of bugs and un-intended behavior (see #410 for a latest example).

Hi @CatalinVoss, from the state of the PR I understand you are still working on it.
Maybe ping me when you think it's ready for merging in master and I'll have a look again?

Hey @thomwolf yeah, just didn't get around to it yet, but then I needed the BERT decoding piece yesterday so I merged it in. It's imperfect. If we renamed everything to be consistent with words, tokens, token IDs, etc. we would have to change the method names, per my comment above. Do you want to do that? Otherwise perhaps better to do that in a separate PR and target some sort of v0.7 branch?

Ok we still have to add the docstrings, test and details in the readme for these methods.
Haven't find time this week. I will see if I can find time next week. 

@CatalinVoss I also was wondering about this while writing some extra functionality for the GPT2 tokenizer to encode new strings for finetuning tasks, but ended up writing a `special_token_to_id` functionality for getting IDs of special tokens -- the pattern of usage of the GPT1 tokenizer for finetuning tasks seems to be to add the special token IDs after encoding the rest of the string to process.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.


Hey @thomwolf, what was the decision on this? I can revisit if you want. We still wanted this in our fork… thanks much!

I never really had time to add tests and documentation to the PR but it's a good idea.
Let's add this feature in the new release.

Looks like this was taken care of with your refactor in `tokenization_utils.py`. Very nice!!