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

OmDet Turbo processor standardization #34937

Merged
merged 13 commits into from
Jan 17, 2025

Conversation

qubvel
Copy link
Member

@qubvel qubvel commented Nov 26, 2024

What does this PR do?

Standardize post-processing for OmDet Turbo model (see #34926 for more info)

  1. Rename score_threshold to threshold argument in post_process_grounded_object_detection
  2. Rename classes to text_labels argument in post_process_grounded_object_detection and make it optional
  3. Rename classes key to text_labels in post-processed output dictionary
  4. Add labels (class indexes) key to post-processed output dictionary

The signature is going to be:

    def post_process_grounded_object_detection(
        self,
        outputs: "OmDetTurboObjectDetectionOutput",
        text_labels: Optional[Union[List[str], List[List[str]]]] = None,           # <--------- renamed: classes -> text_labels + Optional now
        threshold: float = 0.3,                                                    # <--------- renamed: score_threshold -> threshold
        nms_threshold: float = 0.5,
        target_sizes: Optional[Union[TensorType, List[Tuple]]] = None,
        max_num_det: Optional[int] = None,
    )

The output is going to be:

[
  {
      "boxes": torch.tensor of shape (N, 4),
      "labels": torch.tensor of shape (N,),                                       # <-------- new key, int indices, consistent with other object detection outputs
      "scores": torch.tensor of shape (N,),
      "text_labels": list of str names of len N (previously classes) or `None`    # <-------- renamed: classes -> text_labels
  },
  ...
]

"classes" key is also available, it will return text_labels and issue a warning

TODO:

  • Add deprecation for dict key

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?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@qubvel qubvel marked this pull request as draft November 26, 2024 12:52
@HuggingFaceDocBuilderDev

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.

@qubvel qubvel marked this pull request as ready for review November 26, 2024 17:22
@qubvel qubvel requested a review from yonigozlan November 26, 2024 17:22
@qubvel
Copy link
Member Author

qubvel commented Nov 26, 2024

cc @yonigozlan for review if you have bandwidth

Copy link
Member

@yonigozlan yonigozlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much better thanks for refactoring! 🤗

Co-authored-by: Yoni Gozlan <74535834+yonigozlan@users.noreply.github.com>
@qubvel qubvel requested a review from ArthurZucker December 2, 2024 14:57
@qubvel
Copy link
Member Author

qubvel commented Dec 2, 2024

@ArthurZucker please review when you have bandwidth

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🤗 sorry for the delay

@@ -55,11 +69,23 @@ class OmDetTurboProcessorKwargs(ProcessingKwargs, total=False):
}


if is_torch_available():
import torch
class _dict_with_warning(dict):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use camel casing please!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2d7a6b2

)
result = _dict_with_warning({"boxes": boxes, "scores": scores, "labels": labels, "text_labels": None})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not 100% we need this (the class is a bit over engineered) but alright

@qubvel qubvel merged commit 42b2857 into huggingface:main Jan 17, 2025
16 checks passed
bursteratom pushed a commit to bursteratom/transformers that referenced this pull request Jan 31, 2025
* Fix docstring

* Fix docstring

* Add `classes_structure` to model output

* Update omdet postprocessing

* Adjust tests

* Update code example in docs

* Add deprecation to "classes" key in output

* Types, docs

* Fixing test

* Fix missed clip_boxes

* [run-slow] omdet_turbo

* Apply suggestions from code review

Co-authored-by: Yoni Gozlan <74535834+yonigozlan@users.noreply.github.com>

* Make CamelCase class

---------

Co-authored-by: Yoni Gozlan <74535834+yonigozlan@users.noreply.github.com>
elvircrn pushed a commit to elvircrn/transformers that referenced this pull request Feb 13, 2025
* Fix docstring

* Fix docstring

* Add `classes_structure` to model output

* Update omdet postprocessing

* Adjust tests

* Update code example in docs

* Add deprecation to "classes" key in output

* Types, docs

* Fixing test

* Fix missed clip_boxes

* [run-slow] omdet_turbo

* Apply suggestions from code review

Co-authored-by: Yoni Gozlan <74535834+yonigozlan@users.noreply.github.com>

* Make CamelCase class

---------

Co-authored-by: Yoni Gozlan <74535834+yonigozlan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants