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

Module.save moves Module to CPU #372

Closed
lostmsu opened this issue Sep 29, 2021 · 6 comments · Fixed by #483
Closed

Module.save moves Module to CPU #372

lostmsu opened this issue Sep 29, 2021 · 6 comments · Fixed by #483

Comments

@lostmsu
Copy link
Contributor

lostmsu commented Sep 29, 2021

Version: 0.93.0.0

In contrast in Python torch.save(module, path) does not.

@NiklasGustafsson
Copy link
Contributor

I did it that way because it was the quick and safe thing to do, and documented it in the article on saving and loading.

If the difference in behavior is truly problematic, it should be relatively straight-forward to address it.

lostmsu added a commit to losttech/Torch.Siren that referenced this issue Nov 17, 2021
(save is moving model to CPU)
@NiklasGustafsson NiklasGustafsson linked a pull request Dec 1, 2021 that will close this issue
@lostmsu
Copy link
Contributor Author

lostmsu commented Dec 2, 2021

@NiklasGustafsson what article are you referring to? The C++ frontend documentation has no remarks about CPU vs CUDA tensors for torch::save.

@NiklasGustafsson
Copy link
Contributor

@lostmsu I'm sorry -- I'm not quite sure I understand what statement of mine you are referring to.

@lostmsu
Copy link
Contributor Author

lostmsu commented Dec 2, 2021

@NiklasGustafsson oh, sorry, I thought

and documented it in the article on saving and loading

refers to PyTorch documentation, not TorchSharp documentation you wrote.

Still I don't see anything in PyTorch C++ documentation, that would suggest, that tensors need to be moved to CPU in order to save them. Does torch::save fail for GPU tensors?

Perhaps it will make more sense to make a CPU copy of the tensor before saving it instead of moving.

@NiklasGustafsson
Copy link
Contributor

NiklasGustafsson commented Dec 2, 2021

Right.

My statement was intended to refer to the TorchSharp article. With this latest PR, the tensor is moved to the CPU, then moved back to where it came from. I'll look into making a copy, instead.

The problem is that the .NET logic to saving and loading tensors is based on Span, which cannot accept a pointer to GPU memory, so it has to be in CPU memory to work.

@NiklasGustafsson
Copy link
Contributor

Turns out, copy works on save(), but not on load().

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 a pull request may close this issue.

2 participants