Skip to content

Add Fast Image Processor for ImageGPT #39592

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
Merged
merged 38 commits into from
Sep 4, 2025
Merged

Add Fast Image Processor for ImageGPT #39592

merged 38 commits into from
Sep 4, 2025
+407 −93

Conversation

agamjots05
Copy link
Contributor

@agamjots05 agamjots05 commented Jul 22, 2025 •
edited
Loading

What does this PR do?

Related #36978

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@Rocketknight1
Copy link
Member

cc @yonigozlan

@yonigozlan
Copy link
Member

Hey @agamjots05 and @ayaayethan ! Thanks for opening this PR, don't hesitate to ping me when it's ready to review or if you need help!

@agamjots05
Copy link
Contributor Author

Hey @agamjots05 and @ayaayethan ! Thanks for opening this PR, don't hesitate to ping me when it's ready to review or if you need help!

Thank you for letting us know! We'll let you know if we get stuck on anything 👍

@yonigozlan yonigozlan mentioned this pull request Aug 11, 2025
Open
79 tasks
@agamjots05
Copy link
Contributor Author

agamjots05 commented Aug 12, 2025 •
edited
Loading

@yonigozlan Quick question
My team and I have made the fast processor for ImageGPT and have come across an interesting issue in regards to the testing file for ImageGPT.

To get the final two slow/fast parity tests to pass, we needed to make two very small changes in the slow ImageGPTImageProcessor:

  1. Always include pixel_values in the returned BatchFeature even when do_color_quantize=True (alongside input_ids).
    This avoids KeyError/AttributeError in the shared test mixins that access encoding.pixel_values. It’s additive-only.
  2. Override to_dict() to convert clusters from np.ndarray to a plain Python list.
    This makes the config JSON-serializable, which allows us to pass a test we were failing

Would these two minimal adjustments to the slow processor be acceptable?
If not should we, for the time being, ignore these failing test cases occurring due to the base processor?

@agamjots05 agamjots05 marked this pull request as ready for review August 13, 2025 18:04
@github-actions github-actions bot requested review from ydshieh and yonigozlan August 13, 2025 18:05
@yonigozlan
Copy link
Member

Hey @agamjots05 !
These seems like reasonable changes, strange that it didn't cause any issues before, thanks for fixing!

@agamjots05
Copy link
Contributor Author

Hey @agamjots05 ! These seems like reasonable changes, strange that it didn't cause any issues before, thanks for fixing!

Sounds good. If that's the case, then this PR should be ready for review 👍

yonigozlan
yonigozlan reviewed Aug 15, 2025
View reviewed changes
Copy link
Member

@yonigozlan yonigozlan left a comment

Choose a reason for hiding this comment

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

Hey @agamjots05 and @ayaayethan , thanks for contributing! There's a few changes to do, and I'll be off for two weeks, but happy to do another review when I'm back!

@agamjots05
Copy link
Contributor Author

@yonigozlan I believe I have implemented all the changes you have requested. Please take a look when you can 👍

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 4, 2025

[For maintainers] Suggested jobs to run (before merge)

run-slow: auto, imagegpt

yonigozlan
yonigozlan approved these changes Sep 4, 2025
View reviewed changes
Copy link
Member

@yonigozlan yonigozlan left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @agamjots05 and @ayaayethan !
I just pushed some small simplifications to the color quantize logic, and I'll merge once the CI passes!

@yonigozlan yonigozlan enabled auto-merge (squash) September 4, 2025 22:37
@yonigozlan yonigozlan merged commit 4a88e81 into huggingface:main Sep 4, 2025
23 checks passed
@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.

snorkelopstesting2-coder pushed a commit to snorkel-marlin-repos/huggingface_transformers_pr_39592_23238a28-91c4-4655-8b68-c30889921584 that referenced this pull request Oct 11, 2025
Add Fast Image Processor for ImageGPT
3619d2a 
Original PR #39592 by agamjots05
Original: huggingface/transformers#39592
@snorkelopstesting2-coder snorkelopstesting2-coder mentioned this pull request Oct 11, 2025
Merged
snorkelopstesting2-coder added a commit to snorkel-marlin-repos/huggingface_transformers_pr_39592_23238a28-91c4-4655-8b68-c30889921584 that referenced this pull request Oct 11, 2025
@snorkelopstesting2-coder
Merge pull request #1: Add Fast Image Processor for ImageGPT
583d568 
Merged from original PR #39592
Original: huggingface/transformers#39592
snorkelopstesting2-coder pushed a commit to snorkel-marlin-repos/huggingface_transformers_pr_39592_4c61250d-7b82-48a4-91e2-bb28ade47661 that referenced this pull request Oct 11, 2025
Add Fast Image Processor for ImageGPT
f6de35f 
Original PR #39592 by agamjots05
Original: huggingface/transformers#39592
@snorkelopstesting2-coder snorkelopstesting2-coder mentioned this pull request Oct 11, 2025
Merged
snorkelopstesting2-coder added a commit to snorkel-marlin-repos/huggingface_transformers_pr_39592_4c61250d-7b82-48a4-91e2-bb28ade47661 that referenced this pull request Oct 11, 2025
@snorkelopstesting2-coder
Merge pull request #1: Add Fast Image Processor for ImageGPT
3edec36 
Merged from original PR #39592
Original: huggingface/transformers#39592
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yonigozlan yonigozlan yonigozlan approved these changes

@ydshieh ydshieh Awaiting requested review from ydshieh

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@agamjots05 @Rocketknight1 @yonigozlan @HuggingFaceDocBuilderDev @ayaayethan @updawg420