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

load checkpoint from URL #1532

Closed
williamFalcon opened this issue Apr 19, 2020 · 5 comments · Fixed by #1667
Closed

load checkpoint from URL #1532

williamFalcon opened this issue Apr 19, 2020 · 5 comments · Fixed by #1667
Assignees
Labels
feature Is an improvement or enhancement good first issue Good for newcomers help wanted Open to be worked on let's do it! approved to implement

Comments

@williamFalcon
Copy link
Contributor

Let's enable loading weights from a URL directly

Option 1:

Automate it with our current API

Trainer.load_from_checkpoint('http://')

Option 2:

Have a separate method

Trainer.load_from_checkpoint_at_url('http://')

Resources

We can use this under the hood:
(https://pytorch.org/docs/stable/hub.html#torch.hub.load_state_dict_from_url)

Any thoughts on which one is better?
@PyTorchLightning/core-contributors

@williamFalcon williamFalcon added feature Is an improvement or enhancement help wanted Open to be worked on labels Apr 19, 2020
@yukw777
Copy link
Contributor

yukw777 commented Apr 20, 2020

I recently had to implement a similar functionality and I found this library very useful: https://github.com/RaRe-Technologies/smart_open. It handles various protocols other than http(s) like s3 with a very simple interface like python’s default file open. Thought you might be interested.

@jeremyjordan
Copy link
Contributor

slight preference for option 1

@williamFalcon
Copy link
Contributor Author

williamFalcon commented Apr 20, 2020

@yukw777 nice. want to submit a PR?
Seems like overkill for the first instance no? we want to keep this simple.

Maybe we do a v1 that supports http, https only?
Expand functionality in v2 for other protocols?

@Borda Borda added good first issue Good for newcomers let's do it! approved to implement labels Apr 20, 2020
@yukw777
Copy link
Contributor

yukw777 commented Apr 20, 2020

@williamFalcon yeah i can give it a shot. I just read the code of load_state_dict_from_url and it seems like we don't really need to use smart_open for this, so I'll proceed without that. I also prefer Option 1.

A few questions:

  1. Should we provide a default directory to which we'd download the checkpoints? If so, where?
  2. What's your recommendation on writing tests for this? It'd be simplest to have a test checkpoint file uploaded somewhere and just download that...

@justusschock
Copy link
Member

Maybe we'll also use torch cache for that? Since after all it's still a PyTorch model? Or do we want to create yet another cache?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement good first issue Good for newcomers help wanted Open to be worked on let's do it! approved to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants