[WIP] Implementation of SuperGlue#25697
Conversation
|
Hey! Thanks a lot for contributing! 🚀 as a first step I would suggest you to upload the model on the hub following this tutorial, as it will make the process a lot easier, and won't need to go through our strict CIs! 🤗 |
|
Hi ! Ok, I have a first question regarding this specific model. The question now : should I add the implementation of SuperPoint in the SuperGlue code and consider this combo SuperPoint + SuperGlue as the SuperGlue model in transformers (and also calling the class SuperGlueSuperPoint to fit the naming convention), or should I add SuperPoint as another model, standalone, part of transformers ? I myself can't really tell what would be best since :
Hope the question is clear and also since I'm new to all this "collaborating" thing on GitHub, let me know if this kind of questions should belong here or somewhere else. Thanks again for considering my contribution ! EDIT : Also what is the difference between a model on the hub and a model added in the transformers library, I got confused by the existence of both this page and this page |
|
cc @amyeroberts I need your take on this 😄 |
- Added the SuperGlueConfig - Added the SuperGlueModel and its implementation - Added basic weight conversion script - Added new ImageMatchingOutput dataclass
|
Hi, In the meantime I added the basics for the implementation of SuperGlue by following the example of the tutorial you provided me earlier. I also looked around other models on how conversion scripts were implemented and mimicked it for the SuperGlue case. |
|
@sbucaille Thanks for the detailed explanation about the two models! The model PR is a good place to ask questions about implementation :) As both models, SuperPoint and SuperGlue offer new capabilities and are very popular, I would consider them good additions directly into transformers. However, as Arthur mentions, this would involve going through the PR review process which will be slower and more restrictive than adding directly onto the hub. If you want to go straight to the hub - you can decide how you would like to add the model! If adding to transformers, what I would suggest is implementing SuperPoint as its own model and PR with task models e.g. Then we can implement SuperGlue. Similar to e.g. MusicGen we can have SuperGlue load in any keypoint detection model using AutoModelForPointCorrespondence. Then, if in the future you wanted to add DISK, SuperGlue could load either DISK or SuperPoint. Likewise, if you wanted to add LightGlue, it can then load DISK or SuperPoint using the same AutoModelForPointCorrespondence structure. The important thing is that all the models being loaded using AutoModelForXxx have the same input / output structure. |
|
I created the PR for the SuperPoint implementation. |
|
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
|
@sbucaille I'll leave this closed for now so we don't have to keep re-opening every week. I know you're off for a few weeks - just ping when you're back and I can reopen again then |
|
@sbucaille Unfortunately I can't reopen the PR, as it says the branch has been force pushed or recreated. |
|
@amyeroberts That's correct I rebased the branch on the latest changes (with SuperPoint) and added the first changes a couple days ago, does that mean I should open a new PR ? |
|
@sbucaille Yes please! |
What does this PR do?
This PR implements the SuperGlue model.
#25489
Who can review?
@amyeroberts
Todo's