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

Update to support new from_transformers methods #1113

Closed
LinasKo opened this issue Apr 12, 2024 · 10 comments
Closed

Update to support new from_transformers methods #1113

LinasKo opened this issue Apr 12, 2024 · 10 comments
Assignees

Comments

@LinasKo
Copy link
Contributor

LinasKo commented Apr 12, 2024

Description

Supervision contains the function from_transformers that includes the results of a Hugging Face transformer and converts it into Detections.

Up until now, we were recommending users to call one of two transformers functions:

  1. post_process_segmentation for any segmentation task
  2. post_process for detection.

Reading through the code of transformers, it appears both are being deprecated and will be removed in version 5. At the time of writing, the latest is v4.39.3, with the first v4 release in late 2020.

Let's make sure we support the new version! Detections object is universal - it can contain both masks and segmentation. Let's change the from_transformers method so it checks its inputs, determines which function was called formats the output, and builds the Detections object like we're doing now.

Additional

  • Note: Please share a Google Colab with minimal code to test the new feature. We know it's additional work, but it will speed up the review process. The reviewer must test each change. Setting up a local environment to do this is time-consuming. Please ensure that Google Colab can be accessed without any issues (make it public). Thank you! 🙏🏻
@LinasKo
Copy link
Contributor Author

LinasKo commented Apr 12, 2024

You can find examples of the new and old function calls in this Colab

@shaddu
Copy link
Contributor

shaddu commented Jul 6, 2024

@LinasKo I was checking on this issue and below are my observations :

  • in transformer code image_processing_detr.py there are four functions that are getting depreciated in v5
Old (v4) New (v5)
post_process post_process_object_detection
post_process_panoptic post_process_panoptic_segmentation
post_process_segmentation post_process_semantic_segmentation
post_process_instance post_process_instance_segmentation
  • I checked locally and in official documentation also and found that post_process_object_detection is working fine
  • For post_process_panoptic method as shown in shared collab , I can see that the method returns List[Dict] each dictionary containing a PNG string and segments_info which is not supported by method from_transformers in current version also.
  • For post_process_segmentation in v5, the new method returns totally different return type and we should change from_transformers but you have mentioned in this issue that we need to change from_tensors. If you could add some more info then I would like to work on this issue
  • Same for post_process_instance

@LinasKo
Copy link
Contributor Author

LinasKo commented Jul 15, 2024

Hi @shaddu 👋

That's an analysis we vitally needed. Thank you very much!

from_tensors was a typo and I've now fixed it.
I very much appreciate you clarifying it before jumping in.

If you still have the time, I'd gladly assign the issue to you. Is there any more information you'd like?

@shaddu
Copy link
Contributor

shaddu commented Jul 15, 2024

@LinasKo ,

Thank you for the response. I would like to work on the issue. One assumption I'm making is that the Detection class is universal, so we should avoid making any changes to it and only modify the from_transformers method.

@LinasKo
Copy link
Contributor Author

LinasKo commented Jul 15, 2024

I've assigned the ticket to you. Let us know how it goes!

The assumption is true, however there's some flexibility - Detections.data allows you to include additional data from the model. E.g. if there was an extra set of classes we'd like to store, we'd create an entry there (probably defining the key in supervision/config.py)

You have a very thoughtful approach to the problem. If you see an obstacle that can't be solved without modifying core classes - I trust your judgment; feel free to make the changes and we can discuss in the PR.

I'm away from Wednesday, but you may have some luck catching @SkalskiP if you have more questions.

@shaddu
Copy link
Contributor

shaddu commented Jul 20, 2024

Hi @SkalskiP / @LinasKo ,

I have created a new PR to add support for post_process_semantic_segmentation method. PR reference #1386. I’m open to any feedback or reviews you have.

@SkalskiP
Copy link
Collaborator

Hi @shaddu 👋🏻 Thank you for your time and dedication in migrating from_transformers to the new Transformers API. @LinasKo is currently on vacation, so I will take care of reviewing your PR.

@shaddu
Copy link
Contributor

shaddu commented Jul 23, 2024

Hi @SkalskiP, I need a quick confirmation about post_process_panoptic. This method from v4 transformers is not supported in the from_transformers method. Should we add support for it, or just for the v5 version post_process_panoptic_segmentation?

@SkalskiP
Copy link
Collaborator

Hi @shaddu 👋🏻 that was not part of the original task, but if you have the time to make it happen, I'd love it!

@SkalskiP
Copy link
Collaborator

SkalskiP commented Aug 5, 2024

Implemented via #1386. Closing the issue.

@SkalskiP SkalskiP closed this as completed Aug 5, 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

No branches or pull requests

3 participants