Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds ASTLayer support for BetterTransformer #548

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ravenouse
Copy link
Contributor

@ravenouse ravenouse commented Dec 6, 2022

What does this PR do?

Adds ASTLayer support for BetterTransformer

Fixes Community contribution - BetterTransformer integration for more models! #20372

Questions:

  1. I used "MIT/ast-finetuned-audioset-10-10-0.4593 as the test model to run pytest but some tests failed. I think one test model, like "hf-internal-testing/tiny-random-MBartModel", is needed to test the ASTLayerBetterTransformer.

截屏2022-12-05 下午9 59 59

  1. I have one question about the BetterTransformerBaseLayer class. I notice that we set self. use_gelu as false for the default setting but lots of the supported transformer models actually use the gelu activation function, like bert. Could you provide more information about it?

Thank you so much for your effort!!

Please let me know what else I need to do!

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@ravenouse
Copy link
Contributor Author

ravenouse commented Dec 18, 2022

Hi @younesbelkada !
I have made two additional changes to the branch to solve the test problems.
For the second change especially:

  1. I created a tiny ast test model, tiny-random-ast
    Compared with the original ast model, I did the following changes :
  • changed hidden_size from 768 to 32
  • changed intermediate_size from 3072 to 37
  • changed patch_size from 16 to 1
  • changed num_attention_heads from 12 to 4
  • changed num_hidden_layers from 12 to 5
  1. I separated the ast model from BetterTransformersAudioTest class and created another class BetterTransformersASTTest in test_bettertransformer_audio.py
    file to test the ast model.
    This exactly solved the test problems since the ast model does not have the processor as other audio models do. Instead, it uses featureExtractor, which has similar functionality.

Please let me know what you think about these changes and let me what else I need to do on this topic.

Thank you very much!

截屏2022-12-17 下午8 35 03

@younesbelkada
Copy link
Contributor

Hi @ravenouse !
This is great! Thanks for adding this, could you please rebase with main branch so that we can merge this PR? 🙏
Thanks!

@ravenouse
Copy link
Contributor Author

Hi @younesbelkada ! Happy New Year!

I have just rebased this branch with the current optimum main branch, and I re-run the test, pytest tests/bettertransformer/test_bettertransformer_audio.py::BetterTransformersASTTest.
The tests went well this time, except for one error with collections packages.
I believe this is caused by the version of python. I run the test with python 3.10.4.
截屏2023-01-04 下午6 50 41

Please let me know what else I can do!

Thank you so much!

@ravenouse
Copy link
Contributor Author

Hi @younesbelkada @fxmarty!

I did rebase some time ago. Could you spare some time to review this?
I deeply appreciate all the help!
Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Community contribution - BetterTransformer integration for more models!
3 participants