Skip to content

Add gallery example for drawing keypoints #4892

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

Merged
merged 9 commits into from
Nov 10, 2021
Merged

Conversation

oke-aditya
Copy link
Contributor

@oke-aditya oke-aditya commented Nov 9, 2021

Follow up of #4216

Work in progress should be done in few hours :)

Open for review. Feedbacks are welcome!

@facebook-github-bot
Copy link

facebook-github-bot commented Nov 9, 2021

💊 CI failures summary and remediations

As of commit 71584c0 (more details on the Dr. CI page):


  • 2/2 failures introduced in this PR

2 failures not recognized by patterns:

Job Step Action
CircleCI binary_libtorchvision_ops_android Build 🔁 rerun
CircleCI binary_win_conda_py3.9_cu113 Build conda packages 🔁 rerun

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@datumbox
Copy link
Contributor

datumbox commented Nov 9, 2021

@oke-aditya I'm not sure from where you sourced the image you are using but I recommend finding an alternative picture from the Coco keypoint dataset used to train the keypoint model.

@oke-aditya
Copy link
Contributor Author

oke-aditya commented Nov 9, 2021

This image was sourced from COCO dataset. Is it a concern since it displays a human image?

Edit: The reason for the image was that it is easier to connect and visualize the skeleton on this image.

@datumbox
Copy link
Contributor

datumbox commented Nov 9, 2021

@oke-aditya It's good you got it from COCO. My concern is that it seems to depict a minor so let's be extra careful and replace it. Do you think that we can reuse the standard Grace Hopper picture we already have? This might be interesting because on your example you might need to filter out keypoints that don't appear on the picture (and thus it's going to be a more realistic example of visualization). Thoughts?

@oke-aditya
Copy link
Contributor Author

Oh! So sorry for depicting a minor! I completely agree we should avoid this image.
Very very thankful to you @datumbox for pointing this out.

My apologies to the minor person depicted (who would have grown up by today 😃 ) who might see this PR.

That's Good idea to add an example where we need to filter out keypoints. Both based on visibility and detection threshold.
But I feel that joining a complete skeleton is particular useful as for pose-detection / other tasks. So I will keep two examples.
First one just joining skeletons and next one helping to filter keypoints well

@datumbox
Copy link
Contributor

datumbox commented Nov 9, 2021

@oke-aditya Sounds good. Let's select an alternative picture from COCO and do 2 examples as you proposed. Thanks!

Copy link
Contributor Author

@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.

Fixing a small bug.
I'm still unable to find an image where visibility of keypoint isn't 1.000.

@oke-aditya
Copy link
Contributor Author

Absolutely baffled still that every run with keypoint_rcnn_resnet50 with both pretrained=True and False, the visibility was always 1.00 for all the keypoints and instances.

Tried over 15 images from coco, of not just persons, dogs, cats, surfboards. :(

Is there really a case where visibility won't be 1 ? Can someone provide an image for such case.

@datumbox
Copy link
Contributor

datumbox commented Nov 9, 2021

@oke-aditya I had the same exact discussion with Francisco couple of days ago and he clarified that this is always 1. So I suggest to just ignore this column and move on.

@fmassa could you please confirm that this is the case?

# As we see the keypoints appear as colored circles over the image.
# The coco keypoints for a person are ordered and represent the following list.\

coco_keypoints = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this will be great when people can use metadata from the new keypoint rcnn weights.
Right now we have to explicitly remember this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although it isn't used in the code. I made this as a code block so that it is copy-pastable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes agreed. We've already added it on the new API:

_common_meta = {"categories": _COCO_PERSON_CATEGORIES, "keypoint_names": _COCO_PERSON_KEYPOINT_NAMES}

_COCO_PERSON_CATEGORIES = ["no person", "person"]
_COCO_PERSON_KEYPOINT_NAMES = [
"nose",
"left_eye",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess once the new prototype models and transforms are moved to stable, we would need to revisit the gallery examples.

@oke-aditya oke-aditya changed the title [WIP] Add gallery example for drawing keypoints Add gallery example for drawing keypoints Nov 9, 2021
@oke-aditya oke-aditya marked this pull request as ready for review November 9, 2021 20:19
@oke-aditya
Copy link
Contributor Author

I have added one single example showing plotting keypoints and joining them.
I felt that one detailed and simple example is necessary as keypoint rcnn involves little bit more processing and understanding to connect keypoints.

Let me know if an additional example such as batch of images or plotting multi instance images is needed.
I think it will be nice to have.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

Overall it looks good to me, thanks @oke-aditya.

Just a couple minor comments. Let me know what you think:

# As we see the keypoints appear as colored circles over the image.
# The coco keypoints for a person are ordered and represent the following list.\

coco_keypoints = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes agreed. We've already added it on the new API:

_common_meta = {"categories": _COCO_PERSON_CATEGORIES, "keypoint_names": _COCO_PERSON_KEYPOINT_NAMES}

_COCO_PERSON_CATEGORIES = ["no person", "person"]
_COCO_PERSON_KEYPOINT_NAMES = [
"nose",
"left_eye",

@NicolasHug
Copy link
Member

Thanks @oke-aditya , I just gave a brief look to the rendered example and it looks great. The only comment I have is about the first sentence in the example which is

This example illustrates some of the utilities that torchvision offers for visualizing images, bounding boxes, and segmentation masks.

and that could now be updated to

This example illustrates some of the utilities that torchvision offers for visualizing images, bounding boxes, segmentation masks, and keypoints.

Nice work! :)

@oke-aditya
Copy link
Contributor Author

This example illustrates some of the utilities that torchvision offers for visualizing images, bounding boxes, and segmentation masks.

Nice, catch. I will also update the visualization thumbnail to include this image too.

Nice work! :)

Thanks Nicolas :) once we finish supporting multiple colors, line colors, etc.
I will add one small example which shows multiple instances. The utility looks cool 😎 on multiple images.

@oke-aditya
Copy link
Contributor Author

oke-aditya commented Nov 10, 2021

Here is the final gallery. Feel free to suggest changes :)

https://980059-73328905-gh.circle-artifacts.com/0/docs/auto_examples/index.html

cc @NicolasHug @datumbox

@oke-aditya oke-aditya requested a review from datumbox November 10, 2021 12:08
@datumbox datumbox merged commit 30669a5 into pytorch:main Nov 10, 2021
@github-actions
Copy link

Hey @datumbox!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@oke-aditya oke-aditya deleted the kpt_gall branch November 10, 2021 13:07
@datumbox
Copy link
Contributor

@oke-aditya Great job as always Oke. Thanks for contributing this and for your patience for the reviews on the original PR.

cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
* Start writing gallery example

* Remove the child image fix implementation add code

* add docs

* Apply suggestions from code review

Co-authored-by: Vasilis Vryniotis <[email protected]>

* address review update thumbnail

Co-authored-by: Philip Meier <[email protected]>
Co-authored-by: Vasilis Vryniotis <[email protected]>
facebook-github-bot pushed a commit that referenced this pull request Nov 17, 2021
Summary:
* Start writing gallery example

* Remove the child image fix implementation add code

* add docs

* Apply suggestions from code review

* address review update thumbnail

Reviewed By: datumbox

Differential Revision: D32470492

fbshipit-source-id: ae83abd054dccd4c2d423bf0f1c7e1ae04852b8a

Co-authored-by: Vasilis Vryniotis <[email protected]>
Co-authored-by: Philip Meier <[email protected]>
Co-authored-by: Vasilis Vryniotis <[email protected]>
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