supervision
supervision copied to clipboard
roboflow
feat: 🚀 initial sv.Keypoints features added
Description
This pull request introduces the KeyPoints class. This class aims to provide a unified format for results from various keypoint detection models. First support for yolov8 pose added.
Fix: #824
Type of change
- [X] New feature (non-breaking change which adds functionality)
- [X] This change requires a documentation update
How has this change been tested, please provide a testcase or example of how you tested the change?
YOUR_ANSWER
Any specific deployment considerations
For example, documentation changes, usability, usage/costs, secrets, etc.
Docs
- [X] Docs updated? What were the changes: New section keypoints added
Google Collab Link : https://colab.research.google.com/drive/1uqpgORL1VZebM1MRnP-KGR6SmrIPLAsg?usp=sharing
@SkalskiP before going further can you please check in current state. I would like to hear your opinion.
Hey @onuralpszr :wave:
That looks really good. I'm very fond of having validators, especially if they're nice and encapsulated like that. Plus, the structure is pretty much like Detections - new contributors would feel right at home. I've seen a spec floating around in one of GitHub issues, and it seems like this matches it exactly.
The issue is, in supervision, I keep bumping into problems coming from having this architecture of related multi-arrays. Synchronizing those is painful, and various bits of the codebase are just inconsistent with that. Sometimes easy to fix, sometimes not (especially if there's the data field).
By now, I'm gravitating towards an opinion that turning it into a list-of-structs would solve 99% of those - we just can't since it's painful for users to deprecate things. Here there'll be an extra hierarchy layer too - we'll have a list of keypoint-sets, for each person detected in an image, for example.
(Edit: or we need an 'object_id' array where values change for person 1, person_2, etc.)
The most concise example I saw here: https://www.geeksforgeeks.org/numpy-structured-array/
It's nice since it seems to be memory-efficient, and every object is packed together with all variables it'd need. Iteration is easy, nullability wouldn't be a problem. A bit of thought is needed in how we define special values, but otherwise I feel like it'd be an optimal solution.
Stepping out of this PR (I'm not suggesting you change anything :slightly_smiling_face:), what are your thoughts? Do you think we'd run into any issues?
Hey @onuralpszr 👋
That looks really good. I'm very fond of having validators, especially if they're nice and encapsulated like that. Plus, the structure is pretty much like
Detections- new contributors would feel right at home. I've seen a spec floating around in one of GitHub issues, and it seems like this matches it exactly.The issue is, in
supervision, I keep bumping into problems coming from having this architecture of related multi-arrays. Synchronizing those is painful, and various bits of the codebase are just inconsistent with that. Sometimes easy to fix, sometimes not (especially if there's thedatafield).By now, I'm gravitating towards an opinion that turning it into a list-of-structs would solve 99% of those - we just can't since it's painful for users to deprecate things. Here there'll be an extra hierarchy layer too - we'll have a list of keypoint-sets, for each person detected in an image, for example.
(Edit: or we need an 'object_id' array where values change for
person 1,person_2, etc.)The most concise example I saw here: https://www.geeksforgeeks.org/numpy-structured-array/
It's nice since it seems to be memory-efficient, and every object is packed together with all variables it'd need. Iteration is easy, nullability wouldn't be a problem. A bit of thought is needed in how we define special values, but otherwise I feel like it'd be an optimal solution.
Stepping out of this PR (I'm not suggesting you change anything 🙂), what are your thoughts? Do you think we'd run into any issues?
Thank you for your comment.
I will do more in-depth comments but one quick comment that comes to my mind is changing to structural needs change of internal gears and possibly without affecting others but If we change on the front that's also something I don't want right now (maybe we need to keep for old one for 10-20 releases maybe) I need to think bit more around it too.
In this case, I only mean the new Keypoints module. Detections would remain as it currently is.
And yes, no rush! :)
In this case, I only mean the new
Keypointsmodule.Detectionswould remain as it currently is.And yes, no rush! :)
I had that idea too, do only in Keypoints but still thinking :)
Hi @onuralpszr 👋🏻, I'll be taking a look at this PR tomorrow! 🔥
Sure, waiting your review.
Hi @onuralpszr 👋🏻, I'm closing this PR. @LinasKo picked up key points for work here: https://github.com/roboflow/supervision/pull/1128. Your commits are in the git history of this branch, so no contribution is lost. Thanks a lot!