Skip to content

Allow torch.hub.load() to load models from any local directory with a hubconf.py #44204

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.

Sign up for GitHub

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

Jump to bottom
Closed
AdeelH wants to merge 10 commits into from
Closed

Allow torch.hub.load() to load models from any local directory with a hubconf.py #44204

AdeelH wants to merge 10 commits into from

Conversation

@AdeelH
Copy link
Contributor

@AdeelH AdeelH commented Sep 4, 2020 •
edited
Loading

Fixes #43622

  • Moves the model loading part of torch.hub.load() into a new torch.hub._load_local() torch.hub._load_local() function that takes in a path to a local directory that contains a hubconf.py instead of a repo name.
  • Refactors torch.hub.load() so that it now calls torch.hub.load_local() after downloading and extracting the repo.
  • Refactors torch.hub.load() so that it can now take in a local path instead of a repo name. Adds a source='github'|'local' param to choose between the two behaviors.
  • Updates torch.hub docs to include the new function + minor fixes.

@dr-ci
Copy link

dr-ci bot commented Sep 4, 2020 •
edited
Loading

💊 CI failures summary and remediations

As of commit 19d603d (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 26 times.

@AdeelH AdeelH force-pushed the torch.hub.load_local branch from 44cffd0 to ddc17e4 Compare September 5, 2020 08:15
@AdeelH
Copy link
Contributor Author

AdeelH commented Sep 5, 2020

@ailzhang, here's the PR. I don't believe the CI errors are related.

@ngimel ngimel requested a review from ailzhang September 15, 2020 06:25
@ngimel ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 15, 2020
ailzhang
ailzhang reviewed Sep 16, 2020
View reviewed changes
Copy link
Contributor

@ailzhang ailzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Overall looks good!
Btw I feel having something like torch.load(<path_to_local_dir>|<github_repo>, source='github'|'local') could be a better unified user facing API? I'm definitely open to suggestions, let me know!

@AdeelH
Copy link
Contributor Author

AdeelH commented Sep 17, 2020

Btw I feel having something like torch.load(<path_to_local_dir>|<github_repo>, source='github'|'local') could be a better unified user facing API? I'm definitely open to suggestions, let me know!

I don't have a strong opinion either way. The source= way might be easier to extend if we want to add more sources in the future (say, a gitlab or a general uri source) without cluttering the API.

@AdeelH
Copy link
Contributor Author

AdeelH commented Sep 18, 2020

I have changed it to the source='github'|'local' approach and I agree that it does look better from an API perspective.

I have also taken the liberty to expand out the documentation for load() a little bit. Let me know if you agree with the changes.
image

@AdeelH AdeelH requested a review from ailzhang September 18, 2020 18:09
ailzhang
ailzhang approved these changes Sep 18, 2020
View reviewed changes
Copy link
Contributor

@ailzhang ailzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!
I think this is good to go after adding a test for hub.load(source='local') API in https://github.com/pytorch/pytorch/blob/master/test/test_utils.py#L585.

@ailzhang ailzhang force-pushed the torch.hub.load_local branch from adcc61b to 19d603d Compare September 21, 2020 16:37
facebook-github-bot
facebook-github-bot reviewed Sep 21, 2020
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@codecov
Copy link

codecov bot commented Sep 21, 2020 •
edited
Loading

Codecov Report

Merging #44204 into master will increase coverage by 0.00%.
The diff coverage is 93.75%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #44204   +/-   ##
=======================================
  Coverage   67.85%   67.85%           
=======================================
  Files         384      384           
  Lines       50020    50026    +6     
=======================================
+ Hits        33942    33947    +5     
- Misses      16078    16079    +1
Impacted Files Coverage Δ
torch/hub.py 71.20% <93.75%> (+0.68%) ⬆️
torch/testing/_internal/expecttest.py 77.55% <0.00%> (-1.03%) ⬇️
torch/utils/_benchmark/utils/common.py 79.33% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4810365...19d603d. Read the comment docs.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 21, 2020

@ailzhang merged this pull request in 1cab27d.

@AdeelH AdeelH changed the title Add a torch.hub.load_local() function that can load models from any local directory with a hubconf.py Allow torch.hub.load() to load models from any local directory with a hubconf.py Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@ailzhang ailzhang ailzhang approved these changes

Assignees

No one assigned

Labels

Merged open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a torch.hub.load_local() function that can load models from any local directory with a hubconf.py

6 participants

@AdeelH @facebook-github-bot @ailzhang @ngimel @pytorchbot @mruberry