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

Add keypoint-detection task to Hub #870

Merged
merged 5 commits into from
Sep 2, 2024
Merged

Conversation

merveenoyan
Copy link
Contributor

@merveenoyan merveenoyan commented Aug 27, 2024

I have opened PR to keypoint detection because we already have around 30 keypoint detection models on Hub (currently opening PRs to change pipeline tag), one model in transformers, and this doesn't fall under any other task.

Screenshot 2024-08-27 at 15 08 26

@merveenoyan
Copy link
Contributor Author

also cc @NielsRogge

@@ -131,6 +131,7 @@ export const TASKS_MODEL_LIBRARIES: Record<PipelineType, ModelLibraryKey[]> = {
"image-to-image": ["diffusers", "transformers", "transformers.js"],
"image-to-text": ["transformers", "transformers.js"],
"image-to-video": ["diffusers"],
"keypoint-detection": ["transformers"],
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this mean? Cause there's no keypoint detection pipeline in Transformers yet

Copy link
Contributor Author

@merveenoyan merveenoyan Aug 28, 2024

Choose a reason for hiding this comment

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

this is not about pipelines, it's to show sort of which libraries support this task, e.g. in tasks
Screenshot 2024-08-28 at 12 20 12

we currently have SuperPoint and will soon have ViTPose in transformers so we do support it

Copy link
Member

Choose a reason for hiding this comment

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

we currently have SuperPoint and will soon have ViTPose

I think we should tag those models then before the PR is merged

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 have opened PRs to 30-40 models and some are merged already I think, see

also Sapiens model will have a lot of keypoint models soon (it was one unified model repo, I sent authors a script to automate separation)

Screenshot 2024-08-29 at 14 02 42

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

@pcuenca pcuenca Aug 29, 2024

Choose a reason for hiding this comment

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

I have opened PRs to 30-40 models and some are merged

Yes, I see 4 models now 👍

Comment on lines 661 to 666
"keypoint-detection": {
name: "Keypoint Detection",
modality: "cv",
color: "red",
hideInDatasets: true,
},
Copy link
Member

Choose a reason for hiding this comment

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

Should it be a subtask of object-detection instead? There's already a face-detection subtask, for example.

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 it's not. object detection is where you detect bounding boxes, but in this one outputs look like these:
keypoint-detection-output
Screenshot 2024-08-29 at 14 01 01

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with @merveenoyan, keypoint-detection can be seen as a task at the same level of object detection, it's not a subtask

@@ -131,6 +131,7 @@ export const TASKS_MODEL_LIBRARIES: Record<PipelineType, ModelLibraryKey[]> = {
"image-to-image": ["diffusers", "transformers", "transformers.js"],
"image-to-text": ["transformers", "transformers.js"],
"image-to-video": ["diffusers"],
"keypoint-detection": ["transformers"],
Copy link
Member

Choose a reason for hiding this comment

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

we currently have SuperPoint and will soon have ViTPose

I think we should tag those models then before the PR is merged

Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Looking nice!

name: "Keypoint Detection",
modality: "cv",
color: "red",
hideInDatasets: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Any datasets for this on the Hub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just set to false

@@ -131,6 +131,7 @@ export const TASKS_MODEL_LIBRARIES: Record<PipelineType, ModelLibraryKey[]> = {
"image-to-image": ["diffusers", "transformers", "transformers.js"],
"image-to-text": ["transformers", "transformers.js"],
"image-to-video": ["diffusers"],
"keypoint-detection": ["transformers"],
Copy link
Contributor

Choose a reason for hiding this comment

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

@merveenoyan
Copy link
Contributor Author

lol I opened too many PRs Hub thinks I'm spamming

Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

LGTM with the contributions from #873

merveenoyan and others added 3 commits August 29, 2024 22:28
Co-authored-by: Merve Noyan <mervenoyan@Merve-MacBook-Pro.local>
Co-authored-by: Pedro Cuenca <pedro@huggingface.co>
@merveenoyan merveenoyan merged commit 678eba8 into main Sep 2, 2024
3 of 5 checks passed
@merveenoyan merveenoyan deleted the add-keypoint-detection branch September 2, 2024 11:28
merveenoyan added a commit that referenced this pull request Sep 2, 2024
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.

4 participants