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

Add Fast Grounding-Dino Processor #37108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

keetrap
Copy link
Contributor

@keetrap keetrap commented Mar 29, 2025

Related #36978
cc @yonigozlan

@github-actions github-actions bot marked this pull request as draft March 29, 2025 19:40
Copy link

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 and trigger CI.

@keetrap keetrap marked this pull request as ready for review March 29, 2025 19:45
@github-actions github-actions bot requested review from ydshieh and yonigozlan March 29, 2025 19:46
@yonigozlan
Copy link
Member

Hi @keetrap, same comment as for Conditional DETR about using modular if possible, thanks!

@keetrap
Copy link
Contributor Author

keetrap commented Apr 1, 2025

@yonigozlan To make the modular grounding_dino implementation work, we need to implement the following changes:

Required Changes:

  • Update the naming convention from grounding-dino to grounding_dino for consistency
  • Rename docs/source/en/model_doc/grounding-dino.md to grounding_dino.md.
  • Modify files (configuration_auto.py, image_processing_auto.py, modeling_auto.py, processing_auto.py, tokenization_auto.py) to updated naming convention.

Are you up for these changes?

@yonigozlan
Copy link
Member

@keetrap Why are these changes needed? Are you getting an error when using modular? It would be great to have a traceback of the error to see if we can fix it instead of changing the naming convention which might cause some backward compatibility issue

@keetrap
Copy link
Contributor Author

keetrap commented Apr 7, 2025

@yonigozlan Yes I am getting an error when using modular, to be specific when we run python utils/check_modular_conversion.py
traceback ->

File "C:\Users\parte\Desktop\Open_Source\transformers\utils\check_modular_conversion.py", line 151, in <module>
   non_matching_files += compare_files(modular_file_path, args.fix_and_overwrite)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\utils\check_modular_conversion.py", line 58, in compare_files
   generated_modeling_content = convert_modular_file(modular_file_path)
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\utils\modular_model_converter.py", line 1689, in convert_modular_file
   wrapper.visit(cst_transformers)
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\metadata\wrapper.py", line 204, in visit
   return self.module.visit(visitor)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\module.py", line 89, in visit
   result = super(Module, self).visit(visitor)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\base.py", line 233, in visit
   visitor.on_leave(self)
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_visitors.py", line 137, in on_leave
   leave_func(original_node)
 File "C:\Users\parte\Desktop\Open_Source\transformers\utils\modular_model_converter.py", line 1313, in leave_Module
   renamed_module = module.visit(renamer)
                    ^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\module.py", line 89, in visit
   result = super(Module, self).visit(visitor)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\base.py", line 227, in visit
   _CSTNodeSelfT, self._visit_and_replace_children(visitor)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\module.py", line 74, in _visit_and_replace_children
   body=visit_body_sequence(self, "body", self.body, visitor),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\internal.py", line 227, in visit_body_sequence
   return tuple(visit_body_iterable(parent, fieldname, children, visitor))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\internal.py", line 193, in visit_body_iterable
   new_child = child.visit(visitor)
               ^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\base.py", line 227, in visit
   _CSTNodeSelfT, self._visit_and_replace_children(visitor)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\statement.py", line 442, in _visit_and_replace_children
   body=visit_sequence(self, "body", self.body, visitor),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\internal.py", line 177, in visit_sequence
   return tuple(visit_iterable(parent, fieldname, children, visitor))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\internal.py", line 159, in visit_iterable
   new_child = child.visit(visitor)
               ^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\base.py", line 227, in visit
   _CSTNodeSelfT, self._visit_and_replace_children(visitor)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\statement.py", line 1372, in _visit_and_replace_children
   module=visit_optional(self, "module", self.module, visitor),
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\internal.py", line 110, in visit_optional
   result = node.visit(visitor)
            ^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\base.py", line 236, in visit
   leave_result = visitor.on_leave(self, with_updated_children)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\matchers\_visitors.py", line 519, in on_leave
   retval = CSTTransformer.on_leave(self, original_node, updated_node)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_visitors.py", line 71, in on_leave
   updated_node = leave_func(original_node, updated_node)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\utils\modular_model_converter.py", line 135, in leave_Name
   return self._replace_name(original_node, updated_node)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\utils\modular_model_converter.py", line 127, in _replace_name
   return updated_node.with_changes(value=update)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\base.py", line 324, in with_changes
   return replace(self, **changes)
          ^^^^^^^^^^^^^^^^^^^^^^^^
 File "C:\Users\parte\AppData\Roaming\uv\python\cpython-3.11.11-windows-x86_64-none\Lib\dataclasses.py", line 1503, in replace
   return obj.__class__(**changes)
          ^^^^^^^^^^^^^^^^^^^^^^^^
 File "<string>", line 6, in __init__
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\base.py", line 117, in __post_init__
   self._validate()
 File "C:\Users\parte\Desktop\Open_Source\transformers\.venv\Lib\site-packages\libcst\_nodes\expression.py", line 355, in _validate
   raise CSTValidationError(f"Name {self.value!r} is not a valid identifier.")
libcst._nodes.base.CSTValidationError: Name 'image_processing_grounding-dino' is not a valid identifier.

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.

2 participants