-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[RLlib] Cleanup examples folder #10: Add custom_rl_module.py
example script and matching RLModule example class (tiny CNN)..
#45774
Changes from 8 commits
16ec8ad
0af3228
bbd5ec2
375c7ba
df4192b
65f32d2
b8b459f
682da4d
4f0f949
ee86f90
6f4672a
a30f9b6
587b5f0
5994553
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,12 +91,9 @@ def __init__(self, config: AlgorithmConfig, **kwargs): | |
try: | ||
module_spec: SingleAgentRLModuleSpec = self.config.rl_module_spec | ||
module_spec.observation_space = self._env_to_module.observation_space | ||
# TODO (simon): The `gym.Wrapper` for `gym.vector.VectorEnv` should | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great that this is gone now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it didn't seem to be a problem anymore (e.g. for PPO Pendulum, everything looks completely fine w/o any weird space errors on the Box actions). So I removed this comment. |
||
# actually hold the spaces for a single env, but for boxes the | ||
# shape is (1, 1) which brings a problem with the action dists. | ||
# shape=(1,) is expected. | ||
module_spec.action_space = self.env.envs[0].action_space | ||
module_spec.model_config_dict = self.config.model_config | ||
if module_spec.model_config_dict is None: | ||
module_spec.model_config_dict = self.config.model_config | ||
# Only load a light version of the module, if available. This is useful | ||
# if the the module has target or critic networks not needed in sampling | ||
# or inference. | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not using
TorchMultiCategorical
andTorchMultiDistribution
- things that get assembled inside of theCatalog
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure either, tbh. I just wanted to get the most simple setup automated. I feel like users that just want to "hack together an RLModule" should not be concerned about picking the categorical distr for their CartPole action space :)
Yes, we should extend this method to even more decent defaults, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's continue brainstorming how to simplify the general RLModule experience for the user ...