supervision icon indicating copy to clipboard operation
supervision copied to clipboard

WIP - feat: 🚀 initial keypoint support for transformers added

Open onuralpszr opened this issue 1 year ago • 5 comments

Description - Working in Progress

Recent release of transformers introduce keypoint support and initial support for keypoint added.

Type of change

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

Docs

  • [X] Docs updated? What were the changes:

onuralpszr avatar Sep 27 '24 15:09 onuralpszr

@merveenoyan 👋 hello I notice current latest and dev doesn't have "post_process_keypoint_detection" function and I was able to run via only this pull request branch "https://github.com/huggingface/transformers/pull/33200" Today I read about "https://huggingface.co/docs/transformers/tasks/keypoint_detection" you posted in social and I was unable run keypoint but I was able to do via using install this repo/branch, I think it is forgot to merge ?

!pip install git+https://github.com/sbucaille/transformers@superpoint_fix -q

Error output when I ran from "main"

    outputs = processor.post_process_keypoint_detection(outputs, image_sizes)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'SuperPointImageProcessor' object has no attribute 'post_process_keypoint_detection'

Here is my initial collab link for working version

https://colab.research.google.com/drive/1gxpE9iDR7gXl2VwwScB5W8Yw-e_xUo3i?usp=sharing

onuralpszr avatar Sep 27 '24 16:09 onuralpszr

@LinasKo missing function basically converting normalize values to w,h values to position correctly should I finish based on that in current release and finish the PR ?

onuralpszr avatar Oct 01 '24 06:10 onuralpszr

@onuralpszr, I need more details.

  1. Missing function where? Our repo or theirs?
  2. "should I finish based on that in current release and finish the PR ?" - Based on what? Based on the function being missing? I'd like more clarity here 😉

LinasKo avatar Oct 01 '24 08:10 LinasKo

@onuralpszr, I need more details.

  1. Missing function where? Our repo or theirs?
  2. "should I finish based on that in current release and finish the PR ?" - Based on what? Based on the function being missing? I'd like more clarity here 😉

Sorry, I was little bit hasty I guess and for answering your questions

1 - Missing function is theirs and it is post process function for converting normalize xy values to pixel based on xy values (post_process_keypoint_detection is missing) 2 - Currently latest version of transfomers has SuperPointImageProcessor but, doesn't have SuperPointImageProcessor.post_process_keypoint_detection() so I will finish PR based on latest version of transformers to and I will write the helper function to convert image size correction or I had to assume it does use function and leave as it is. You can check the PR link I posted above to see it.

onuralpszr avatar Oct 01 '24 08:10 onuralpszr

@LinasKo I see approve so I am going to move forward based on this PR ("huggingface/transformers#33200" ) I am going to finish this PR

onuralpszr avatar Oct 16 '24 01:10 onuralpszr

Hey @onuralpszr ,

How's this one going? Is it ready for review, or is there some work left to do?

LinasKo avatar Nov 02 '24 07:11 LinasKo

Hey @onuralpszr ,

How's this one going? Is it ready for review, or is there some work left to do?

I have small problem when I detect multiple images at once keypoint list doesn't come in same sizes. Point lists are different

onuralpszr avatar Nov 02 '24 08:11 onuralpszr