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

[RLlib] AlgorithmConfig cleanup 02: Restructure methods and properties. #44896

Conversation

sven1977
Copy link
Contributor

@sven1977 sven1977 commented Apr 22, 2024

AlgorithmConfig cleanup 02: Restructure methods and properties:

  • Rename rollouts() into env_runners().
  • Dissolve exploration() into env_runners(explore=.., exploration_config=..).
  • Rename/deprecate some properties: num_rollout_workers -> num_env_runners, num_envs_per_worker -> num_envs_per_env_runner, etc..
  • Tag old API stack properties with \@OldAPIStack.

Why are these changes needed?

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: sven1977 <svenmika1977@gmail.com>
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 added rllib RLlib related issues rllib-newstack rllib-oldstack-cleanup Issues related to cleaning up classes, utilities on the old API stack labels Apr 22, 2024
@sven1977 sven1977 marked this pull request as ready for review April 22, 2024 13:53
Signed-off-by: sven1977 <svenmika1977@gmail.com>
Signed-off-by: sven1977 <svenmika1977@gmail.com>
Signed-off-by: sven1977 <svenmika1977@gmail.com>
Signed-off-by: sven1977 <svenmika1977@gmail.com>
…rithm_config_cleanup_02_restructure_methods

Signed-off-by: sven1977 <svenmika1977@gmail.com>

# Conflicts:
#	rllib/examples/evaluation/evaluation_parallel_to_training.py
Copy link
Collaborator

@simonsays1980 simonsays1980 left a comment

Choose a reason for hiding this comment

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

LGTM. Small changes but a lot of them.

@@ -29,7 +29,7 @@ def get(self):
# __rllib-adv_api_explore_begin__
from ray.rllib.algorithms.algorithm_config import AlgorithmConfig

config = AlgorithmConfig().exploration(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't rollout a typically used name in RL for rolling out the policy for sampling?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally, but I felt like some of these settings in here are less concerned about the rollout process itself, but more about how the EnvRunner (that performs these rollouts) is set up. Let me think about it again a bit ...

Signed-off-by: sven1977 <svenmika1977@gmail.com>
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 merged commit 10ec614 into ray-project:master Apr 25, 2024
5 checks passed
@sven1977 sven1977 deleted the algorithm_config_cleanup_02_restructure_methods branch April 25, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rllib RLlib related issues rllib-newstack rllib-oldstack-cleanup Issues related to cleaning up classes, utilities on the old API stack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants