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.

Sign up for GitHub

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

Jump to bottom

Fix import error from deprecation in transformers #1415

Merged
merged 2 commits into from
Mar 11, 2024
Merged

Fix import error from deprecation in transformers #1415

merged 2 commits into from
Mar 11, 2024

Conversation

lewtun
Copy link
Member

@lewtun lewtun commented Mar 11, 2024 •
edited
Loading

The top_k_top_p_filtering() method will be deprecated (i.e. removed) from transformers >= 4.39 and the simplest solution is to copy the method directly into trl

Closes #1409

younesbelkada
younesbelkada reviewed Mar 11, 2024
View reviewed changes
Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much @lewtun !
It says:

ImportError: cannot import name 'TopKLogitsWarper' from 'transformers.generation_utils' (/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/transformers/generation_utils.py)

Do we need to guard the import for transformers main that have TopKLogitsWarper ?

@lewtun
Copy link
Member Author

lewtun commented Mar 11, 2024

Thanks so much @lewtun ! It says:

ImportError: cannot import name 'TopKLogitsWarper' from 'transformers.generation_utils' (/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/transformers/generation_utils.py)

Do we need to guard the import for transformers main that have TopKLogitsWarper ?

Both logits processors are present as far back as our min required transformers version (v31.0): https://huggingface.co/docs/transformers/v4.31.0/en/internal/generation_utils#transformers.TopPLogitsWarper

The new import path should work across all of them I think!

@HuggingFaceDocBuilderDev
Copy link

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@lewtun lewtun mentioned this pull request Mar 11, 2024
Merged
pacman100 added a commit to pacman100/trl that referenced this pull request Mar 11, 2024
@pacman100
fix deprecation using changes from PR huggingface#1415
77b9cb1
younesbelkada
younesbelkada approved these changes Mar 11, 2024
View reviewed changes
Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much @lewtun ! 🚀

@younesbelkada younesbelkada merged commit 7630f87 into main Mar 11, 2024
9 checks passed
@younesbelkada younesbelkada deleted the fix-imports branch March 11, 2024 12:23
@hiyouga hiyouga mentioned this pull request Mar 12, 2024
Closed
1 task
younesbelkada added a commit that referenced this pull request Mar 13, 2024
@pacman100 @younesbelkada
Add support for FSDP+QLoRA and DeepSpeed ZeRO3+QLoRA (#1416)
58c0888 
* don't do mp casting

* don't use `prepare_for_kbit` when using fsdp+qlora or dsz3+qlora

* changes to enable fsdp+qlora and dsz3+qlora

* revert

* Update sft_trainer.py

* quality

* fix deprecation using changes from PR #1415

* fixes

* quality

* Update trl/trainer/sft_trainer.py

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>

* quality

* relaunch tests

---------

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>
@Haruka1307
Copy link

Thanks so much @lewtun ! It says:

ImportError: cannot import name 'TopKLogitsWarper' from 'transformers.generation_utils' (/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/transformers/generation_utils.py)

Do we need to guard the import for transformers main that have TopKLogitsWarper ?

Both logits processors are present as far back as our min required transformers version (v31.0): https://huggingface.co/docs/transformers/v4.31.0/en/internal/generation_utils#transformers.TopPLogitsWarper

The new import path should work across all of them I think!

I wonder the statement is
from transformers.generation import TopKLogitsWarper, TopPLogitsWarper
I check the source code updated in trl .
1710408796081

kashif pushed a commit that referenced this pull request Mar 14, 2024
@pacman100 @younesbelkada @kashif
Add support for FSDP+QLoRA and DeepSpeed ZeRO3+QLoRA (#1416)
e3babbf 
* don't do mp casting

* don't use `prepare_for_kbit` when using fsdp+qlora or dsz3+qlora

* changes to enable fsdp+qlora and dsz3+qlora

* revert

* Update sft_trainer.py

* quality

* fix deprecation using changes from PR #1415

* fixes

* quality

* Update trl/trainer/sft_trainer.py

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>

* quality

* relaunch tests

---------

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>
kashif pushed a commit to fe1ixxu/trl that referenced this pull request Mar 15, 2024
@lewtun @kashif
Fix import error from deprecation in transformers (huggingface#1415)
291adae 
* Fix import error from  deprecation in transformers

* Fix import path
kashif pushed a commit to fe1ixxu/trl that referenced this pull request Mar 15, 2024
@pacman100 @younesbelkada @kashif
Add support for FSDP+QLoRA and DeepSpeed ZeRO3+QLoRA (huggingface#1416)
163c36e 
* don't do mp casting

* don't use `prepare_for_kbit` when using fsdp+qlora or dsz3+qlora

* changes to enable fsdp+qlora and dsz3+qlora

* revert

* Update sft_trainer.py

* quality

* fix deprecation using changes from PR huggingface#1415

* fixes

* quality

* Update trl/trainer/sft_trainer.py

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>

* quality

* relaunch tests

---------

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>
@shub-kris shub-kris mentioned this pull request Mar 20, 2024
Merged
@mjschock mjschock mentioned this pull request Mar 27, 2024
Closed
lapp0 pushed a commit to lapp0/trl that referenced this pull request May 10, 2024
@lewtun
Fix import error from deprecation in transformers (huggingface#1415)
829c317 
* Fix import error from  deprecation in transformers

* Fix import path
lapp0 pushed a commit to lapp0/trl that referenced this pull request May 10, 2024
@pacman100 @younesbelkada
Add support for FSDP+QLoRA and DeepSpeed ZeRO3+QLoRA (huggingface#1416)
aabf89b 
* don't do mp casting

* don't use `prepare_for_kbit` when using fsdp+qlora or dsz3+qlora

* changes to enable fsdp+qlora and dsz3+qlora

* revert

* Update sft_trainer.py

* quality

* fix deprecation using changes from PR huggingface#1415

* fixes

* quality

* Update trl/trainer/sft_trainer.py

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>

* quality

* relaunch tests

---------

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>
@anirudTT anirudTT mentioned this pull request Jul 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@younesbelkada younesbelkada younesbelkada approved these changes

@lvwerra lvwerra Awaiting requested review from lvwerra

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can not import name top_k_top_p_filtering
4 participants
@lewtun @HuggingFaceDocBuilderDev @Haruka1307 @younesbelkada