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

1114 rle support for coco #1163

Merged
merged 60 commits into from May 21, 2024
Merged

Conversation

emSko
Copy link
Contributor

@emSko emSko commented May 5, 2024

Description

PR adds support for the RLE format in the COCO dataset. Based on #1114 .

It required implementing RLE encode/decode functions and extending the coco_annotations_to_detections/detections_to_coco_annotations functions.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

All functionalities are covered with unit tests + notebook checks the functionalities on larger images.

Any specific deployment considerations

None

Docs

  • Docs updated? What were the changes: docs/datasets/utils.md

@CLAassistant
Copy link

CLAassistant commented May 5, 2024

CLA assistant check
All committers have signed the CLA.

@SkalskiP
Copy link
Collaborator

SkalskiP commented May 6, 2024

Hi @emSko 👋🏻 Could you accept CLA? Without this, we won't be able to merge your PR.

@SkalskiP
Copy link
Collaborator

SkalskiP commented May 6, 2024

Let's split the work on this PR into three parts:

  • Implementation of rle_to_mask and mask_to_rle. Please write valid docstrings in Google format for both methods. For example, take a look at other documented functions in our repository here. The documentation should describe the input and output arguments, as well as provide a simple usage example. Regarding the tests, create several cases: when the image is entirely zeros, when the image is entirely ones, when the image contains a mask with a hole, when the image contains a disjoined mask consisting of multiple elements. Use pytest.mark.parametrize as demonstrated in this test.
    • code
    • documentation
    • unit tests
  • Integration of rle_to_mask method into sv.DetectionDataset.from_coco logic.
    • code
    • documentation updates
  • Integration of mask_to_rle method into sv.DetectionDataset.as_coco logic.
    • code
    • documentation updates

supervision/dataset/formats/coco.py Outdated Show resolved Hide resolved
supervision/dataset/utils.py Outdated Show resolved Hide resolved
supervision/dataset/utils.py Outdated Show resolved Hide resolved
@emSko
Copy link
Contributor Author

emSko commented May 6, 2024

@SkalskiP what do you mean by the documentation? Only docstrings or are there additional files? Where to find them?

@SkalskiP
Copy link
Collaborator

SkalskiP commented May 8, 2024

Hi @emSko 👋🏻 Thank you very much for dedicating time to this task. In the meantime, I did some light research and found these two videos on YouTube:

I compared the results of our implementation of encoding and decoding as well as the execution time. The outcomes are identical; however, the execution time, especially for encoding, is significantly slower. Could we utilize similar tricks to accelerate the encoding process? Here's the Colab I used.

https://colab.research.google.com/drive/1Yl3w0B12htaso_VxZ-S2LvvxftiRk-ta?usp=sharing

@SkalskiP
Copy link
Collaborator

SkalskiP commented May 8, 2024

@SkalskiP what do you mean by the documentation? Only docstrings or are there additional files? Where to find them?

I would like the mask_to_rle and rle_to_mask functions to be included here. To make this happen, you need to add the appropriate changes to this file.

@LinasKo
Copy link
Collaborator

LinasKo commented May 17, 2024

Hi @emSko 👋

Do a git pull - I've added a line that fixes the segfault.

I couldn't reproduce it as well, but found this issue

We'll eventually update the opencv to >=4.6.0.0 which allegedly resolves it, but for now this should be sufficient.

@emSko
Copy link
Contributor Author

emSko commented May 17, 2024

Hi @LinasKo, thank you for your help.

@SkalskiP I think the PR is ready for review. I don't think that from_coco requires any documentation updates.

@emSko emSko changed the title [WIP] 1114 rle support for coco 1114 rle support for coco May 17, 2024
supervision/dataset/formats/coco.py Outdated Show resolved Hide resolved
supervision/dataset/formats/coco.py Outdated Show resolved Hide resolved
test/dataset/formats/test_coco.py Outdated Show resolved Hide resolved
@SkalskiP
Copy link
Collaborator

Hi @emSko 👋🏻 At the outset, I apologize for the late reply - I was traveling at the end of the week and was not on GitHub. @LinasKo, thanks for your support! 🙏🏻

@emSko you did an excellent job here! We are already on the last straight. I left some final comments.

@SkalskiP
Copy link
Collaborator

Hi @emSko 👋🏻

I just tested your code in Google Colab. Everything seems to be working fine. I also made a few minor changes to the docs, including adding some visualizations. Everything looks great. Many thanks for this work. This is a massive upgrade for COCO dataset support. Working with you was a pleasure. Thank you! 🙏🏻

@SkalskiP SkalskiP merged commit 1bbe03c into roboflow:develop May 21, 2024
9 checks passed
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.

None yet

4 participants