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

Adds DoG-HardNet model #103

Merged
merged 14 commits into from
Jan 23, 2024
Merged

Adds DoG-HardNet model #103

merged 14 commits into from
Jan 23, 2024

Conversation

ducha-aiki
Copy link
Contributor

@ducha-aiki ducha-aiki commented Jan 15, 2024

Link to the weights: http://cmp.felk.cvut.cz/~mishkdmy/models/doghardnet_lightglue.pth

Benchmark on MD1500 with OpenCV:

49.8 / 66.2 / 79.0

@Phil26AT
Copy link
Collaborator

Hi @ducha-aiki, thank you for this excellent contribution! This model seems to be a straight improvement over SIFT+LightGlue, which is great.

I think we can simplify the DogHardNet class a bit. Inherit from SIFT, and put these few lines into the forward method (after p=self.extract_singe_image(img)). extract_single_image can then be reused from SIFT, which makes code maintenance easier.

If this is done we can merge, and I can attach your weights to the release.

@ducha-aiki
Copy link
Contributor Author

@Phil26AT Thanks. I can do that, however, there will be a slight inefficiency in the code, as the SIFT descriptors computation will be wasted.

@ducha-aiki
Copy link
Contributor Author

@Phil26AT Done. I have also added missing import into __init__.py and to example in Readme.

@Phil26AT
Copy link
Collaborator

Looks great!

@Phil26AT Thanks. I can do that, however, there will be a slight inefficiency in the code, as the SIFT descriptors computation will be wasted.

Yes correct, but this would only apply to opencv SIFT extraction (pycolmap does not allow to turn description off). Did you ever benchmark this? If the difference is big we can maybe pass a flag to run_opencv_sift.

@ducha-aiki
Copy link
Contributor Author

@Phil26AT that is 10ms, which we probably can ignore - the HardNet extraction is dominating the time anyway (esp. on CPU).

@Phil26AT
Copy link
Collaborator

Okay yeah I guess we can ignore it for now. I'll merge this PR then, thank you for the contribution!

@Phil26AT Phil26AT merged commit be49528 into cvg:main Jan 23, 2024
1 check passed
@ducha-aiki
Copy link
Contributor Author

Great, thank you! Don't forget to name the weights under your convention.

@ExtReMLapin
Copy link

In the assets, dog_hardnet_lightlue should be named doghardnet_lightlue because the dl fails

@Phil26AT
Copy link
Collaborator

Fixed, thank you for the note.

@ducha-aiki
Copy link
Contributor Author

@Phil26AT Sorry to bother you again, but it is still wrong at release page:
doghardnet_lightlue.pth instead of doghardnet_lightglue.pth

@Phil26AT
Copy link
Collaborator

@ducha-aiki I am sorry for the typo, it is fixed now.

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.

3 participants