Add support for modular with fast image processors #35379

yonigozlan · 2024-12-21T00:34:44Z

What does this PR do?

Add support for using modular with fast image processors

Also add a fix for using modular on files other than modeling, which seem to have been broken by a recent change

Who can review?

yonigozlan · 2024-12-21T00:39:01Z

src/transformers/models/deformable_detr/modular_deformable_detr.py

+    def post_process_segmentation():
+        raise NotImplementedError("Segmentation post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_instance():
+        raise NotImplementedError("Instance post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_panoptic():
+        raise NotImplementedError("Panoptic post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_instance_segmentation():
+        raise NotImplementedError("Segmentation post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_semantic_segmentation():
+        raise NotImplementedError("Semantic segmentation post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_panoptic_segmentation():
+        raise NotImplementedError("Panoptic segmentation post-processing is not implemented for Deformable DETR yet.")


Here I wanted to remove these inherited functions. Somehow this works, but I'm not sure if there is a better way to do this?

Using modular the other way (with DetrImageProcessorFast inheriting from DeformableDetrImageProcessorFast) would solve the problem, but this wouldn't make sense chronologically.

This is the only way to "properly" do it!

yonigozlan · 2024-12-21T00:39:44Z

src/transformers/models/rt_detr/modular_rt_detr.py

+    def from_dict():
+        raise NotImplementedError("No need to override this method for RT-DETR yet.")
+
+    def post_process():
+        raise NotImplementedError("Post-processing is not implemented for RT-DETR yet.")
+
+    def post_process_segmentation():
+        raise NotImplementedError("Segmentation post-processing is not implemented for RT-DETR yet.")
+
+    def post_process_instance():
+        raise NotImplementedError("Instance post-processing is not implemented for RT-DETR yet.")
+
+    def post_process_panoptic():
+        raise NotImplementedError("Panoptic post-processing is not implemented for RT-DETR yet.")
+
+    def post_process_instance_segmentation():
+        raise NotImplementedError("Segmentation post-processing is not implemented for RT-DETR yet.")
+
+    def post_process_semantic_segmentation():
+        raise NotImplementedError("Semantic segmentation post-processing is not implemented for RT-DETR yet.")
+
+    def post_process_panoptic_segmentation():
+        raise NotImplementedError("Panoptic segmentation post-processing is not implemented for RT-DETR yet.")


yonigozlan · 2024-12-21T00:41:14Z

utils/create_dependency_mapping.py

+            if dep_name == node_name:
+                # Skip self dependencies for topological sort as they create cycles
+                continue


This is the case in ijepa for example, where we import IJepaConfig in modular

that's quite an edge case!

yonigozlan · 2024-12-21T00:42:42Z

utils/create_dependency_mapping.py

@@ -54,7 +59,7 @@ def extract_classes_and_imports(file_path):
    for node in ast.walk(tree):
        if isinstance(node, (ast.Import, ast.ImportFrom)):
            module = node.module if isinstance(node, ast.ImportFrom) else None
-            if module and (".modeling_" in module):
+            if module and (".modeling_" in module or "transformers.models" in module):


@ArthurZucker @Cyrilvallez This was changed in a recent PR, but seems to have broken modular for generating files other than modeling

yep, let's be more restrictive I think I was trying to avoid imports that are not from a modeling code directly, like models.auto etc. Can you make sure the order is still correct?

The order was off for instruct_blib_video, because llava_next was listed twice as a dep. I added a check right after this line to avoid this and now the order is the same as it was

HuggingFaceDocBuilderDev · 2024-12-21T01:02:13Z

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.

ArthurZucker

Thanks!

ArthurZucker · 2024-12-23T16:00:16Z

src/transformers/models/deformable_detr/modular_deformable_detr.py

+    def post_process_segmentation():
+        raise NotImplementedError("Segmentation post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_instance():
+        raise NotImplementedError("Instance post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_panoptic():
+        raise NotImplementedError("Panoptic post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_instance_segmentation():
+        raise NotImplementedError("Segmentation post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_semantic_segmentation():
+        raise NotImplementedError("Semantic segmentation post-processing is not implemented for Deformable DETR yet.")
+
+    def post_process_panoptic_segmentation():
+        raise NotImplementedError("Panoptic segmentation post-processing is not implemented for Deformable DETR yet.")


This is the only way to "properly" do it!

ArthurZucker · 2024-12-23T16:01:01Z

src/transformers/models/rt_detr/modular_rt_detr.py

+            ]
+        return encoded_inputs
+
+    # Copied from transformers.models.rt_detr.image_processing_rt_detr.RTDetrImageProcessor.post_process_object_detection


modular == no need for copied from -> you are missing an iinheritance here?

Yes ideally I would want to inherit this function from the slow rtdetr image processor, but that would mean I'd have to inherit from two different models and this doesn't seem to be supported.
I just removed the copied from for now.

ArthurZucker · 2024-12-23T16:01:36Z

utils/create_dependency_mapping.py

+            if dep_name == node_name:
+                # Skip self dependencies for topological sort as they create cycles
+                continue


that's quite an edge case!

ArthurZucker · 2024-12-23T16:02:25Z

utils/create_dependency_mapping.py

@@ -54,7 +59,7 @@ def extract_classes_and_imports(file_path):
    for node in ast.walk(tree):
        if isinstance(node, (ast.Import, ast.ImportFrom)):
            module = node.module if isinstance(node, ast.ImportFrom) else None
-            if module and (".modeling_" in module):
+            if module and (".modeling_" in module or "transformers.models" in module):


yep, let's be more restrictive I think I was trying to avoid imports that are not from a modeling code directly, like models.auto etc. Can you make sure the order is still correct?

ArthurZucker · 2024-12-23T16:03:45Z

utils/modular_model_converter.py

+        file_name_prefix = file_type.split("*")[0]
+        file_name_suffix = file_type.split("*")[-1] if "*" in file_type else ""
+        new_file_name = modular_file.replace("modular_", f"{file_name_prefix}_").replace(
+            ".py", f"{file_name_suffix}.py"
+        )


what is enabled by this? 🤗

Unlike other modeling files, fast image processing files have a suffix (_fast), so this is a way to account for that when writing or reading those files. It's also why I defined the file type for fast image processors as image_processing*_fast here

ArthurZucker · 2024-12-23T16:04:43Z

src/transformers/models/deformable_detr/modular_deformable_detr.py

+
+
+class DeformableDetrImageProcessorFast(DetrImageProcessorFast):
+    # Copied from transformers.models.deformable_detr.image_processing_deformable_detr.DeformableDetrImageProcessor.post_process


you should not have copied from in modular!

Yes sorry about that, same reason as for RT-DETR. I have removed them from now

Add support for modular with fast image processors

41fa0fb

yonigozlan commented Dec 21, 2024

View reviewed changes

yonigozlan requested a review from Cyrilvallez December 21, 2024 00:44

ArthurZucker reviewed Dec 23, 2024

View reviewed changes

fix order and remove copied from

9c98542

yonigozlan requested a review from ArthurZucker December 23, 2024 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for modular with fast image processors #35379

Add support for modular with fast image processors #35379

yonigozlan commented Dec 21, 2024

yonigozlan Dec 21, 2024 •

edited

Loading

ArthurZucker Dec 23, 2024

yonigozlan Dec 21, 2024

yonigozlan Dec 21, 2024

ArthurZucker Dec 23, 2024

yonigozlan Dec 21, 2024 •

edited

Loading

ArthurZucker Dec 23, 2024

yonigozlan Dec 23, 2024

HuggingFaceDocBuilderDev commented Dec 21, 2024

ArthurZucker left a comment

ArthurZucker Dec 23, 2024

ArthurZucker Dec 23, 2024

yonigozlan Dec 23, 2024

ArthurZucker Dec 23, 2024

ArthurZucker Dec 23, 2024

ArthurZucker Dec 23, 2024

yonigozlan Dec 23, 2024

ArthurZucker Dec 23, 2024

yonigozlan Dec 23, 2024



		class DeformableDetrImageProcessorFast(DetrImageProcessorFast):
		# Copied from transformers.models.deformable_detr.image_processing_deformable_detr.DeformableDetrImageProcessor.post_process

Add support for modular with fast image processors #35379

Are you sure you want to change the base?

Add support for modular with fast image processors #35379

Conversation

yonigozlan commented Dec 21, 2024

What does this PR do?

Who can review?

yonigozlan Dec 21, 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

yonigozlan Dec 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuggingFaceDocBuilderDev commented Dec 21, 2024

ArthurZucker left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yonigozlan Dec 21, 2024 •

edited

Loading

yonigozlan Dec 21, 2024 •

edited

Loading