Support for transformers v5 post_process functions #1386

shaddu · 2024-07-20T01:22:54Z

Description

post_process_segmentation is getting depreciated in transformers version 5. Code changes added are to support post_process_semantic_segmentation

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)

How has this change been tested, please provide a testcase or example of how you tested the change?

Detection test Colab

Any specific deployment considerations

No

CLAassistant · 2024-07-20T01:23:00Z

All committers have signed the CLA.

SkalskiP · 2024-07-22T12:09:35Z

Hi @shaddu 👋🏻 Thank you for submitting the PR. I have a few questions regarding the scope of the changes that have been and will need to be made in from_transformers to fully migrate us. In #1113 I see that we need to migrate post_process, post_process_panoptic, post_process_segmentation and post_process_instance. Do I understand correctly that this PR only migrates post_process_segmentation for now, and the remaining outputs still need to be migrated?

shaddu · 2024-07-22T12:50:27Z

Hi @SkalskiP , Thank you for reviewing the PR and providing your comments. The scope was to migrate post_process, post_process_panoptic, post_process_segmentation and post_process_instance but I have recently started working on computer vision, so I wanted to make sure before making all the changes that I am in the right direction. If this code is fine then I can migrate the rest of the method in this PR only or another as you prefer.

SkalskiP · 2024-07-22T13:23:08Z

@shaddu I like what you have done so far! Both code quality and functionality look good to me. From my perspective, the most important thing is that from_transformers supports both v4 and v5.

I think you can continue the migration in this PR.

…ntation + post_process_instance_segmentation

shaddu · 2024-07-27T22:30:46Z

Hi @SkalskiP ,

I have finished adding support for the remaining function. Could you please review it and offer feedback?

Thank you

LinasKo · 2024-07-29T06:42:25Z

Hi @shaddu

Thank you for your contribution! I'll check this today; hopefully we can merge soon.

SkalskiP · 2024-07-29T06:32:31Z

supervision/detection/core.py

@@ -483,11 +484,40 @@ class names. If provided, the resulting Detections object will contain
            Class names values can be accessed using `detections["class_name"]`.


We need to update the from_transformers docstring.

Creates a Detections instance from object detection or segmentation Transformer inference result.

I'd mention we support object detection as well as panoptic, semantic and instance segmentation results.

transformers_results (dict): The output of Transformers model inference. A dictionary containing the scores, labels, boxes and masks keys.

Now it can also be a Tensor.

Hi @SkalskiP , One quick query, ideally it should now be transformers_results (Union[dict, torch.Tensor]) but to support this we need to import the torch package, should we add the import or leave it in docstrings for now?

SkalskiP · 2024-07-29T06:44:03Z

supervision/detection/core.py

@@ -483,11 +484,40 @@ class names. If provided, the resulting Detections object will contain
            Class names values can be accessed using `detections["class_name"]`.
        """  # noqa: E501 // docs

-        class_ids = transformers_results["labels"].cpu().detach().numpy().astype(int)
        data = {}


In from_transformers, there are four places where we do:

if id2label is not None: class_names = np.array([id2label[class_id] for class_id in class_ids]) data[CLASS_NAME_DATA_FIELD] = class_names

I'd wrap it in a local helper function. It can be defined inside from_transformers.

def get_data(class_ids: np.ndarray, id2label: Optional[Dict[int, str]]) -> dict: data = {} if id2label is not None: class_names = np.array([id2label[class_id] for class_id in class_ids]) data[CLASS_NAME_DATA_FIELD] = class_names return data

SkalskiP · 2024-07-29T06:45:57Z

supervision/detection/core.py

+                    mask=masks,
+                    class_id=class_ids,
+                    data=data,
+                )
        else:


I think we can drop this else statement.

SkalskiP · 2024-07-29T06:48:06Z

supervision/detection/core.py

+
+        if transformers_results.__class__.__name__ == "Tensor":
+            segmentation_array = transformers_results.cpu().detach().numpy()
+


Let's make it more compact and drop those extra new lines.

SkalskiP · 2024-07-29T06:49:05Z

supervision/detection/utils.py

+    Convert a PNG byte string to a binary mask array.
+
+    Args:
+    - png_string (bytes): A byte string representing the PNG image.


This docstring format is incompatible with rest of docstings in the project.

SkalskiP · 2024-07-29T06:49:46Z

supervision/detection/utils.py

+    """
+    image = Image.open(io.BytesIO(png_string))
+    mask = np.array(image, dtype=np.uint8)
+


Let's drop this extra new line.

SkalskiP · 2024-07-29T06:56:02Z

supervision/detection/core.py

@@ -504,6 +534,60 @@ class names. If provided, the resulting Detections object will contain
                class_id=class_ids,
                data=data,
            )
+        elif "segments_info" in transformers_results:


Personally, I would move segments_info = transformers_results["segments_info"] extraction to if "segmentation" in transformers_results: and elif "png_string" in transformers_results: blocks. We will get one indention less.

SkalskiP · 2024-07-29T07:02:49Z

Hi @shaddu 👋🏻 Awesome work here. I left some comments regarding the current version of the code, but here are some high-level overviews ones:

Could you update your Colab? The current version tests only a limited subset of supported capabilities. It would be awesome to test all functions mentioned in Update to support new from_transformers methods #1113 in a single Colab.
In my opinion, having all that logic in a single function is hard to follow. I would create a new file, supervision/detection/transformers.py, and inside define process_transformers_v5_result, process_transformers_v4_detection_result, process_transformers_v4_segmentation_result, etc, and just call those inside from_transformers in supervision/detection/core.py. This will:
- Make supervision/detection/core.py shorter. It is already too long.
- Make transformers' logic organized and easier to maintain.

In general this looks really good already. Thanks a lot for all the help! 🙏🏻

LinasKo · 2024-07-29T07:04:52Z

Hi @shaddu,

Here's a Colab that may help as a starting point. I've got examples of every segmentation type, except for instance segmentation.

It would be great to also test with a few other models - not just a panoptic segmentation one.

Lastly, I'm curious: does panoptic segmentation add anything to the detections.data field?

shaddu · 2024-07-30T19:32:28Z

Hey @SkalskiP,

Thank you for the detailed feedback. The suggestion of a separate file is a good idea. I'll work on this approach.

Hey @LinasKo,

Thank you for sharing the new collab. I learned something new today on how to use your repos in collab notebook 👍

shaddu · 2024-08-03T23:41:29Z

Hi @SkalskiP / @LinasKo ,

I have made changes as per the feedback, can you please review the PR again?

@LinasKo post_process_panoptic_segmentation adds class names in detection.data.

I will be adding more models and test cases in this colab for review.

SkalskiP · 2024-08-05T13:57:36Z

Hi @shaddu 👋🏻 ! Awesome work! I added small changes - changed names of functions and improved docstrings. We are ready to merge! Thanks a lot for the contribution! 🙏🏻 Supporting transformers is one of our goals; your contribution is a big step in that direction.

shaddu · 2024-08-05T21:30:53Z

@SkalskiP Thank you for merging the PR. It was a great learning experience. I'll look for more issues to work on in Supervision and if you come across any related issues, I'd be happy to help with them.

SkalskiP · 2024-08-06T07:02:52Z

@shaddu, working with you was an awesome experience! 🔥

shaddu and others added 2 commits July 20, 2024 02:08

Support for post_process_semantic_segmentation added

b5ea2cd

Merge branch 'roboflow:develop' into develop

2432040

fix(pre_commit): 🎨 auto format pre-commit hooks

e65892c

shaddu mentioned this pull request Jul 20, 2024

Update to support new from_transformers methods #1113

Closed

shaddu and others added 3 commits July 27, 2024 23:17

Merge branch 'roboflow:develop' into develop

1a1a2d2

support added for post_process_panoptic + post_process_panoptic_segme…

4ce9d76

…ntation + post_process_instance_segmentation

fix(pre_commit): 🎨 auto format pre-commit hooks

ac1fa2e

E501 Line too long (95 > 88) fix

3bb384d

SkalskiP requested changes Jul 29, 2024

View reviewed changes

LinasKo changed the title ~~Support for post_process_semantic_segmentation added~~ Support for transformers v5 post_process functions Jul 29, 2024

Merge branch 'roboflow:develop' into develop

7bba367

Merge branch 'roboflow:develop' into develop

f95f81e

LinasKo mentioned this pull request Aug 1, 2024

Added rtdetr method #1421

Closed

4 tasks

shaddu and others added 7 commits August 1, 2024 21:32

Merge branch 'roboflow:develop' into develop

f6437ab

code refactor for transformers v5 support

7c7c3d8

fix(pre_commit): 🎨 auto format pre-commit hooks

fd568e2

Merge branch 'roboflow:develop' into develop

a5ae72d

cyclic dependency fix + code refactor

da261ff

fix(pre_commit): 🎨 auto format pre-commit hooks

dacd8f5

support of optional mask in convert_to_detections

2c235df

SkalskiP and others added 7 commits August 5, 2024 14:23

small refactor and docs improvement

8a82f4a

fix(pre_commit): 🎨 auto format pre-commit hooks

9fc6656

small refactor and docs improvement

1d3e423

Merge remote-tracking branch 'origin/develop' into develop

fde9cd4

fix(pre_commit): 🎨 auto format pre-commit hooks

b6547ba

ready for tests

7c038f1

fix(pre_commit): 🎨 auto format pre-commit hooks

b2752ac

SkalskiP merged commit aa6c673 into roboflow:develop Aug 5, 2024
9 checks passed

LinasKo mentioned this pull request Aug 27, 2024

supervision-0.23.0 release #1486

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for transformers v5 post_process functions #1386

Support for transformers v5 post_process functions #1386

shaddu commented Jul 20, 2024

CLAassistant commented Jul 20, 2024 •

edited

Loading

SkalskiP commented Jul 22, 2024

shaddu commented Jul 22, 2024

SkalskiP commented Jul 22, 2024

shaddu commented Jul 27, 2024

LinasKo commented Jul 29, 2024 •

edited

Loading

SkalskiP Jul 29, 2024

shaddu Aug 2, 2024

SkalskiP Jul 29, 2024

SkalskiP Jul 29, 2024

SkalskiP Jul 29, 2024

SkalskiP Jul 29, 2024

SkalskiP Jul 29, 2024

SkalskiP Jul 29, 2024

SkalskiP commented Jul 29, 2024

LinasKo commented Jul 29, 2024 •

edited

Loading

shaddu commented Jul 30, 2024

shaddu commented Aug 3, 2024

SkalskiP commented Aug 5, 2024 •

edited

Loading

shaddu commented Aug 5, 2024

SkalskiP commented Aug 6, 2024

		@@ -483,11 +484,40 @@ class names. If provided, the resulting Detections object will contain
		Class names values can be accessed using `detections["class_name"]`.


		if transformers_results.__class__.__name__ == "Tensor":
		segmentation_array = transformers_results.cpu().detach().numpy()

Support for transformers v5 post_process functions #1386

Support for transformers v5 post_process functions #1386

Conversation

shaddu commented Jul 20, 2024

Description

Type of change

How has this change been tested, please provide a testcase or example of how you tested the change?

Any specific deployment considerations

CLAassistant commented Jul 20, 2024 • edited Loading

SkalskiP commented Jul 22, 2024

shaddu commented Jul 22, 2024

SkalskiP commented Jul 22, 2024

shaddu commented Jul 27, 2024

LinasKo commented Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SkalskiP commented Jul 29, 2024

LinasKo commented Jul 29, 2024 • edited Loading

shaddu commented Jul 30, 2024

shaddu commented Aug 3, 2024

SkalskiP commented Aug 5, 2024 • edited Loading

shaddu commented Aug 5, 2024

SkalskiP commented Aug 6, 2024

CLAassistant commented Jul 20, 2024 •

edited

Loading

LinasKo commented Jul 29, 2024 •

edited

Loading

LinasKo commented Jul 29, 2024 •

edited

Loading

SkalskiP commented Aug 5, 2024 •

edited

Loading