-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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] New API stack: On by default for BC/MARWIL/CQL. #48599
[RLlib] New API stack: On by default for BC/MARWIL/CQL. #48599
Conversation
…api_stack_on_by_default_ppo
…api_stack_on_by_default_ppo Signed-off-by: sven1977 <svenmika1977@gmail.com> # Conflicts: # rllib/utils/exploration/tests/test_curiosity.py
…api_stack_on_by_default_ppo Signed-off-by: sven1977 <svenmika1977@gmail.com> # Conflicts: # doc/source/rllib/doc_code/rllib_in_60s.py # rllib/algorithms/tests/test_env_runner_failures.py # rllib/utils/exploration/tests/test_curiosity.py
…api_stack_on_by_default_ppo Signed-off-by: sven1977 <svenmika1977@gmail.com> # Conflicts: # rllib/utils/exploration/tests/test_curiosity.py
…api_stack_on_by_default_bc_marwil_cql Signed-off-by: sven1977 <svenmika1977@gmail.com> # Conflicts: # doc/source/rllib/doc_code/rllib_on_ray_readme.py # rllib/algorithms/ppo/ppo.py # rllib/core/learner/tests/test_learner_group.py # rllib/examples/_docs/rllib_on_rllib_readme.py # rllib/execution/rollout_ops.py # rllib/tuned_examples/bc/cartpole_recording.py
…api_stack_on_by_default_bc_marwil_cql
…tack_on_by_default_bc_marwil_cql
…accumulation_of_results_in_algorithm Signed-off-by: sven1977 <svenmika1977@gmail.com> # Conflicts: # rllib/core/learner/tests/test_learner_group.py
# `self.api_stack()` | ||
self.enable_rl_module_and_learner = True | ||
self.enable_env_runner_and_connector_v2 = True | ||
self.api_stack( |
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.
We have to call this method here (unfortunately) to make sure a possibly old API stack exploration_config
gets properly saved (and then restored if necessary).
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.
LGTM. To ensure all offline algos run smoothly, let's put in the following into the configs:
self.dataset_num_iters_per_learner=1 if args.num_learners == 0 else None
@@ -108,19 +108,9 @@ def __init__(self, algo_class=None): | |||
|
|||
# Changes to Algorithm's/SACConfig's default: | |||
|
|||
# `.api_stack()` |
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 put the following in here for the offline algos:
self.dataset_num_iters_per_learner=1 if args.num_learners == 0 else None
…api_stack_on_by_default_bc_marwil_cql
…api_stack_on_by_default_bc_marwil_cql
…api_stack_on_by_default_bc_marwil_cql
…48599) Signed-off-by: JP-sDEV <jon.pablo80@gmail.com>
…48599) Signed-off-by: mohitjain2504 <mohit.jain@dream11.com>
New API stack: On by default for BC/MARWIL/CQL.
Why are these changes needed?
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.