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

Much slower than original spinningup tf version #8

Open
zhan0903 opened this issue Nov 17, 2019 · 6 comments
Open

Much slower than original spinningup tf version #8

zhan0903 opened this issue Nov 17, 2019 · 6 comments

Comments

@zhan0903
Copy link

Hi, I ran this pytorch version SAC on Mujoco, which took time almost three times more than the original tf version code? Why did this happen? Is there any way to improve the speed?

@kashif
Copy link
Owner

kashif commented Nov 18, 2019

@zhan0903 thanks for trying it out! So there could be 2 reasons for this:

  1. I am calculating the gradients of the computational graph un-necessarily (thats where TF is better since it only runs the part of the graph that is needed) and a solution might be to add torch.no_grad() wrappers where needed
  2. TF will use the GPU if you are running on a GPU machine whereas currently I only run on the CPU

Do you have a benchmarking setup to test these reasons?

Thanks!
Kashif

@zhan0903
Copy link
Author

@kashif thanks for your response. The TF version does not use the GPU either. I will try torch.no_grad() wrappers . I test your code in my experiments.

Thanks
Han

@kashif
Copy link
Owner

kashif commented Nov 18, 2019

thank you!

@zhan0903
Copy link
Author

@kashif Hi, I used the torch.no_grad() in the backpropagation process for SAC, but it didn't improve the speed. The TF version doesn't use the GPU, but it is faster than the pytorch GPU version (TF version SAC takes 7000 seconds, while the pytorch GPU version takes around 15000 seconds).

@jachiam
Copy link

jachiam commented Feb 2, 2020

I've observed the same thing in the official Spinning Up PyTorch SAC code. For whatever reason, it's just slower, even when you're being very careful to only calculate quantities that are absolutely necessary. I haven't figured out why, yet! Hopefully will crack this eventually.

@kashif
Copy link
Owner

kashif commented Feb 2, 2020

Thanks @jachiam I'll have a look too perhaps after the icml deadline...

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

No branches or pull requests

3 participants