Skip to content

Random colors for draw_bounding_boxes #4658

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

Closed
wants to merge 7 commits into from
Closed

Random colors for draw_bounding_boxes #4658

wants to merge 7 commits into from

Conversation

ABD-01
Copy link
Contributor

@ABD-01 ABD-01 commented Oct 19, 2021

Hi!
This pr is with reference to #4528

I have made the changes for generating random colors from the palette for bounding boxes.

But I am not getting why the tests are failing for the following cases even before the changes are done.
i.e. it is not accepting colors="red" or colors="#FF00FF" as valid inputs

======================================================== short test summary info =========================================================
FAILED test/test_utils.py::test_draw_boxes_colors[red] - ValueError: unknown color specifier: 'r'
FAILED test/test_utils.py::test_draw_boxes_colors[#FF00FF] - ValueError: unknown color specifier: '#'
FAILED test/test_utils.py::test_draw_boxes_colors[colors4] - IndexError: tuple index out of range
FAILED test/test_utils.py::test_draw_boxes_grayscale - TypeError: Cannot handle this data type: (1, 1, 1), |u1
FAILED test/test_utils.py::test_draw_invalid_boxes - Failed: DID NOT RAISE <class 'ValueError'>

Thanks to @oke-aditya for pseudocode.

@oke-aditya
Copy link
Contributor

oke-aditya commented Oct 19, 2021

Hi @ABD-01 I don't think so the error you mentioned might be valid, CI failures don't point to that.

I think you need to update the expected image, in the assets.

A simple way is to delete the existing images, and re-run the pytest.

Apart from that can you just have a trial run like an example from our gallery and post the outputs here?

Edit:
The optimization to pseudocode was nice 😃 I wrote it way longer than needed.

@oke-aditya
Copy link
Contributor

Also

if fill:
    if color is None:
fill_color = (255, 255, 255, 100)

Do you think we will ever hit color is None here in fill? we can just remove these lines I guess?

Copy link
Contributor

@oke-aditya oke-aditya left a comment

Choose a reason for hiding this comment

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

We also need to add to docstring.
By default, random colors are generated for boxes. If labels are provided, boxes with same labels have same color.

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.

Thanks @ABD-01 and @oke-aditya for the review. This looks great, I just have one minor comment below. The ones from @oke-aditya above are also relevant :)

Looks like some related tests are failing, would you mind taking a look?

Also, since these things are hard to test, would you mind providing a few visual examples of images with boxes illustrating the new color palette in use?

Thanks!

@ABD-01
Copy link
Contributor Author

ABD-01 commented Oct 24, 2021

When we pass colors ourselves:

boxes = torch.tensor([[50, 50, 100, 200], [210, 150, 350, 430], [150, 30, 900, 650]], dtype=torch.float)
colors = ["blue", "yellow", "red"]
result = draw_bounding_boxes(dog1_int, boxes, colors=colors, width=8)
show(result)

image

When colors or labels argument is not given:
It creates N colors for N boxes

boxes = torch.tensor([[50, 50, 100, 200], [210, 150, 350, 430], [150, 30, 900, 650]], dtype=torch.float)
result = draw_bounding_boxes(dog1_int, boxes, width=8)
show(result)

image

When we pass labels:
it generated colors of each label.

boxes = torch.tensor([[50, 50, 100, 200], [210, 150, 350, 430], [150, 30, 900, 650]], dtype=torch.float)
labels = ["box1", "box2", "box3"]
result = draw_bounding_boxes(dog1_int, boxes, labels=labels, width=8)
show(result)

image

What if labels are repeated:
It creates one color for each label

boxes = torch.tensor([[210, 150, 350, 430], [50, 50, 100, 200], [150, 30, 900, 650]], dtype=torch.float)
labels = ["box1", "box2", "box1"]
result = draw_bounding_boxes(dog1_int, boxes, labels=labels, width=8)
show(result)

image

The image is attached with the respective code snippets.
One thing I noticed is that the function _generate_color_palette doesn't generate random colors but creates a spectrum starting near the green color with first color as BLack

@ABD-01
Copy link
Contributor Author

ABD-01 commented Oct 24, 2021

Looks like some related tests are failing, would you mind taking a look?

Yes Sure.

Looks like we gave the expected image to look like this:
here

But now we are asking it to make boxes of different color, so it will look like this:
image

@oke-aditya
Copy link
Contributor

oke-aditya commented Oct 24, 2021

The test will be flaky if we expect to check if a randomly generated image matches the same image. So we have to pass colors to the function while testing.

The logic looks to be working fine, as we are getting consistent colors for boxes with same label.

I am bit busy this week. I will have a run and check why the green color is getting preferred.
Possibly reason is as num_boxes is fixed, we will be getting same colors. As describe by the function it is generate_color_palette and not generate_random_color_palette. (This is probably a thing to enhance as well)

Edit:
If you don't want to update the existing image.
A simple fix to test can be just pass colors = ["white", "white", "white"] or probably colors = ["white"] * boxes.size()

@oke-aditya
Copy link
Contributor

To generate a A Tensor containing random RGB colors

>>> random_color_tensor = torch.randperm(256)[:3]
>>> random_color_tensor
tensor([232,  98, 231])

We can modify generate_color_palette and make it generate random colors.

Can you have a go with these modifications @ABD-01 ?

@NicolasHug
Copy link
Member

let's just pass colors="white" to avoid having to push new image. We don't want to rely on the randomly generated colors for the tests because the RNG isn't guaranteed to be the same across pytorch version

colors = _generate_color_palette(len(img_boxes))
else:
assert len(labels) == len(img_boxes)
label_color_map = dict(zip(labels, _generate_color_palette(len(labels))))
Copy link
Contributor

Choose a reason for hiding this comment

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

Since now we agree that len(labels) == len(img_boxes) Line 207 and 204 can be simplified.

@datumbox
Copy link
Contributor

datumbox commented Feb 2, 2022

Closing in favour of #5127

@datumbox datumbox closed this Feb 2, 2022
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.

5 participants