Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Consolidate BERT, XLM and RobERTa Tensorizers #1119

Closed

Conversation

kartikayk
Copy link
Contributor

Summary:
In this diff I take a fast stab at consolidating the XLM, BERT and RoBERTa Tensorizers. I kill a bunch of dead code and simiplify a lot.

  • I create a BERTTensorizerBase class which derives from Tensorizer and not TokenTensorizer since this makes the logic a lot easier especially since we no longer have to deal with all the bos, eos flags. Given that tokenize and lookup_tokens are not part of TokenTensorizer, I think this formulation makes a lot of sense.
  • As per suggestions, I derive the config classes from Tensorizer as well and kill all of the special flags.
  • I try to put as much of the functionality in the base class as possible in order to minimize copy paste code. There is still some but I dont want perfect to be the enemy of better.
  • I kill TLM - long live TLM.
  • I (temporaarily) kill support for OSS XLM which probably should have its own tensorizer anyways since it has nothing to do with transformer_sentence_encoder.

Reviewed By: rutyrinott

Differential Revision: D18290264

@facebook-github-bot facebook-github-bot added CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported labels Nov 8, 2019
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D18290264

Summary:
Pull Request resolved: facebookresearch#1119

In this diff I take a fast stab at consolidating the XLM, BERT and RoBERTa Tensorizers. I kill a bunch of dead code and simiplify a lot.
- I create a BERTTensorizerBase class which derives from Tensorizer and not TokenTensorizer since this makes the logic a lot easier especially since we no longer have to deal with all the bos, eos flags. Given that tokenize and lookup_tokens are not part of TokenTensorizer, I think this formulation makes a lot of sense.
- As per suggestions, I derive the config classes from Tensorizer as well and kill all of the special flags.
- I try to put as much of the functionality in the base class as possible in order to minimize copy paste code. There is still some but I dont want perfect to be the enemy of better.
- I kill TLM - long live TLM.
- I (temporaarily) kill support for OSS XLM which probably should have its own tensorizer anyways since it has nothing to do with transformer_sentence_encoder.

Reviewed By: rutyrinott

Differential Revision: D18290264

fbshipit-source-id: 573970838808399b6534dad1464057ebd9cb6d44
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D18290264

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 5b83da0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants