Skip to content

OwlViT/Owlv2 post processing standardization - #34929

Merged
qubvel merged 12 commits into
huggingface:mainfrom
qubvel:owlvit-post-processing-standardization
Jan 17, 2025
Merged

OwlViT/Owlv2 post processing standardization#34929
qubvel merged 12 commits into
huggingface:mainfrom
qubvel:owlvit-post-processing-standardization

Conversation

@qubvel

@qubvel qubvel commented Nov 25, 2024

Copy link
Copy Markdown
Contributor

What does this PR do?

Standardize OwlViT and Owlv2 post-processing

  1. Add post_process_grounded_object_detection method for Processor (+ deprecation for post_process_object_detection method)
def post_process_grounded_object_detection(
    self,
    outputs,
    threshold: float = ...,
    target_sizes: Optional[Union[TensorType, List[Tuple]]] = None,
    text_labels: Union[List[str], List[List[str]]] = None,  # <------------- new keyword arg
)
  1. Add text_labels for post-processed output (if text_labels are provided as a keyword argument for post_process_grounded_object_detection). It's already mapped text names for detected bounding boxes.

For a full description regarding standardization see #34926

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 commented Nov 25, 2024

Copy link
Copy Markdown
Contributor Author

Hey @molbap! Please review if you have bandwidth!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not related changes, just refactoring

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

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.

@molbap molbap left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice standardization, just added a comment related to documentation :) also wondered if you could get away with a is_grounded boolean arg and just have one method, or even an Enum with either grounded, guided or default values, if that makes sense

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

would it make sense to add some documentation for post_process_image_guided_detection as well in addition to post_process_grounded_object_detection, just to show usage? Also, the underlying method isn't deprecated IIUC, so would need to still be documented

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks for the suggestion! Added documentation for the method in b13c68c

@qubvel

qubvel commented Dec 2, 2024

Copy link
Copy Markdown
Contributor Author

I would prefer to leave it as separate methods. This way, it is more consistent with other post-processing methods like post_process_object_detection, post_process_image_segmentation, etc., and there is no need to overcomplicate the code with more if/else code paths.

@qubvel
qubvel requested a review from ArthurZucker December 2, 2024 15:17
@qubvel

qubvel commented Dec 2, 2024

Copy link
Copy Markdown
Contributor Author

@ArthurZucker please review when you have bandwidth

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM sorry for putting it off for so long! 🤗

label = l[s > threshold]
box = b[s > threshold]
results.append({"scores": score, "labels": label, "boxes": box})
for scores, labels, boxes in zip(batch_scores, batch_labels, batch_boxes):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cool to move away from single letter variables

@qubvel
qubvel merged commit 94ae9a8 into huggingface:main Jan 17, 2025
bursteratom pushed a commit to bursteratom/transformers that referenced this pull request Jan 31, 2025
* Refactor owlvit post_process_object_detection + add text_labels

* Fix copies in grounding dino

* Sync with Owlv2 postprocessing

* Add post_process_grounded_object_detection method to processor, deprecate post_process_object_detection

* Add test cases

* Move text_labels to processors only

* [run-slow] owlvit owlv2

* [run-slow] owlvit, owlv2

* Update snippets

* Update docs structure

* Update deprecated objects for check_repo

* Update docstring for post processing of image guided object detection
elvircrn pushed a commit to elvircrn/transformers that referenced this pull request Feb 13, 2025
* Refactor owlvit post_process_object_detection + add text_labels

* Fix copies in grounding dino

* Sync with Owlv2 postprocessing

* Add post_process_grounded_object_detection method to processor, deprecate post_process_object_detection

* Add test cases

* Move text_labels to processors only

* [run-slow] owlvit owlv2

* [run-slow] owlvit, owlv2

* Update snippets

* Update docs structure

* Update deprecated objects for check_repo

* Update docstring for post processing of image guided object detection
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