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

SanitizeBoundingBox based on minimum area #7735

Merged
merged 8 commits into from
Jun 4, 2024

Conversation

antoinebrl
Copy link
Contributor

@antoinebrl antoinebrl commented Jul 11, 2023

During transformations, bounding boxes can be filtered based on minimum size and area.

cc @vfdev-5

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 11, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/7735

Note: Links to docs will display an error until the docs builds have been completed.

❌ 16 New Failures, 14 Unrelated Failures

As of commit 73212ca with merge base f7d9e75 (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot
Copy link

Hi @antoinebrl!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@NicolasHug
Copy link
Member

Thanks for the PR @antoinebrl , I'm curious about the applications where you found this feature useful?

@antoinebrl
Copy link
Contributor Author

antoinebrl commented Jul 11, 2023

Hi @NicolasHug. Filtering by either size and/or area allows for more control when selecting bounding boxes with different aspect ratios. The smallest edge of a rectangle can be smaller than the sides of a square but the rectangle can occupy more space.

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@NicolasHug
Copy link
Member

Isn't passing min_area=N strictly equivalent to passing min_size=sqrt(N)?

@antoinebrl
Copy link
Contributor Author

If you have square bounding boxes this would be equivalent indeed. However, if there are rectangles with one side ~3-4x longer than the other one, they cover a much larger area than squares with the same small edge. One might want to drop the small squares and keep the rectangles. This effect can be accentuated when resizing images without keeping the aspect ratio.

@NicolasHug
Copy link
Member

NicolasHug commented Jul 12, 2023

To be clear, min_size will drop a bbox if any of H or W is lower than min_size. So passing min_area=N is equivalent to passing min_size=sqrt(N).

taking N = a * b, the only way for a * b to be lower than N is for either a or b to be less than sqrt(N). I.e. both parametrizations are equivalent. (here, N is min_area and both a and b are min_size)

@pmeier
Copy link
Collaborator

pmeier commented Jul 12, 2023

Here is "proof" of Nicolas statement above

a * b <= N with a, b, N > 0

Assume a = sqrt(N) + eps with eps > 0

(sqrt(N) + eps) * b <= N <=> b <= N / (sqrt(N) + eps) = sqrt(N) * sqrt(N) / (sqrt(N) + eps)

With c = sqrt(N) / (sqrt(N) + eps) and 0 < c < 1

=> b <= sqrt(N) * c < sqrt(N)

@antoinebrl
Copy link
Contributor Author

antoinebrl commented Jul 12, 2023

The min_size only tells one thing about the removed bounding boxes. I made a sketch to illustrate the case I want to cover. In this scenario, how would you drop the square (considered too small) and keep both rectangles (who occupy a much larger region)?

Screenshot 2023-07-12 at 23 30 13

@pmeier
Copy link
Collaborator

pmeier commented Jul 13, 2023

Thanks for the sketch @antoinebrl! Here is a snippet to play with it:

import math

import torch

from torchvision import datapoints
import torchvision.transforms.v2 as transforms
from torchvision.utils import draw_bounding_boxes
from torchvision.ops import box_area
from torchvision.transforms.v2 import functional as F


def draw(bounding_boxes, *, name):
    image = torch.zeros((3, *bounding_boxes.spatial_size), dtype=torch.uint8)
    annotated_image = draw_bounding_boxes(image, bounding_boxes, colors=["red", "green", "blue"])
    F.to_image_pil(annotated_image).save(f"{name}.png")


input = datapoints.BoundingBox(
    [
        [20, 20, 360, 80],  # large rectangle
        [40, 40, 380, 60],  # small rectangle
        [30, 30, 70, 70],  # black square
    ],
    format=datapoints.BoundingBoxFormat.XYXY,
    spatial_size=(100, 400),
)
draw(input, name="input")


areas = box_area(input)
square_area = int(areas[2])

transform = transforms.SanitizeBoundingBox(min_size=math.sqrt(square_area) + 1, labels_getter=None)
draw(transform(input), name="output_min_size")

transform = transforms.SanitizeBoundingBox(min_area=square_area + 1, labels_getter=None)
draw(transform(input), name="output_min_area")

input
output_min_size
output_min_area

You are right that just min_size there is no way that I see to just drop the square, but not the larger rectangle. What both @NicolasHug and I have neglected above is that we may want to keep bounding boxes that have a shorter side length but ultimately a larger size because of a large aspect ratio.

NicolasHug
NicolasHug previously approved these changes Jul 13, 2023
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

I see, thanks for clarifying @antoinebrl . This is indeed not equivalent.

I wonder if we should just accept a filter callable (in addition to min_size which I think we should keep) so that users can pass arbitrary filters. @pmeier ?

@NicolasHug NicolasHug dismissed their stale review July 13, 2023 08:35

Didn't mean to approve just now, sorry (brain fart)

@pmeier
Copy link
Collaborator

pmeier commented Jul 13, 2023

I wonder if we should just accept a filter callable (in addition to min_size which I think we should keep) so that users can pass arbitrary filters.

IMO min_size and min_area are reasonably fundamental to have them as keywords. However, I agree that if anything else comes along we probably should just accept a callable instead of providing more builtin functionality.

@NicolasHug
Copy link
Member

NicolasHug commented Jul 13, 2023

I'm not convinced TBH. How do we justify min_area over e.g. min_height or min_ratio or min_coverage or min_iou? It seems a bit arbitrary to me for now.

@NicolasHug NicolasHug merged commit 1023987 into pytorch:main Jun 4, 2024
9 of 37 checks passed
@NicolasHug
Copy link
Member

Alright, let's go with min_area. We'll see if users want more filtering strategies. Thanks a lot for the PR @antoinebrl , I took the liberty to resolve some merge conflicts with the latest branch and made some minor updates, but the PR was already in great state.

facebook-github-bot pushed a commit that referenced this pull request Jun 7, 2024
Summary: Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

Reviewed By: vmoens

Differential Revision: D58283864

fbshipit-source-id: d598aa25d3d581b1af0f35d082fc050b353488ba
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