Refactor image processor phi4
Author: yonigozlan
Pull request: yes

# What does this PR do?

Refactor phi4 image processor fast to better respect the standards.
Compared locally with the current image processor, with very little differences to report (mean of ~1e-7).
No tests yet as it would need a refactor of the base image processor tests to support fast only tests.
The vision integration tests still pass.

There is more optimization to be found probably (batch processing of images and patches, flattening patches in output instead of padding etc.), but I'd prefer to merge this first so that all fast image processors in the library respect the standards, as I'm planning to start a community issue to implement the rest of fast image processors.

Cc @Cyrilvallez @ArthurZucker


Comments:
Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. The CI will be paused while the PR is in draft mode. When it is ready for review, please click the `Ready for review` button (at the bottom of the PR page). This will assign reviewers.

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_36976). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Can we also add image processor tests? I follow the rule of adding tests whenever the model has non-100% copied processors

> Can we also add image processor tests? I follow the rule of adding tests whenever the model has non-100% copied processors

Yes agreed 100% this should have image tests. However since this is our first model with only a fast image processor, it might break our usual tests, so I should probably open another PR for that.

In the meantime I wanted to have this merged quite quickly so that we have good standards for all fast image processors in the library, especially with the community issue being open.


For now, adding common tests from the Mixin should be enough. They can already run tests for both the fast and slow processors, so if we leave out `slow_processor` and set `test_slow_image_processor=False`, it should work out of the box, cmiiw

As for edge cases, agreed, the test set can be extended later

> For now, adding common tests from the Mixin should be enough. They can already run tests for both the fast and slow processors, so if we leave out slow_processor and set test_slow_image_processor=False, it should work out of the box, cmiiw

Oh didn't know about `test_slow_image_processor=False`, that's great. I'll try!

Hey @zucchini-nlp ! I'm having issues getting the integration tests to pass, looks like there's no checkpoint on the hub that work with the recent changes in this PR  https://github.com/huggingface/transformers/pull/36996 ? Tried PR 55 and 56 here https://huggingface.co/microsoft/Phi-4-multimodal-instruct but none of them work...

@yonigozlan hm, what are the error you got? I feel like the two PRs have to be merged together correctly to get it running

@zucchini-nlp Just saw your comment here https://huggingface.co/microsoft/Phi-4-multimodal-instruct/discussions/56, I'll wait for both PR to be merged then 🤗

Hey @zucchini-nlp @Cyrilvallez ! Coming back to this PR after a while 😅. Looks like the integration tests are broken on main. Changing the revision to Cyril's PR on the hub and updating the placeholder tokens in the tests seems to fix the issue. I'll merge if you don't see any issues with these updates!