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

Uniformize model processors #31368

Merged
merged 107 commits into from
Oct 2, 2024
Merged

Uniformize model processors #31368

merged 107 commits into from
Oct 2, 2024

Conversation

molbap
Copy link
Contributor

@molbap molbap commented Jun 11, 2024 •
edited
Loading

What does this PR do?

Adds uniformized processors following #30511, #31197 in particular, and #31198 .

Adds support for:

  • Blip
  • Blip2
  • Bridgetower
  • Donut
  • Wav2vec2

molbap added 30 commits June 3, 2024 09:38
@molbap
add initial design for uniform processors + align model
b85036f
@molbap
add uniform processors for altclip + chinese_clip
1336931
@molbap
add uniform processors for blip + blip2
691a298
@molbap
fix mutable default 👀
bb8ac70
@molbap
add configuration test
cd8c601
@molbap
handle structured kwargs w defaults + add test
f00c852
@molbap
protect torch-specific test
693036f
@molbap
fix style
766da3a
@molbap
fix
844394d
@molbap
rebase
7d860a0
@molbap
update processor to generic kwargs + test
7cb9925
@molbap
fix style
ad4cbf7
@molbap
add sensible kwargs merge
def56cd
@molbap
update test
2e6b7e1
@molbap
fix assertEqual
c19bbc6
@molbap
move kwargs merging to processing common
3c38119
@molbap
rework kwargs for type hinting
81ae819
@molbap
just get Unpack from extensions
ce4abcd
@molbap
run-slow[align]
3acdf28
@molbap
handle kwargs passed as nested dict
404239f
@molbap
add from_pretrained test for nested kwargs handling
603be40
@molbap
[run-slow]align
71c9d6c
@molbap
update documentation + imports
26383c5
@molbap
update audio inputs
4521f4f
@molbap
protect audio types, silly
b96eb64
@molbap
try removing imports
9c5c01c
@molbap
make things simpler
3ccb505
@molbap
simplerer
142acf3
@molbap
move out kwargs test to common mixin
60a5730
@molbap
[run-slow]align
be6c141
@molbap
Copy link
Contributor Author

molbap commented Sep 24, 2024

@amyeroberts I finished digging through feature extractors for audio, not trivial, but seems it was missing something testwise. Now all seems to work 🧹 🧹

@molbap molbap mentioned this pull request Sep 24, 2024
Merged
@molbap
Copy link
Contributor Author

molbap commented Sep 24, 2024

One slow test model fails because of #33678

@amyeroberts
Copy link
Collaborator

@molbap Re this comment - is this PR ready for review now?

@molbap
Copy link
Contributor Author

molbap commented Sep 26, 2024

It should! failing tests are unrelated and should be fixed in #33678

@molbap
Copy link
Contributor Author

molbap commented Sep 27, 2024

I fixed a concat bug I noticed in another PR, #33678 , where I fixed it for instructblip but it remained for blip_2 🫣

@molbap molbap requested a review from amyeroberts September 27, 2024 09:17
amyeroberts
amyeroberts reviewed Sep 30, 2024
View reviewed changes
Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks! Just a few small comments

tests/test_processing_common.py Outdated Show resolved Hide resolved
tests/test_processing_common.py Show resolved Hide resolved
src/transformers/models/altclip/processing_altclip.py Outdated Show resolved Hide resolved
src/transformers/models/altclip/processing_altclip.py Outdated Show resolved Hide resolved
src/transformers/models/altclip/processing_altclip.py Outdated Show resolved Hide resolved
src/transformers/models/blip_2/processing_blip_2.py Outdated Show resolved Hide resolved
src/transformers/models/blip_2/processing_blip_2.py Show resolved Hide resolved
src/transformers/models/bridgetower/processing_bridgetower.py Outdated Show resolved Hide resolved
src/transformers/models/donut/processing_donut.py Outdated Show resolved Hide resolved
src/transformers/models/donut/processing_donut.py Show resolved Hide resolved
molbap and others added 5 commits October 1, 2024 09:34
@HuggingFaceDocBuilderDev
Copy link

Hey! 🤗 Thanks for your contribution to the transformers library!

Before merging this pull request, slow tests CI should be triggered. To enable this:

  • Add the run-slow label to the PR
  • When your PR is ready for merge and all reviewers' comments have been addressed, push an empty commit with the command [run-slow] followed by a comma separated list of all the models to be tested, i.e. [run_slow] model_to_test_1, model_to_test_2
    • If the pull request affects a lot of models, put at most 10 models in the commit message
  • A transformers maintainer will then approve the workflow to start the tests

(For maintainers) The documentation for slow tests CI on PRs is here.

@molbap molbap requested a review from amyeroberts October 1, 2024 09:22
amyeroberts
amyeroberts approved these changes Oct 1, 2024
View reviewed changes
Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks!

@molbap molbap merged commit 50290cf into huggingface:main Oct 2, 2024
36 checks passed
@molbap molbap deleted the uniform_processors_3 branch October 2, 2024 08:41
NielsRogge pushed a commit to NielsRogge/transformers that referenced this pull request Oct 21, 2024
@molbap @amyeroberts @NielsRogge
Uniformize model processors (huggingface#31368)
c21341f 
* add initial design for uniform processors + align model

* add uniform processors for altclip + chinese_clip

* add uniform processors for blip + blip2

* fix mutable default 👀

* add configuration test

* handle structured kwargs w defaults + add test

* protect torch-specific test

* fix style

* fix

* rebase

* update processor to generic kwargs + test

* fix style

* add sensible kwargs merge

* update test

* fix assertEqual

* move kwargs merging to processing common

* rework kwargs for type hinting

* just get Unpack from extensions

* run-slow[align]

* handle kwargs passed as nested dict

* add from_pretrained test for nested kwargs handling

* [run-slow]align

* update documentation + imports

* update audio inputs

* protect audio types, silly

* try removing imports

* make things simpler

* simplerer

* move out kwargs test to common mixin

* [run-slow]align

* skip tests for old processors

* [run-slow]align, clip

* !$#@!! protect imports, darn it

* [run-slow]align, clip

* [run-slow]align, clip

* update common processor testing

* add altclip

* add chinese_clip

* add pad_size

* [run-slow]align, clip, chinese_clip, altclip

* remove duplicated tests

* fix

* add blip, blip2, bridgetower

Added tests for bridgetower which override common. Also modified common
tests to force center cropping if existing

* fix

* update doc

* improve documentation for default values

* add model_max_length testing

This parameter depends on tokenizers received.

* Raise if kwargs are specified in two places

* fix

* removed copied from

* match defaults

* force padding

* fix tokenizer test

* clean defaults

* move tests to common

* add missing import

* fix

* adapt bridgetower tests to shortest edge

* uniformize donut processor + tests

* add wav2vec2

* extend common testing to audio processors

* add testing + bert version

* propagate common kwargs to different modalities

* BC order of arguments

* check py version

* revert kwargs merging

* add draft overlap test

* update

* fix blip2 and wav2vec due to updates

* fix copies

* ensure overlapping kwargs do not disappear

* replace .pop by .get to handle duplicated kwargs

* fix copies

* fix missing import

* add clearly wav2vec2_bert to uniformized models

* fix copies

* increase number of features

* fix style

* [run-slow] blip, blip2, bridgetower, donut, wav2vec2, wav2vec2_bert

* [run-slow] blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

* fix concatenation

* [run-slow] blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

* Update tests/test_processing_common.py

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>

* 🧹

* address comments

* clean up + tests

* [run-slow] instructblip, blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

---------

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
BernardZach pushed a commit to BernardZach/transformers that referenced this pull request Dec 5, 2024
@molbap @amyeroberts @BernardZach
Uniformize model processors (huggingface#31368)
73d3a6f 
* add initial design for uniform processors + align model

* add uniform processors for altclip + chinese_clip

* add uniform processors for blip + blip2

* fix mutable default 👀

* add configuration test

* handle structured kwargs w defaults + add test

* protect torch-specific test

* fix style

* fix

* rebase

* update processor to generic kwargs + test

* fix style

* add sensible kwargs merge

* update test

* fix assertEqual

* move kwargs merging to processing common

* rework kwargs for type hinting

* just get Unpack from extensions

* run-slow[align]

* handle kwargs passed as nested dict

* add from_pretrained test for nested kwargs handling

* [run-slow]align

* update documentation + imports

* update audio inputs

* protect audio types, silly

* try removing imports

* make things simpler

* simplerer

* move out kwargs test to common mixin

* [run-slow]align

* skip tests for old processors

* [run-slow]align, clip

* !$#@!! protect imports, darn it

* [run-slow]align, clip

* [run-slow]align, clip

* update common processor testing

* add altclip

* add chinese_clip

* add pad_size

* [run-slow]align, clip, chinese_clip, altclip

* remove duplicated tests

* fix

* add blip, blip2, bridgetower

Added tests for bridgetower which override common. Also modified common
tests to force center cropping if existing

* fix

* update doc

* improve documentation for default values

* add model_max_length testing

This parameter depends on tokenizers received.

* Raise if kwargs are specified in two places

* fix

* removed copied from

* match defaults

* force padding

* fix tokenizer test

* clean defaults

* move tests to common

* add missing import

* fix

* adapt bridgetower tests to shortest edge

* uniformize donut processor + tests

* add wav2vec2

* extend common testing to audio processors

* add testing + bert version

* propagate common kwargs to different modalities

* BC order of arguments

* check py version

* revert kwargs merging

* add draft overlap test

* update

* fix blip2 and wav2vec due to updates

* fix copies

* ensure overlapping kwargs do not disappear

* replace .pop by .get to handle duplicated kwargs

* fix copies

* fix missing import

* add clearly wav2vec2_bert to uniformized models

* fix copies

* increase number of features

* fix style

* [run-slow] blip, blip2, bridgetower, donut, wav2vec2, wav2vec2_bert

* [run-slow] blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

* fix concatenation

* [run-slow] blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

* Update tests/test_processing_common.py

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>

* 🧹

* address comments

* clean up + tests

* [run-slow] instructblip, blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

---------

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
BernardZach pushed a commit to innovationcore/transformers that referenced this pull request Dec 6, 2024
@molbap @amyeroberts @BernardZach
Uniformize model processors (huggingface#31368)
42265c1 
* add initial design for uniform processors + align model

* add uniform processors for altclip + chinese_clip

* add uniform processors for blip + blip2

* fix mutable default 👀

* add configuration test

* handle structured kwargs w defaults + add test

* protect torch-specific test

* fix style

* fix

* rebase

* update processor to generic kwargs + test

* fix style

* add sensible kwargs merge

* update test

* fix assertEqual

* move kwargs merging to processing common

* rework kwargs for type hinting

* just get Unpack from extensions

* run-slow[align]

* handle kwargs passed as nested dict

* add from_pretrained test for nested kwargs handling

* [run-slow]align

* update documentation + imports

* update audio inputs

* protect audio types, silly

* try removing imports

* make things simpler

* simplerer

* move out kwargs test to common mixin

* [run-slow]align

* skip tests for old processors

* [run-slow]align, clip

* !$#@!! protect imports, darn it

* [run-slow]align, clip

* [run-slow]align, clip

* update common processor testing

* add altclip

* add chinese_clip

* add pad_size

* [run-slow]align, clip, chinese_clip, altclip

* remove duplicated tests

* fix

* add blip, blip2, bridgetower

Added tests for bridgetower which override common. Also modified common
tests to force center cropping if existing

* fix

* update doc

* improve documentation for default values

* add model_max_length testing

This parameter depends on tokenizers received.

* Raise if kwargs are specified in two places

* fix

* removed copied from

* match defaults

* force padding

* fix tokenizer test

* clean defaults

* move tests to common

* add missing import

* fix

* adapt bridgetower tests to shortest edge

* uniformize donut processor + tests

* add wav2vec2

* extend common testing to audio processors

* add testing + bert version

* propagate common kwargs to different modalities

* BC order of arguments

* check py version

* revert kwargs merging

* add draft overlap test

* update

* fix blip2 and wav2vec due to updates

* fix copies

* ensure overlapping kwargs do not disappear

* replace .pop by .get to handle duplicated kwargs

* fix copies

* fix missing import

* add clearly wav2vec2_bert to uniformized models

* fix copies

* increase number of features

* fix style

* [run-slow] blip, blip2, bridgetower, donut, wav2vec2, wav2vec2_bert

* [run-slow] blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

* fix concatenation

* [run-slow] blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

* Update tests/test_processing_common.py

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>

* 🧹

* address comments

* clean up + tests

* [run-slow] instructblip, blip, blip_2, bridgetower, donut, wav2vec2, wav2vec2_bert

---------

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhuvanmdev bhuvanmdev bhuvanmdev left review comments

@SangbumChoi SangbumChoi SangbumChoi left review comments

@amyeroberts amyeroberts amyeroberts approved these changes

Assignees
No one assigned
Labels
run-slow
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@molbap @HuggingFaceDocBuilderDev @SangbumChoi @amyeroberts @bhuvanmdev