Add BARTpho: Pre-trained Sequence-to-Sequence Models for Vietnamese

[datquocnguyen]

What does this PR do?

Before submitting

• This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue or the forum? Please add a link
  to it if that's the case.
• Did you make sure to update the documentation with your changes? Here are the
  documentation guidelines, and
  here are tips on formatting docstrings.
• Did you write any new necessary tests?

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.

Please help have a look: @patrickvonplaten, @patil-suraj Thanks.

[patil-suraj]

Thanks a lot for adding this, great to have BARTpho integrated into Transformers!

I have left a few comments below, let me know if something is not clear, happy to help :)

Also AFAIK it's not possible to add a fast version of this type of tokenizer where sentencepiece is used only for tokenization and a different vocab file is used to id to token and token to id conversion. But @SaulLu , @n1t0 would know more :)
(cc @LysandreJik @sgugger)

[sgugger]

I wonder how this tokenizer exactly differs from a classic sentencepiece tokenizer (like BART)? Could we also add the fast version?

[datquocnguyen]

Thanks, @patil-suraj and @sgugger
I fixed the pull request based on you guys' comments.

The same comment I get from both of you is regarding the vocab_file. Here is a summary:

I did not train a sentencepiece for Vietnamese.
bartpho-syllable employs the existing pre-trained "sentencepiece" model from XLMRoBERTaTokenizer, and this pre-trained "sentencepiece" model is referred to as a vocab_file of 250K types.
reduced_vocab_file is a vocab containing 40K Vietnamese-specificized types extracted from the XLMRoBERTaTokenizer vocab of 250K types.

Usecase of BartphoTokenizer: Other languages can thus simply reuse BartphoTokenizer with their own reduced_vocab_file. The goal here is to reduce model sizes of existing pre-trained XLM-RoBERTa/mBART models when applying to a smaller set of languages instead of the whole 50/100 languages.

[patrickvonplaten]

this looks like BERT or RoBERTa - I don't think BART makes use of the CLS token no?
Think it would be better to write the function more like it is in T5:

transformers/src/transformers/models/t5/tokenization_t5.py

Line 217 in 3a8de58

return len(token_ids_0 + eos + token_ids_1 + eos) * [0]

[patrickvonplaten]

With the following statement:

transformers/src/transformers/models/t5/tokenization_t5.py

Line 201 in 3a8de58

Create a mask from the two sequences passed to be used in a sequence-pair classification task. T5 does not make

[patrickvonplaten]

Bart is a bad example as it inherits from RoBERTa

[datquocnguyen]

The BartphoTokenizer also looks like BarthezTokenizer, as they are both inherited from BartTokenizer (RobertaTokenizer). And BARThez also got approved.

transformers/src/transformers/models/barthez/tokenization_barthez.py

Lines 227 to 229 in 85d69a7

	if token_ids_1 is None:
	return len(cls + token_ids_0 + sep) * [0]
	return len(cls + token_ids_0 + sep + sep + token_ids_1 + sep) * [0]

• When BartphoTokenizer written following RobertaTokenizer, the transformers BARTpho produces the same feature outputs as its fairseq BARTpho counterpart. This is exactly what I expected since I originally trained BARTpho using fairseq and then converted it into transformers.

• Then when I wrote BartphoTokenizer following T5Tokenizer as you suggested, transformers and fairseq BARTpho variants produced different feature outputs given the same input text.

I am not sure why it's a bad example. Any feedback on this @patrickvonplaten ? Thanks.

[SaulLu]

Thank you so much for this addition and your work @datquocnguyen !

Concerning tokenizer fast, indeed I don't think that our tokenizer fast were designed to support this kind of behavior. It might be possible to find a trick to make it work but unfortunately I don't see it now.

[datquocnguyen]

@sgugger , @LysandreJik, @patil-suraj , @SaulLu and @patrickvonplaten
Please could you have a look and provide your feedback for my recent changes? Thanks.

[LysandreJik]

This looks good to me @datquocnguyen, thank you for adding another tokenizer alongside BERTweet and PhoBERT!

Too bad that there can be no fast tokenizer, but this LGTM either way!

[datquocnguyen]

Thanks @LysandreJik

My pull request suddenly failed the check run_tests_torch_and_flax:

FAILED tests/test_modeling_flax_clip.py::FlaxCLIPModelTest::test_equivalence_flax_to_pt
FAILED tests/test_modeling_flax_clip.py::FlaxCLIPModelTest::test_equivalence_pt_to_flax

They are out of my control, not relating to BartphoTokenizer.

[sgugger]

Yes this is a problem unrelated to this PR so you can ignore those failures. They should be fixed tomorrow :-)

[sgugger]

One last small comment and you should then run make fix-copies. Then we will be good to merge :-)

[datquocnguyen]

@sgugger I made a revision following your last comment. Thanks.
FYI, two failed tests are not related to BartphoTokenizer.

[sgugger]

Thanks a gain for your contribution!