-
Notifications
You must be signed in to change notification settings - Fork 7k
[RLlib] Merge tuned examples into examples #58893
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] Merge tuned examples into examples #58893
Conversation
Signed-off-by: Mark Towers <[email protected]>
Signed-off-by: Mark Towers <[email protected]>
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.
Code Review
This pull request refactors the repository by merging the tuned_examples directory into the examples directory, aiming for a simpler project structure. The changes primarily consist of moving files and updating their paths in the rllib/BUILD.bazel file. While most of the path updates are correct, I've identified a critical issue where one of the test configurations points to an incorrect file path, which will cause the test to fail. Additionally, I've found a minor, pre-existing issue in a commented-out test that could be addressed now to prevent future problems. Overall, this is a positive structural change for the repository.
Signed-off-by: Mark Towers <[email protected]>
# Conflicts: # rllib/BUILD.bazel
Signed-off-by: Mark Towers <[email protected]>
Signed-off-by: Mark Towers <[email protected]>
Signed-off-by: Mark Towers <[email protected]>
rllib/BUILD.bazel
Outdated
| # | ||
| # This will test python/yaml config files | ||
| # inside rllib/tuned_examples/[algo-name] for actual learning success. | ||
| # inside rllib/examples/algorithm/[algo-name] for actual learning success. |
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.
| # inside rllib/examples/algorithm/[algo-name] for actual learning success. | |
| # inside rllib/examples/algorithms/[algo-name] for actual learning success. |
kamil-kaczmarek
left a comment
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.
Great!
I left few minor requests for edits.
# Conflicts: # rllib/BUILD.bazel # rllib/examples/algorithms/bc/cartpole_bc.py # rllib/examples/algorithms/bc/cartpole_bc_with_offline_evaluation.py # rllib/examples/algorithms/bc/pendulum_bc.py # rllib/examples/algorithms/iql/pendulum_iql.py # rllib/examples/algorithms/marwil/cartpole_marwil.py
Signed-off-by: Mark Towers <[email protected]>
Signed-off-by: Mark Towers <[email protected]>
Signed-off-by: Mark Towers <[email protected]>
kamil-kaczmarek
left a comment
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!
## Description RLlib has a tuned-examples and examples folder however we believe this is a confusing structure which this PR merges Therefore, this PR moves all tuned-example scripts into `examples/algorithms` for the new stack api and `examples/_old_api_stack/algorithms` for the old stack api. --------- Signed-off-by: Mark Towers <[email protected]> Co-authored-by: Mark Towers <[email protected]> Co-authored-by: Kamil Kaczmarek <[email protected]>
Description
RLlib has a tuned-examples and examples folder however we believe this is a confusing structure which this PR merges
Therefore, this PR moves all tuned-example scripts into
examples/algorithmsfor the new stack api andexamples/_old_api_stack/algorithmsfor the old stack api.