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

Updated gradient_descent example #351

Merged
merged 2 commits into from
May 13, 2021
Merged

Updated gradient_descent example #351

merged 2 commits into from
May 13, 2021

Conversation

douweschulte
Copy link
Contributor

We ran into some problems in running the gradient_descent example, these changes made it possible to run the example for us.

(Worked with @chrisalgerges98)

We ran into some problems in running the `gradient_descent` example, these changes made it possible to run the example for us. 

(Worked with @chrisalgerges98)
README.md Outdated
use tch::nn;
use tch::Tensor;
use tch::nn::*;
use tch::*;
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we could avoid the wildcard uses as they bring lots of things in the scope and instead a use tch::kind would be enough?

@douweschulte
Copy link
Contributor Author

Leaving out all wildcards would give the following imports.

use tch::kind;
use tch::nn;
use tch::nn::Module; // for the `forward` method
use tch::nn::OptimizerConfig; // for the `build` method
use tch::Device;
use tch::Tensor;

If you feel this is a better way to do it I will change my PR.

@LaurentMazare
Copy link
Owner

I would slightly lean towards something like this indeed. Note that you can make things a bit more compact if you prefer, e.g.:

use tch::{kind,nn,Device,Tensor};
use tch::nn::{Module,OptimizerConfig};

@douweschulte
Copy link
Contributor Author

Great, it is pushed to this PR. Thanks for the reminder of the compact form of the imports.

@LaurentMazare LaurentMazare merged commit 1603bbc into LaurentMazare:master May 13, 2021
@LaurentMazare
Copy link
Owner

Merged, thanks for the PR!

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 this pull request may close these issues.

2 participants