Move Theano-dependent code in garage.theano #245
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There are some docstring errors produced by moved files, but that is fixed in #244. |
ryanjulian
approved these changes
Aug 1, 2018
ryanjulian
force-pushed
the
refactor_garage_theano
branch
from
bbeb71f
to
f8d3ef3
Compare
|
Great work! Let me know if you need me to push this past the CI |
jonashen
reviewed
Aug 1, 2018
| @@ -272,12 +273,12 @@ def init_opt(self): | |||
| policy_updates = self.policy_update_method( | |||
| policy_reg_surr, self.policy.get_params(trainable=True)) | |||
|
|
|||
| f_train_qf = ext.compile_function( | |||
| f_train_qf = tensor_utils.compile_function( | |||
There was a problem hiding this comment.
What's the reason you switched ext to tensor_utils? Just curious.
There was a problem hiding this comment.
Since we want to keep all the Theano-dependent code in garage.theano, and the module garage.misc.ext had both independent and Theano-dependent code, I moved all the latter code into the new module garage.theano.misc.tensor_utils to create the separation.
This is a particular case where not the whole module but only parts of it are only used by the Theano branch, while the other parts are used by both TensorFlow and Theano.
There was a problem hiding this comment.
In generally we have like O(2^n) util.py files when we should really have like O(n), so i'm in favor of merging whichever ones make any sense.
jonashen
approved these changes
Aug 1, 2018
added 19 commits
August 1, 2018 11:13
Remove Theano comments from layers.py, and replace theano log with TensorFlow branch.
The following modules were moved into the garage.theano package: - lasagne_layers - lasagne_powered - network The corresponding imports were modified as well. Also, the module lasagne_helpers was removed since no other module in the project makes reference to it, with the functions in lasagne.layers used instead.
While refactoring garage.core to separate the Theano-dependent code, some PEP8 style errors were found.
All the modules from the distribution package were moved into garage.theano.
While refactoring garage.distributions to separate the Theano-dependent code, some PEP8 style errors were found.
The following modules were moved into the garage.theano package: - ddpg - erwr - npo - ppo - reps - tnpg - trpo - vpg The corresponding imports were modified as well.
While refactoring garage.algos to separate the Theano-dependent code, some PEP8 style errors were found.
The module special.py is used by both the Theano and TensorFlow packages, but it contained some Theano dependent functions. Only those functions were moved into garage.theano to keep the rest of the functions for both distributions in the new module tensor_utils.py. The same thing was done for ext.py. The rest of the modules in garage.misc are independent of Theano.
While refactoring garage.misc to separate the Theano-dependent code, some PEP8 style errors were found.
All modules in garage.optimizers except for minibatch_dataset were moved into the garage.theano package. The corresponding imports were modified as well.
All modules in garage.policies except for base were moved into the garage.theano package. The corresponding imports were modified as well.
The module continuous_mlp_q_function was moved into the garage.theano package. The corresponding imports were modified as well.
All modules in garage.regressors except for product_regressor were moved into the garage.theano package. The corresponding imports were modified as well.
The class ReplayPool found in module utils from garage.sampler, was extracted from the module and moved to garage.theano. The rest of the modules were not modified.
Only the function new_tensor_variable in module util makes use of Theano. It was removed since it's not called anywhere and other versions of it in garage.theano.spaces in modules product, box and discrete are called instead.
The following modules were moved into the garage.theano package: - gaussian_conv_baseline - gaussian_mlp_baseline The corresponding imports were modified as well.
The issues were produced during the refactor: - The function flatten_tensors was called from the incorrect module. - The function from_onehot_n may receive Numpy arrays and regular arrays, so we need to verify the type before checking if the corresponding array is empty.
ghost
force-pushed
the
refactor_garage_theano
branch
from
de9c758
to
ba75aeb
Compare
ghost
mentioned this pull request
|
huzzah! |
krzentner
pushed a commit
to krzentner/garage
that referenced
this pull request
Nov 13, 2018
The following modules were moved from garage.core into garage.theano.core package: lasagne_layers lasagne_powered network Also, the module lasagne_helpers was removed since no other module in the project makes reference to it, with the functions in lasagne.layers used instead. All the modules from garage.distribution package were moved into garage.theano.distribution. The following modules were moved from garage.algos into garage.theano.algos package: ddpg erwr npo ppo reps tnpg trpo vpg Regarding garage.misc, the module special.py is used by both the Theano and TensorFlow packages, but it contained some Theano dependent functions. Only those functions were moved into garage.theano.misc to keep the rest of the functions for both distributions in the new module tensor_utils.py. The same thing was done for ext.py. The rest of the modules in garage.misc are independent of Theano. All modules in garage.optimizers except for minibatch_dataset were moved into the garage.theano.optimizers package. All modules in garage.policies except for base were moved into the garage.theano.policies package. The module continuous_mlp_q_function was moved from garage.q_functions into the garage.theano.q_functions package. All modules in garage.regressors except for product_regressor were moved into the garage.theano.regressors package. The class ReplayPool found in module utils from garage.sampler was extracted from the module and moved to garage.theano.sampler. For garage.envs, only the function new_tensor_variable in module util makes use of Theano. It was removed since it's not called anywhere and other versions of it in garage.theano.spaces in modules product, box and discrete are called instead. The following modules in garage.baselines were moved into the garage.theano.baseline package: gaussian_conv_baseline gaussian_mlp_baseline The corresponding imports for each moved module were modified as well. Finally, all the PEP8 style errors were fixed.
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The following modules were moved from garage.core into
garage.theano.core package:
Also, the module lasagne_helpers was removed since no other module in
the project makes reference to it, with the functions in lasagne.layers
used instead.
All the modules from garage.distribution package were moved into
garage.theano.distribution.
The following modules were moved from garage.algos into
garage.theano.algos package:
Regarding garage.misc, the module special.py is used by both the Theano
and TensorFlow packages, but it contained some Theano dependent
functions. Only those functions were moved into garage.theano.misc to
keep the rest of the functions for both distributions in the new module
tensor_utils.py. The same thing was done for ext.py.
The rest of the modules in garage.misc are independent of Theano.
All modules in garage.optimizers except for minibatch_dataset were moved
into the garage.theano.optimizers package.
All modules in garage.policies except for base were moved into the
garage.theano.policies package.
The module continuous_mlp_q_function was moved from garage.q_functions into the garage.theano.q_functions package.
All modules in garage.regressors except for product_regressor were moved
into the garage.theano.regressors package.
The class ReplayPool found in module utils from garage.sampler was
extracted from the module and moved to garage.theano.sampler.
For garage.envs, only the function new_tensor_variable in module util
makes use of Theano. It was removed since it's not called anywhere and
other versions of it in garage.theano.spaces in modules product, box and
discrete are called instead.
The following modules in garage.baselines were moved into the
garage.theano.baseline package:
The corresponding imports for each moved module were modified as well.
Finally, all the PEP8 style errors were fixed.