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.

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

Faster image processor #31236

Closed
wants to merge 9 commits into from
Closed

Conversation

g1y5x3
Copy link
Contributor

@g1y5x3 g1y5x3 commented Jun 4, 2024

What does this PR do?

My attempt to #31205, most of the work were already done in #28221. I just added the adoption in detr and include new tests in both videomae and detr as suggested.

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?

@amyeroberts @ikergarcia1996
The tests might be a bit artificial would love to switch it to a more realistic use case, e.g maybe make a list of actual videos and images. The image processor from videomae didn't really triggered the recursive check but the one from detr did. Let me know if this is what you are looking for and then I will fix the rest of detr variants.

@amyeroberts
Copy link
Collaborator

Hi @g1y5x3, thanks for opening this PR and apologies for the delay in responding.

In terms of these transforms, we'll need to add some additional logic. At the moment, doing np.array before passing into BatchFeature assumes that the input can be batched i.e. the images are of the same size. We'll need to add additional logic checks which ensure this still passes when e.g. do_resize=False

@g1y5x3
Copy link
Contributor Author

g1y5x3 commented Jul 2, 2024

Hi, no worries. I just need to reload the cache on what exactly I did and run some benchmarks locally to understand the need of that logic better. When I was writing this PR, I didn't notice the speed difference without it but it wasn't anywhere precise but just a feeling.

Copy link

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.

@github-actions github-actions bot closed this Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants