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

[Doc] [Tune] Fix tune resources user guide example code #28210

Merged

Conversation

justinvyu
Copy link
Contributor

Why are these changes needed?

The tune resources user guide contained broken code snippets. This PR fixes those, adds some extra clarifying comments, and improves the code style for readability.

Related issue number

Closes #28206

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

@justinvyu justinvyu added the docs An issue or change related to documentation label Aug 31, 2022
@xwjiang2010 xwjiang2010 self-assigned this Aug 31, 2022
@xwjiang2010
Copy link
Contributor

Awesome! Thanks Justin!

@Yard1
Copy link
Member

Yard1 commented Aug 31, 2022

This can be done in a followup PR, but can we move those snippets to doccode, so it can be tested in CI?

@krfricke krfricke merged commit 5cec249 into ray-project:master Aug 31, 2022
Comment on lines +116 to +118
# We choose to use a `PlacementGroupFactory` here to specify trial resources
resource_group = tune.PlacementGroupFactory([{"CPU": 2, "GPU": 1}])
trainable_with_resources = tune.with_resources(trainable, resource_group)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I know this was already like this, but is there reason why this one doesn't just use a dictionary?

XiaodongLv pushed a commit to XiaodongLv/ray that referenced this pull request Sep 2, 2022
The tune resources user guide contained broken code snippets. This PR fixes those, adds some extra clarifying comments, and improves the code style for readability.

Signed-off-by: Justin Yu <[email protected]>
ilee300a pushed a commit to ilee300a/ray that referenced this pull request Sep 12, 2022
The tune resources user guide contained broken code snippets. This PR fixes those, adds some extra clarifying comments, and improves the code style for readability.

Signed-off-by: Justin Yu <[email protected]>
Signed-off-by: ilee300a <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs An issue or change related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue on page /tune/tutorials/tune-resources.html
5 participants