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

Feature registries for models #153

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

theabhirath
Copy link
Member

This is a first attempt at using FeatureRegistries.jl to allow the user to view the models available in Metalhead.jl. This is still a work in progress though - most notably, the time taken for using Metalhead rises significantly because of the extra code being executed.

It looks pretty though:
Screenshot 2022-05-13 at 4 42 39 PM

This is a first attempt at using FeatureRegistries.jl to allow the user to view the models available in Metalhead.jl. This is still a work in progress though - most notably, the time taken for `using Metalhead` rises significantly because of the extra code being executed.
@darsnack
Copy link
Member

Looks great! Seems like this is related to #117, i.e. having a repo separate for "just the models" and then a "vision" repo. The latter can take on heavier deps. Maybe the models should go in MetalheadModels.jl?

@theabhirath
Copy link
Member Author

That sounds great! Metalhead can simply call using MetalheadModels.jl then.

One question I have regarding this PR, though, is that the major rise in time for using Metalhead seems to be not from using FeatureRegistries (which although not negligible, is still only about 200 ms). The rise in time is more around 3-4 seconds, which is mainly because at every using call, the parameters of every model are being initiated and summed over. We could theoretically just store the numbers but that doesn't seem so nice. If we continue to give parameter information programmatically, then maybe we need a way to lazy load the numbers only when the function is called - not sure if there is such a method. The other option, of course, is simply to give a different set of information. Would the docstring make more sense here? It would also help the user understand the variants for the model

@darsnack
Copy link
Member

Yeah a docstring seems more useful than number of parameters. Maybe a description of the configurations too?

Also correct some docstrings to add warnings for no pretrained models being present yet
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.

2 participants