lpiccinelli-eth / UniDepth

Universal Monocular Metric Depth Estimation
Other
473 stars 39 forks source link

Make sure HF download metrics work #7

Closed NielsRogge closed 3 months ago

NielsRogge commented 3 months ago

Dear authors,

I noticed your awesome repository trending on X. I see you already leverage hf_hub_download, however all checkpoints are present in a single repository, which means download metrics won't work. ๐Ÿ˜ž

This PR resolves that by showcasing the PyTorchModelHubMixin class, which is a minimal class that allows to easily add functionalities like from_pretrained, save_pretrained, and push_to_hub to any custom nn.Module (without the need to rely on Torch hub or define your own from_pretrained).

Similar to models in the Transformers/Diffusers library, it allows to push and reload models in the format of safetensors (for the weights) and a config.json (for the model's configuration). It also creates an automated model card, along with tags for better discoverability of your models. Usage is as follows:

from unidepth.models import UniDepthV1HF

model = UniDepthV1HF.from_pretrained("nielsr/unidepth-v1-convnext-large")

The corresponding model is here for now: https://huggingface.co/nielsr/unidepth-v1-convnext-large, but I could transfer it (along with the ViT-large checkpoint) to your organization.

Here's a notebook regarding how I did that :)

Would you be interested in this integration?

Kind regards,

Niels ML @ HF

lpiccinelli-eth commented 3 months ago

Thank you for your contribution!

I will review and test it asap

NielsRogge commented 3 months ago

Thanks! Let me know if you need any help. Would be great if you could try it out to upload the various checkpoints to the hub!

lpiccinelli-eth commented 3 months ago

LGTM

NielsRogge commented 3 months ago

Thanks for merging my PR! ๐Ÿค—

Would you be up for pushing the various checkpoints to the hub as shown in this notebook?

This way you'll get download metrics for your various checkpoints. Also feel free to update the README/sample code so that people can start using them!

lpiccinelli-eth commented 3 months ago

We tested them and pushed the checkpoints on the hub as you suggested, now we are updating the README, thanks again!

NielsRogge commented 3 months ago

Awesome!! Btw I was actually just working on porting ZoeDepth to the Transformers library (https://github.com/huggingface/transformers/pull/30136), since it was SOTA on metric depth estimation. Guess that's no longer the case ;)

If you would be up for contributing your model to the Transformers library, let me know!

lpiccinelli-eth commented 3 months ago

That would be really cool for users, we are releasing version 2 soon (faster, more flexible architecture and larger and corrected dataset), that would be nice to be integrated in transformers!

NielsRogge commented 3 months ago

Ok great!

Btw is it possible that the Mixin class wasn't leveraged to push the following 2 checkpoints to the hub?

Because normally a model card would be created automatically, as well as tags. Or perhaps the huggingface_hub version was outdated?

lpiccinelli-eth commented 3 months ago

Thank you for pointing this out and yes, you are right, my huggingface_hub was outdated, now I updated it and amended the requirements, too, thanks!

NielsRogge commented 3 months ago

Great! Btw @wauplin (maintainer of huggingface_hub) pointed out that we could merge both classes (UniDepthV1 and UniDepthHF) into one.

The reason I created a separate UniDepthHF class was only because there was already a from_pretrained method defined.

Basically there's no need for torch hub anymore, since I see that even the torch hub code loads the checkpoints from the ๐Ÿค— hub ๐Ÿ˜… (but without download metrics working due to use of hf_hub_download).

Do you want me to submit a PR to simplify things?

lpiccinelli-eth commented 3 months ago

If you want to open a PR to clean it, I appreciate it. I would like to keep hubconf.py still, just in case someone likes it more for some reasons :sweat_smile:

NielsRogge commented 3 months ago

Looks like download metrics are working ๐Ÿ™Œ

Btw, those model cards look pretty empty.. ๐Ÿ˜‰

Screenshot 2024-04-12 at 21 34 45
NielsRogge commented 3 months ago

Opened a PR to simplify things: #14