Improve discoverability on the hub #325
Conversation
There was a problem hiding this comment.
Thanks for the PR @NielsRogge! Looks like a good addition to me :)
Co-authored-by: Lucain <lucainp@gmail.com>
Co-authored-by: Lucain <lucainp@gmail.com>
Co-authored-by: Lucain <lucainp@gmail.com>
| class ACTPolicy(nn.Module, PyTorchModelHubMixin, | ||
| library_name="lerobot", | ||
| repo_url="https://github.com/huggingface/lerobot", | ||
| tags=["act-policy", "robotics"], |
There was a problem hiding this comment.
Since "act-policy" is added as a tag here, it would make sense to add the policy as a tag for DiffusionPolicy, TDMPCPolicy and VQBeTPolicy as well.
There was a problem hiding this comment.
Thank you a lot @NielsRogge, this is a great addition!
I left a few minor comments, almost ready to merge for me.
Also, I synced with main to fix the CI as we recently fixed a bug in #361 which you still had ;)
And thank you @Wauplin for the review!
| PyTorchModelHubMixin, | ||
| library_name="lerobot", | ||
| repo_url="https://github.com/huggingface/lerobot", | ||
| tags=["robotics", "act-policy"], |
There was a problem hiding this comment.
EDIT: Deleted suggestion
There was a problem hiding this comment.
Personal preference but I'd rather set a single tag act-policy (and same for the other policies). Feels more self-explanatory to me + in the UI it will allow to click on the act-policy tag to list all models with it. If split in two tags, you can filter models with act but some might not even be policies or robotics related. Especially in the case of diffusion-policy where "diffusion" alone can mean everything.
There was a problem hiding this comment.
Ha I see, I updated my comments. Thanks for the tip.
aliberts
added
✨ Enhancement
Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com>
Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com>
Co-authored-by: Lucain <lucainp@gmail.com>
Co-authored-by: Lucain <lucainp@gmail.com>
|
As you prefer. It feels weird to me not to have the same naming convention between |
|
@alexander-soare, @Cadene WDYT? |
In response to the "policy" naming. I think Diffusion Policy is the only one that needs it in the official name. Then we decided in our code to append Anyway, I think this weirdness should be isolated to just our class naming. Everything else should just be "act", "vqbet", "diffusion-policy", "tdmpc". |
What this does
Great to see you're leveraging the PyTorchModelHubMixin (wondering how you discovered it? ;) )
This PR aims to improve the discoverability of models pushed using lerobot on the hub. The Mixin class has undergone quite some improvements, you can now also include metadata which will get pushed automatically into the model card.
Refer to the docs: https://huggingface.co/docs/huggingface_hub/package_reference/mixins#huggingface_hub.PyTorchModelHubMixin
cc @Wauplin