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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FR][hub] load_state_dict_from_url should avoid using tempfile or allow specifying tmp dir #23607

Closed
ssnl opened this issue Jul 31, 2019 · 3 comments
Labels
module: hub triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@ssnl
Copy link
Collaborator

ssnl commented Jul 31, 2019

馃殌 Feature

The torch.hub.load_state_dict_from_url function

def load_state_dict_from_url(url, model_dir=None, map_location=None, progress=True):

calls _download_url_to_file which creates a temp file and download to that first, does sha check, and them move to the target location.

f = tempfile.NamedTemporaryFile(delete=False)

When the model is huge and /tmp is small, users can not actually finish downloading. And there is no way to specify the tmp location.

Alternatives

I see two potential solutions here:

  1. Allow specifying tmp_folder
  2. By default download to file_name + random_uuid, and use that as a temp file. This way we are always downloading to the same disk and will not have such issues.
@ssnl
Copy link
Collaborator Author

ssnl commented Jul 31, 2019

cc @ailzhang for comments!

@ailzhang
Copy link
Contributor

Hi @ssnl I think I have a fix for it.

We can specify tempfile.TemporaryFile(delete=False, dir=dst) in _download_url_to_file and place the file directly in the dst folder. It seems much simpler this way :D

If this sounds good to you, I can quickly make a PR later today, although this might not go in 1.2 release :P

@VitalyFedyunin VitalyFedyunin added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jul 31, 2019
@ssnl
Copy link
Collaborator Author

ssnl commented Jul 31, 2019

@ailzhang That sounds a pretty elegant fix. Thanks so much for the quick reply! :)
There is no rush. I live on the edge and run my code on master builds anyways :D

ssnl pushed a commit to ssnl/pytorch that referenced this issue Aug 2, 2019
Summary:
Fixes pytorch#23607
Pull Request resolved: pytorch#23629

Differential Revision: D16594223

Pulled By: soumith

fbshipit-source-id: db0275415111f08fc13ab6be00b76737a20f92df
soumith pushed a commit that referenced this issue Aug 2, 2019
Summary:
Fixes #23607
Pull Request resolved: #23629

Differential Revision: D16594223

Pulled By: soumith

fbshipit-source-id: db0275415111f08fc13ab6be00b76737a20f92df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: hub triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants