Unity-Technologies / pysolotools

Python toolchain for SOLO.
https://Unity-Technologies.github.io/pysolotools
Other
39 stars 14 forks source link

Adding support for keypoint names/labels other than the ones used in the COCO dataset #138

Open hoel-bagard opened 1 year ago

hoel-bagard commented 1 year ago

Hi,

I am working with a SOLO dataset where the keypoint labels are not the ones used in the COCO dataset. I would however like to convert this SOLO dataset to the COCO format. In the Keypoint Detection part of the COCO format documentation, it is explained how other keypoint names should be handled.

I would be interested in making a PR to add support for custom keypoint names. I would like to know if such a PR would be welcome, and, if yes, if there is anything I should know before starting to implement it.

(The current behavior is to simply ignore all the keypoints whose name is not in the COCO dataset, essentially failing silently.)

StevenBorkman commented 1 year ago

@hoel-bagard I think we would be very interested in your solution. We have a contribution guidelines to take a look at. This will be an interesting experiment, as, I believe, you will be the first outside user to contribute back to the project.

hoel-bagard commented 1 year ago

@StevenBorkman Thank you, I'll open a PR soon to show how I would go about implementing it.

I have two questions:

StevenBorkman commented 1 year ago

@hoel-bagard

The SOLO format's documentation puts the skeleton outside of the template field (towards the end of the page). However in the annotation_definitions.json file used for tests it is inside of the template. Is it correct to assume that the tests' version is correct ? Yes, the SkeletonDefinition is inside of the Template, Here's the code that defines it.

In the function creating the categories, the keypoints and skeleton are hard-coded to be empty. Is it fine if I change that with the assumption that there will only be one class when doing keypoint detection ? (I don't understand how keypoints are associated to a class in the SOLO format) I think so. I'm not really following what class is in this example. In perception we could have multiple keypoint labelers running which could create different outputs for each labeler. But each of the labeler's outputs will be unique in solo.

hoel-bagard commented 1 year ago

@StevenBorkman Thank you for your answer.

I think so. I'm not really following what class is in this example. In perception we could have multiple keypoint labelers running which could create different outputs for each labeler. But each of the labeler's outputs will be unique in solo.

Sorry, by class I meant a label_id and/or label_name of a segmentation/bounding box type (in COCO terms it would be a category).

I have created a PR so that you can see what I've modified so far.

hoel-bagard commented 1 year ago

@StevenBorkman Do you have an idea of when you would be able to look into the changes ?

aryansaurav commented 1 year ago

@hoel-bagard following this thread.. thanks for contributing! Is there already a PR ? Also, just curious, have you tried the SAHI package for conversion to COCO? I was looking into that as an alternative..

hoel-bagard commented 1 year ago

@aryansaurav I made a PR here.

Also, just curious, have you tried the SAHI package for conversion to COCO?

I have not tried that package.

aryansaurav commented 1 year ago

@aryansaurav I made a PR here. Great, thanks @hoel-bagard ! I am going to try your branch and ping you here in case of any issues.

Also, just curious, have you tried the SAHI package for conversion to COCO?

I have not tried that package.

aryansaurav commented 1 year ago

Hello @hoel-bagard I tried your package and it seems to work! With a few caveats, for example, the bounding box labels must be available or should handle the exception otherwise.

I have a couple of questions:

  1. Is there support for segmentation masks in the YOLO format?

  2. In COCO format, is the segmentation mask written in compressed format? If so, can there be an option to write it in uncompressed format?

Thanks a lot for sharing your awesome contribution

hoel-bagard commented 1 year ago

With a few caveats, for example, the bounding box labels must be available or should handle the exception otherwise.

They are not optional in the COCO format. That's probably why it's not supported in that repo, if you would like to have some way to make it optional/handle that case better, then you should create a separate issue for it.

Is there support for segmentation masks in the YOLO format?

I don't think so.

In COCO format, is the segmentation mask written in compressed format? If so, can there be an option to write it in uncompressed format?

There are three ways to store the segmentation mask in COCO, so it's related to this package and not to the format itself. This package is using the encoded RLE format (here). There doesn't seem to be an option to use another format, but writing a script that converts from one segmentation format to another is not too hard. If you would like to have the option to do it with this package, then you can try to open an issue for it.

aryansaurav commented 1 year ago

Thanks @hoel-bagard Following your pointers, I was able to convert the segmentation format. I can open an issue but I agree it's not very hard for anyone to do it by themselves.

I have another question though.. have you tested generating segmentation (either instance or semantic) masks for multiple objects? I tried that but on converting to coco format, it includes only one category id segmentation mask.

hoel-bagard commented 1 year ago

@aryansaurav I have generated instance segmentation data and got the expected number of categories after converting to COCO format.

aryansaurav commented 1 year ago

Indeed yes, it works with multiple bounding box.. I had to resolve some issues with the label id configs.

Thanks again for this awesome contribution. Hope your pull request goes through. By the way, there is an exception error that may occur in case of missing annotations for a given frame. That can be handled using the fix suggested the issue I just closed now.