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] [docs] CLI user guide #29661

Merged
merged 5 commits into from
Oct 28, 2022
Merged

[RLlib] [docs] CLI user guide #29661

merged 5 commits into from
Oct 28, 2022

Conversation

maxpumperla
Copy link
Contributor

@maxpumperla maxpumperla commented Oct 25, 2022

Signed-off-by: Max Pumperla <[email protected]>
@@ -1,4 +1,3 @@
# config for quick cli testing. does not give meaningful results.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, True, though, should we make this example not learn to the end (e.g. 150 reward)? It's pretty quick with SimpleQ. Otherwise, it's not really a "tuned" example :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, good point. maybe we should move these files to another folder or so? In this case you're absolutely right, but I really want to have a couple of test files that return in a manner of seconds, not minutes - and give me a checkpoint to evaluate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sven1977 ok, I reset the reward for these examples back to 150, makes sense. if we need other, quicker configs, we can add them later.

@@ -0,0 +1,336 @@
---
Copy link
Contributor

@sven1977 sven1977 Oct 25, 2022

Choose a reason for hiding this comment

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

Awesomeness! :) Dumb question. Why not rst, but md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ha, thanks. well, a) I personally prefer markdown, b) the margin directive doesn't work in rst, and c) the yaml header in the file makes the whole thing an executable format.

It's essentially a jupyter notebook in disguise. The serve team almost exclusively used md by now, and I just wanted to give a reference example for you guys to work off of.

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 this great PR @maxpumperla ! The CLI is really dope now.

Just 2 questions & nits, then happy to merge.

@sven1977 sven1977 merged commit 6fa28bd into master Oct 28, 2022
@sven1977 sven1977 deleted the mp_cli_docs branch October 28, 2022 09:46
WeichenXu123 pushed a commit to WeichenXu123/ray that referenced this pull request Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants