[BT] Add `Bettertransformer` support for FSMT #494

Sumanth077 · 2022-11-20T11:12:59Z

What does this PR do?

Fixes # (issue)

Before submitting

This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
Did you make sure to update the documentation with your changes?
Did you write any new necessary tests?

younesbelkada

Thanks a lot for starting the PR!
First of all, I can see several "unneeded" files (.idea, *.xml files) would you mind removing them as they are not needed at all 🙏
Let me guide you here step by step:

Step 1: I have already identified for you the name of the layer that needs to be replaced. For FSMT the layer is called EncoderLayer

Step 2: In optimum/bettertransformer/models/encoder_models.py add a new class, here you can call it FSMTEncoderLayerBetterTransformer and follow the same template / logic as BertEncoderLayerBetterTransformer for assigning the new keys to the old one. The challenge here is to find out which key corresponds to exactly which part of the TransformerEncoderLayer. This step is described here in the documentation

Step 3: Build the forward pass, again you can follow what is done in BERT and make sure to respect what is said here

Step 4: Import you new class, so here FSMTEncoderLayerBetterTransformer inside optimum/bettertransformer/models/__init__.py, and add a new line inside BETTER_TRANFORMER_LAYERS_MAPPING_DICT, something that would look like: "EncoderLayer":FSMTEncoderLayerBetterTransformer

Step 5: You can check your conversion by doing something like:

from transformers import AutoModel
from optimum.bettertransformer import BetterTransformer

model = AutoModel.from_pretrained("hf-internal-testing/tiny-random-FSMTModel")
print(model)

And make sure your added layers are inside the model!
Let me know if there is anything unclear

Sumanth077 · 2022-11-22T10:06:06Z

Thankyou for the detailed breakdown @younesbelkada. I have followed and added all the things mentioned above.

Created a BertEncoderLayerBetterTransformer Class and assigned all the keys required.
Done Implementing the Forward Pass(Not sure whether this correct, but would love your detailed review and help here)
Imported the new class and added (EncoderLayer":FSMTEncoderLayerBetterTransformer) to the dictionary
Checked the conversion. The added layers are inside the model!
Finally removed the Unnecessary files.

Kindly review the recent commits @younesbelkada. Thankyou ❤️

younesbelkada · 2022-11-22T10:14:23Z

Hi @Sumanth077 !
Thanks a lot for your great work and your message! For now it looks very nice to me and we're close to merging that!
Now let's try to test your implementation before merging 💪
Would you mind adding the line "hf-internal-testing/tiny-random-FSMTModel" exactly here
Then you can just run pytest tests/bettertransformer//test_bettertransformer_encoder.py and let me know how it goes!

.gitignore

Sumanth077 · 2022-11-22T12:18:41Z

Thankyou @younesbelkada. Unfortunately Out of 14, 4 Test Cases got passed, 6 Test cases Failed and 4 got ignored.

Including a screenshot for reference:

I will go through that. Also your assistance would be really great

younesbelkada · 2022-11-22T12:20:54Z

Thanks! Could you please share with me the entire error trace?
Alternatively you can commit your changes (after removing the .gitignore and I can activate the tests here)

Sumanth077 · 2022-11-22T12:43:40Z

@younesbelkada Removed the .gitignorefile and also committed the remaining changes

HuggingFaceDocBuilderDev · 2022-11-22T12:48:32Z

The documentation is not available anymore as the PR was closed or merged.

younesbelkada · 2022-11-22T14:41:20Z

I have merged #501 , could you merge your branch with the current main branch ? 🙏 Thanks!

optimum/bettertransformer/models/encoder_models.py

.gitignore

Sumanth077 · 2022-11-22T15:21:16Z

@younesbelkada I have done the suggested changes. Could you kindly confirm it. I can merge the branch if everything is fine.

younesbelkada

Thanks a lot for your efforts!

It seems that we still need to address few points before merging 💪 Keep it up! And let me know if you face into any issue

tests/bettertransformer/test_bettertransformer_encoder.py

.gitignore

optimum/bettertransformer/models/encoder_models.py

Sumanth077 · 2022-11-22T16:09:06Z

@younesbelkada Sorry for the confusion, I have modified the gitignore file and as well as re added the line in "test_better transformer".

And regarding the conversion, getting few errors after running the above script. I will go through and try to resolve that

younesbelkada · 2022-11-23T09:10:51Z

Hi @Sumanth077 !
How is the integration going ? 💪 You are very close to merge this PR! Let us know here if you face into any issue!

Sumanth077 · 2022-11-23T10:38:13Z

Hi @younesbelkada I have fixed the changes and the conversion is fine.

Now I am not getting any errors while running the above script.

Now only 4 test cases are failing, previously it was 6.

younesbelkada · 2022-11-23T10:42:06Z

Could you let me know which test is failing? and push your latest changes here? Thanks!

Sumanth077 · 2022-11-23T17:44:39Z

Hi @younesbelkada could you kindly help me in resolving the above Failed Test Cases.

One issue I guess is in the defining of Forward Pass.

younesbelkada · 2022-11-24T09:45:14Z

Thanks for your patience @Sumanth077 !
This was slightly trickier than I have expected, I had to slightly refactor the testing suite :) The reason behind that is that encoder-decoder models expects decoder_inputs_ids as input together with input_ids. The best was to define a custom testing class for encoder-decoder based models as I suggested here.
I also ran styling with make style which is necessary before pushing any code here.
Apart from that, great job on converting the model! The PR is now ready for final review 💪

younesbelkada

Looking great! Thanks a lot for your clean implementation of FSMT integration for BetterTransformer ! 🎉
Leaving the last review for @fxmarty and/or @michaelbenayoun

michaelbenayoun · 2022-11-24T10:15:20Z

tests/bettertransformer/testing_bettertransformer_utils.py

@@ -70,7 +70,7 @@ def test_logits(self):
                torch.manual_seed(0)
                bt_hidden_states = converted_model(**inputs)[0]

-                if "gelu_new" in vars(random_config).values():
+                if "gelu_new" in list(random_config.to_dict().values()):


Great change, but no need to cast to list here!

thanks! but I think the to_dict() is necessary (otherwise I get a VERY weird error from transformers)

Yes exactly, but accessing the value is enough here

tests/bettertransformer/testing_bettertransformer_utils.py

Sumanth077 · 2022-11-24T11:13:21Z

Thankyou so much @younesbelkada. I couldn't have done it, without your Support. Really appreciate it❤️

Looking forward to contribute to more projects 🚀

younesbelkada · 2022-11-24T11:18:42Z

Thanks a lot for your contribution @Sumanth077 !
One last thing, could you please add FSMT on the documentation together with the link to the paper: https://arxiv.org/abs/1907.06616 / the addition would go exactly here
Thanks!

Sumanth077 · 2022-11-24T13:48:06Z

Hi @younesbelkada I have mistakenly called git pull origin local branch, now instead of one change I done in doc file. It is asking me commit all these files again.

Could you help me with this.

younesbelkada · 2022-11-24T13:54:50Z

It is fine, I have just pushed it to the doc ;) Thanks again!

sumanth papareddy and others added 3 commits November 20, 2022 15:39

No changes made

8d5bd36

No Significant Changes

5902aa3

No significant changes

ed8e9ad

younesbelkada reviewed Nov 21, 2022

View reviewed changes

Sumanth077 added 2 commits November 22, 2022 10:54

removed unnecessary files like .idea, .xml

ffeb9ab

added encoder layer better transformer for FSMT

adf1aa9

younesbelkada reviewed Nov 22, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

younesbelkada mentioned this pull request Nov 22, 2022

Community contribution - BetterTransformer integration for more models! huggingface/transformers#20372

Closed

14 tasks

removed gitignore and modifed test_bettertransformer_encoder

b897b2d

younesbelkada reviewed Nov 22, 2022

View reviewed changes

optimum/bettertransformer/models/encoder_models.py Outdated Show resolved Hide resolved

younesbelkada reviewed Nov 22, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

Modifed Encoder_Models

94f6d87

younesbelkada reviewed Nov 22, 2022

View reviewed changes

tests/bettertransformer/test_bettertransformer_encoder.py Show resolved Hide resolved

.gitignore Outdated Show resolved Hide resolved

optimum/bettertransformer/models/encoder_models.py Outdated Show resolved Hide resolved

modified test better transformer & gitignore

2ed3dff

ka00ri mentioned this pull request Nov 22, 2022

Community contribution - BetterTransformer integration for more models! #488

Open

15 tasks

minor changes in enocoder models

0c3acfe

refactor + fix tests - add support for FMST

bd735ae

younesbelkada marked this pull request as ready for review November 24, 2022 09:45

younesbelkada changed the title ~~Bettertransformer FSMT~~ [BT] Add Bettertransformer support for FSMT Nov 24, 2022

younesbelkada approved these changes Nov 24, 2022

View reviewed changes

younesbelkada requested review from fxmarty and michaelbenayoun November 24, 2022 09:58

michaelbenayoun reviewed Nov 24, 2022

View reviewed changes

Merge branch 'main' into bettertransformer_FSMT

0d3881b

younesbelkada reviewed Nov 24, 2022

View reviewed changes

tests/bettertransformer/testing_bettertransformer_utils.py Outdated Show resolved Hide resolved

Update tests/bettertransformer/testing_bettertransformer_utils.py

c5c1331

add FSMT to doc

e168962

younesbelkada requested a review from michaelbenayoun November 24, 2022 13:55

michaelbenayoun approved these changes Nov 24, 2022

View reviewed changes

michaelbenayoun merged commit 389a153 into huggingface:main Nov 24, 2022

adit299 mentioned this pull request Jan 9, 2023

Adding BetterTransformer support for ProphetNet #648

Draft

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BT] Add `Bettertransformer` support for FSMT #494

[BT] Add `Bettertransformer` support for FSMT #494

Sumanth077 commented Nov 20, 2022

younesbelkada left a comment •

edited

Sumanth077 commented Nov 22, 2022

younesbelkada commented Nov 22, 2022

Sumanth077 commented Nov 22, 2022

younesbelkada commented Nov 22, 2022 •

edited

Sumanth077 commented Nov 22, 2022

HuggingFaceDocBuilderDev commented Nov 22, 2022 •

edited

younesbelkada commented Nov 22, 2022

Sumanth077 commented Nov 22, 2022

younesbelkada left a comment

Sumanth077 commented Nov 22, 2022

younesbelkada commented Nov 23, 2022

Sumanth077 commented Nov 23, 2022

younesbelkada commented Nov 23, 2022

Sumanth077 commented Nov 23, 2022

younesbelkada commented Nov 24, 2022

younesbelkada left a comment

michaelbenayoun Nov 24, 2022

younesbelkada Nov 24, 2022

michaelbenayoun Nov 24, 2022

Sumanth077 commented Nov 24, 2022

younesbelkada commented Nov 24, 2022

Sumanth077 commented Nov 24, 2022 •

edited

younesbelkada commented Nov 24, 2022

[BT] Add Bettertransformer support for FSMT #494

[BT] Add Bettertransformer support for FSMT #494

Conversation

Sumanth077 commented Nov 20, 2022

What does this PR do?

Before submitting

younesbelkada left a comment • edited

Choose a reason for hiding this comment

Sumanth077 commented Nov 22, 2022

younesbelkada commented Nov 22, 2022

Sumanth077 commented Nov 22, 2022

younesbelkada commented Nov 22, 2022 • edited

Sumanth077 commented Nov 22, 2022

HuggingFaceDocBuilderDev commented Nov 22, 2022 • edited

younesbelkada commented Nov 22, 2022

Sumanth077 commented Nov 22, 2022

younesbelkada left a comment

Choose a reason for hiding this comment

Sumanth077 commented Nov 22, 2022

younesbelkada commented Nov 23, 2022

Sumanth077 commented Nov 23, 2022

younesbelkada commented Nov 23, 2022

Sumanth077 commented Nov 23, 2022

younesbelkada commented Nov 24, 2022

younesbelkada left a comment

Choose a reason for hiding this comment

michaelbenayoun Nov 24, 2022

Choose a reason for hiding this comment

younesbelkada Nov 24, 2022

Choose a reason for hiding this comment

michaelbenayoun Nov 24, 2022

Choose a reason for hiding this comment

Sumanth077 commented Nov 24, 2022

younesbelkada commented Nov 24, 2022

Sumanth077 commented Nov 24, 2022 • edited

younesbelkada commented Nov 24, 2022

[BT] Add `Bettertransformer` support for FSMT #494

[BT] Add `Bettertransformer` support for FSMT #494

younesbelkada left a comment •

edited

younesbelkada commented Nov 22, 2022 •

edited

HuggingFaceDocBuilderDev commented Nov 22, 2022 •

edited

Sumanth077 commented Nov 24, 2022 •

edited