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.

Sign up for GitHub

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

Jump to bottom

[RLlib] Terminated/Truncated in Quickstart Script #31386

Merged
merged 9 commits into from
Jan 5, 2023
Merged

[RLlib] Terminated/Truncated in Quickstart Script #31386

merged 9 commits into from
Jan 5, 2023

Conversation

ArturNiederfahrenhorst
Copy link
Contributor

@ArturNiederfahrenhorst ArturNiederfahrenhorst commented Jan 2, 2023 •
edited
Loading

Signed-off-by: Artur Niederfahrenhorst [email protected]

Why are these changes needed?

Our quickstart example in the docs has been updated to fit gymnasium, but a docstring is missing the truncated flag and all terminated flags are still called done.

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 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 :(

@ArturNiederfahrenhorst
initial
e2afd5a 
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
@ArturNiederfahrenhorst
add back docs string
5c27cb7 
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
@ArturNiederfahrenhorst
fix reset and comments
bea5e52 
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
@ArturNiederfahrenhorst
add truncated to step
7effc82 
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
@ArturNiederfahrenhorst ArturNiederfahrenhorst marked this pull request as draft January 2, 2023 11:00
@ArturNiederfahrenhorst ArturNiederfahrenhorst changed the title [RLlib] Fix Quick Start / Gymnasium mismatch [RLlib] Terminated/Truncated in Quickstart Script Jan 2, 2023
@ArturNiederfahrenhorst
add truncated flag
194b2e3 
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
@ArturNiederfahrenhorst
Copy link
Contributor Author

Thought I had to fix stuff related to #31321 and #31314 . That's already fixed though so I ended up with these minor changes in comments @sven1977 .

@ArturNiederfahrenhorst ArturNiederfahrenhorst marked this pull request as ready for review January 2, 2023 11:09
@ArturNiederfahrenhorst
lint
762d957 
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
@ArturNiederfahrenhorst
typo
c8227ed 
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
sven1977
sven1977 approved these changes Jan 4, 2023
View reviewed changes
Copy link
Contributor

@sven1977 sven1977 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the cleanup.
Probably more in the docs. I have a docs PR for all the gymnasium changes coming up. :)

@ArturNiederfahrenhorst
Copy link
Contributor Author

Thanks!

@sven1977 sven1977 merged commit e618c8c into ray-project:master Jan 5, 2023
@ArturNiederfahrenhorst ArturNiederfahrenhorst deleted the fixreadmeexample branch January 5, 2023 15:39
AmeerHajAli pushed a commit that referenced this pull request Jan 12, 2023
@ArturNiederfahrenhorst @AmeerHajAli
[RLlib] Terminated/Truncated in Quickstart Script. (#31386)
58e0afa
tamohannes pushed a commit to ju2ez/ray that referenced this pull request Jan 25, 2023
@ArturNiederfahrenhorst @tamohannes
[RLlib] Terminated/Truncated in Quickstart Script. (ray-project#31386)
6389359 
Signed-off-by: tmynn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sven1977 sven1977 sven1977 approved these changes

@gjoliver gjoliver Awaiting requested review from gjoliver

@avnishn avnishn Awaiting requested review from avnishn

@smorad smorad Awaiting requested review from smorad

@maxpumperla maxpumperla Awaiting requested review from maxpumperla

@kouroshHakha kouroshHakha Awaiting requested review from kouroshHakha

@krfricke krfricke Awaiting requested review from krfricke

Assignees

@sven1977 sven1977

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ArturNiederfahrenhorst @sven1977