Add methods to PreTrainedModel to use PyTorch's BetterTransformer #21259

fxmarty · 2023-01-23T13:41:06Z

As per title.

Should be merged only on the next Optimum release that will include huggingface/optimum#676

Before submitting

Tests are still to be done.

Who can review?

@younesbelkada @sgugger

HuggingFaceDocBuilderDev · 2023-01-23T13:54:40Z

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

younesbelkada · 2023-01-23T13:57:59Z

as a side note, since in the previous optimum versions the save_pretrained and push_to_hub methods are not blocked, I propose to explicitly block them for transformed models in this PR and/or force users to use a certain version of optimum.

fxmarty · 2023-01-23T14:14:53Z

Yes we should probably force the next optimum version.

sgugger

Thanks for adding those! I'd also make sure to document the methods and add something in the optimization guides we have :-)

src/transformers/modeling_utils.py

sgugger · 2023-01-23T14:46:29Z

src/transformers/modeling_utils.py

+        if not is_optimum_available():
+            raise ImportError("The package `optimum` is required to use BetterTransformer.")
+
+        from optimum.bettertransformer import BetterTransformer


I think a version check on optimum with a clear error message would be good? Also can the transform be applied twice? If not there should be a check and a clear error message as well.

Also can the transform be applied twice? If not there should be a check and a clear error message as well.

I think there's currently no check for this @younesbelkada . I will add it on the Optimum side. So as to keep the transformers side as lightweight as possible.

src/transformers/modeling_utils.py

fxmarty · 2023-02-06T10:47:10Z

Should be ready @sgugger , the documentation has been extended in https://moon-ci-docs.huggingface.co/docs/transformers/pr_21259/en/perf_infer_gpu_one .

Let me know if I should add a test - in which case optimum should be added in the setup.py, I guess.

younesbelkada · 2023-02-06T11:07:29Z

@fxmarty there should be no need to add optimum in setup.py, we can do something similar than bitsandbytes and add optimum in the Dockerfile of the Docker image that will run the slow tests:

transformers/docker/transformers-all-latest-gpu/Dockerfile

Line 52 in 0db5d91

RUN python3 -m pip install --no-cache-dir bitsandbytes

I very much agree that we should add tests, especially to test accelerate compatibility, happy to help you on this, let me know if you need help

fxmarty · 2023-02-06T11:30:03Z

Thanks, will do!

especially to test accelerate compatibility

Isn't this already tested on Optimum side?

younesbelkada · 2023-02-06T11:33:33Z

Isn't this already tested on Optimum side?

Yes but the tests are run on GPU: therefore not run on any of the runners on optimum on a daily basis (but not sue if there are tested somewhere else) - I just asked individually to each contributor to run the accelerate test locally on their GPU before merging (only in case I have serious doubts that the PR breaks anything related to accelerate).
Since in transformers tests are run on GPU on daily basis, we can leverage that and setup a small BetterTransformer testing suite that tests all the tests + accelerate compatibility. Also this enables us to flag anything we need to upstream to accelerate if something breaks BT integration with accelerate

fxmarty · 2023-02-06T11:57:52Z

There are tests on the daily basis on GPU in Optimum, for example https://github.com/huggingface/optimum/blob/main/.github/workflows/test_onnxruntime_train.yml and https://github.com/huggingface/optimum/blob/main/.github/workflows/test_onnxruntime_gpu.yml

In my opinion, thorough tests should be added in Optimum, not Transformers. The test I was thinking of in Transformers was only an integration one to check that there's no error.

younesbelkada · 2023-02-08T11:52:22Z

There is an issue with accelerate loaded models and transform from BT, let's wait until this gets fixed before merging this PR

github-actions · 2023-03-04T15:01:45Z

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

fxmarty · 2023-03-04T22:16:58Z

not stale

sgugger · 2023-03-06T14:35:34Z

If you want this PR included in the next release, you should finish the work and have it merged sooner rather than later :-)
The last I saw was Younes telling we should wait for a fix, was that fix added? Then this needs a rebase on main since it has been a while.

younesbelkada · 2023-03-06T14:39:07Z

Thanks for the headsup!
Indeed we are working on fixing some bugs on optimum side that was introduced by one of my PRs (the revert-transform PR) before adding the invert_transform method
We can maybe merge this PR by keeping only transform method and blocking the save_pretrained & push_to_hub methods after transforming the model

fxmarty · 2023-03-06T14:45:08Z

you should finish the work and have it merged sooner rather than later :-)

There is substantial work left in Optimum before this should be merged. Marking as draft for now!

sgugger · 2023-03-06T14:47:36Z

OK, so this won't be in the next release of Transformers (probably this week in preparation for PyTorch 2.0).

github-actions · 2023-03-30T15:02:41Z

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

LysandreJik · 2023-04-10T13:04:54Z

Hey @fxmarty and @younesbelkada, are there standing PRs in optimum that need to be merged for this to proceed/anything we can help with to have this move forward? Thanks :)

younesbelkada · 2023-04-10T16:15:42Z

Hey @LysandreJik @sgugger
@fxmarty recently managed to fix all issues related to decoder-based models integration in optimum! I believe that this PR could be re-opened, in my understanding we just need to add few tests and we should be good to go

younesbelkada · 2023-04-25T11:22:43Z

@sgugger @LysandreJik this is now ready for review!

sgugger

Thanks! Just have one comment on the is_optimum_available function but the rest looks fine!

src/transformers/__init__.py

src/transformers/integrations.py

src/transformers/modeling_utils.py

src/transformers/testing_utils.py

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

sgugger

Thanks!

michaelbenayoun

Left a few nits.
LGTM!

docs/source/en/perf_infer_gpu_one.mdx

docs/source/en/perf_train_gpu_one.mdx

michaelbenayoun · 2023-04-26T15:38:25Z

src/transformers/modeling_utils.py

+        if not is_optimum_available():
+            raise ImportError("The package `optimum` is required to use Better Transformer.")
+
+        from optimum.version import __version__ as optimum_version
+
+        if version.parse(optimum_version) < version.parse("1.7.0"):
+            raise ImportError(
+                f"Please install optimum>=1.7.0 to use Better Transformer. The version {optimum_version} was found."
+            )


Maybe factor all of this into a is_bettertransformer_available?

Hmm I would say this is too specific, maybe let's keep it as it is

michaelbenayoun · 2023-04-26T15:39:00Z

tests/bettertransformer/__init__.py

@@ -0,0 +1 @@
+


Is it wanted?

Co-authored-by: Michael Benayoun <mickbenayoun@gmail.com>

…ggingface#21259) * fix mess * better documentation * typo * fix doc * update * add test * fix test * more tests * Update src/transformers/modeling_utils.py Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> * move to utils * Apply suggestions from code review Co-authored-by: Michael Benayoun <mickbenayoun@gmail.com> * nit --------- Co-authored-by: younesbelkada <younesbelkada@gmail.com> Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com> Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> Co-authored-by: Michael Benayoun <mickbenayoun@gmail.com>

fxmarty changed the title ~~Add method to PreTrainedModel to use PyTorch's BetterTransformer~~ Add methods to PreTrainedModel to use PyTorch's BetterTransformer Jan 23, 2023

fxmarty requested review from sgugger and younesbelkada January 23, 2023 13:41

sgugger reviewed Jan 23, 2023

View reviewed changes

fix mess

c6dd4b9

fxmarty force-pushed the add-use-better-transformer-option branch from 66366ea to c6dd4b9 Compare February 6, 2023 09:44

fxmarty added 3 commits February 6, 2023 10:59

better documentation

2565272

typo

141bc36

fix doc

12303d7

fxmarty marked this pull request as ready for review February 6, 2023 10:45

fxmarty requested a review from sgugger February 6, 2023 10:47

This was referenced Feb 6, 2023

Raise error on double call to BetterTransformer.transform() huggingface/optimum#750

Merged

Add GPU tests for BetterTransformer huggingface/optimum#751

Merged

fxmarty marked this pull request as draft March 6, 2023 14:45

github-actions bot closed this Apr 8, 2023

younesbelkada reopened this Apr 10, 2023

fxmarty added 3 commits April 11, 2023 12:43

Merge branch 'main' into add-use-better-transformer-option

069d0c3

update

05856df

add test

6fe7ecb

fxmarty marked this pull request as ready for review April 11, 2023 11:42

fix test

0b20952

fxmarty requested a review from LysandreJik April 12, 2023 10:46

younesbelkada added 2 commits April 25, 2023 11:09

Merge remote-tracking branch 'upstream/main' into HEAD

503a180

more tests

b8929e6

sgugger reviewed Apr 25, 2023

View reviewed changes

src/transformers/__init__.py Outdated Show resolved Hide resolved

src/transformers/integrations.py Outdated Show resolved Hide resolved

src/transformers/modeling_utils.py Outdated Show resolved Hide resolved

src/transformers/testing_utils.py Outdated Show resolved Hide resolved

younesbelkada and others added 2 commits April 25, 2023 15:43

Update src/transformers/modeling_utils.py

45c9515

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

move to utils

9c5e0d9

younesbelkada requested a review from sgugger April 25, 2023 13:48

sgugger approved these changes Apr 25, 2023

View reviewed changes

younesbelkada requested a review from michaelbenayoun April 25, 2023 15:52

michaelbenayoun approved these changes Apr 26, 2023

View reviewed changes

younesbelkada and others added 2 commits April 26, 2023 19:02

Apply suggestions from code review

76639a6

Co-authored-by: Michael Benayoun <mickbenayoun@gmail.com>

nit

31a31c9

younesbelkada merged commit 3042c63 into huggingface:main Apr 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add methods to PreTrainedModel to use PyTorch's BetterTransformer #21259

Add methods to PreTrainedModel to use PyTorch's BetterTransformer #21259

fxmarty commented Jan 23, 2023

HuggingFaceDocBuilderDev commented Jan 23, 2023 •

edited

Loading

younesbelkada commented Jan 23, 2023

fxmarty commented Jan 23, 2023

sgugger left a comment

sgugger Jan 23, 2023

fxmarty Feb 6, 2023

fxmarty commented Feb 6, 2023

younesbelkada commented Feb 6, 2023

fxmarty commented Feb 6, 2023 •

edited

Loading

younesbelkada commented Feb 6, 2023 •

edited

Loading

fxmarty commented Feb 6, 2023

younesbelkada commented Feb 8, 2023

github-actions bot commented Mar 4, 2023

fxmarty commented Mar 4, 2023

sgugger commented Mar 6, 2023

younesbelkada commented Mar 6, 2023

fxmarty commented Mar 6, 2023

sgugger commented Mar 6, 2023

github-actions bot commented Mar 30, 2023

LysandreJik commented Apr 10, 2023

younesbelkada commented Apr 10, 2023

younesbelkada commented Apr 25, 2023

sgugger left a comment

sgugger left a comment

michaelbenayoun left a comment

michaelbenayoun Apr 26, 2023

younesbelkada Apr 26, 2023

michaelbenayoun Apr 26, 2023

Add methods to PreTrainedModel to use PyTorch's BetterTransformer #21259

Add methods to PreTrainedModel to use PyTorch's BetterTransformer #21259

Conversation

fxmarty commented Jan 23, 2023

Before submitting

Who can review?

HuggingFaceDocBuilderDev commented Jan 23, 2023 • edited Loading

younesbelkada commented Jan 23, 2023

fxmarty commented Jan 23, 2023

sgugger left a comment

Choose a reason for hiding this comment

sgugger Jan 23, 2023

Choose a reason for hiding this comment

fxmarty Feb 6, 2023

Choose a reason for hiding this comment

fxmarty commented Feb 6, 2023

younesbelkada commented Feb 6, 2023

fxmarty commented Feb 6, 2023 • edited Loading

younesbelkada commented Feb 6, 2023 • edited Loading

fxmarty commented Feb 6, 2023

younesbelkada commented Feb 8, 2023

github-actions bot commented Mar 4, 2023

fxmarty commented Mar 4, 2023

sgugger commented Mar 6, 2023

younesbelkada commented Mar 6, 2023

fxmarty commented Mar 6, 2023

sgugger commented Mar 6, 2023

github-actions bot commented Mar 30, 2023

LysandreJik commented Apr 10, 2023

younesbelkada commented Apr 10, 2023

younesbelkada commented Apr 25, 2023

sgugger left a comment

Choose a reason for hiding this comment

sgugger left a comment

Choose a reason for hiding this comment

michaelbenayoun left a comment

Choose a reason for hiding this comment

michaelbenayoun Apr 26, 2023

Choose a reason for hiding this comment

younesbelkada Apr 26, 2023

Choose a reason for hiding this comment

michaelbenayoun Apr 26, 2023

Choose a reason for hiding this comment

HuggingFaceDocBuilderDev commented Jan 23, 2023 •

edited

Loading

fxmarty commented Feb 6, 2023 •

edited

Loading

younesbelkada commented Feb 6, 2023 •

edited

Loading