-
Notifications
You must be signed in to change notification settings - Fork 552
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
Docs: Add warning against installing Ray in base environment #3267
Docs: Add warning against installing Ray in base environment #3267
Conversation
@Michaelvll if you would, please take a look at it and notify for any additional changes |
@@ -137,3 +137,14 @@ This allows you directly to SSH into the worker nodes, if required. | |||
# Worker nodes. | |||
$ ssh mycluster-worker1 | |||
$ ssh mycluster-worker2 | |||
|
|||
|
|||
Executing a Distributed Ray Program |
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.
For discussion: We may want to add a dedicated guide on how to use Ray as part of the user job. That page can use a tip like this. A full YAML example #3195 may help promote this recommendation more directly.
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.
Yes, let’s add a yaml example of starting a distributed job with ray as mentioned above. We can try to move it to a separate page later after this pr is merged.
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.
So for confirmation: I should add the command to run the example in distributed_ray_train, the one that uses FashionMNIST and add the command for tha yaml file and its output as example in the documentation ?
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.
Yes, we can add the yaml file inline with the command to run that yaml : )
any more suggestions ? |
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.
Thanks for fixing this @MysteryManav! Just left several comments.
Co-authored-by: Zhanghao Wu <[email protected]>
Co-authored-by: Zhanghao Wu <[email protected]>
Co-authored-by: Zhanghao Wu <[email protected]>
Co-authored-by: Zhanghao Wu <[email protected]>
Co-authored-by: Zhanghao Wu <[email protected]>
Co-authored-by: Zhanghao Wu <[email protected]>
Co-authored-by: Zhanghao Wu <[email protected]>
thankful for the opportunity and the guidance @Michaelvll |
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.
Thanks for updating the doc @MysteryManav! This seems good to me as a basic version. We should probably have a more detailed explanation for those concept, e.g. the relationship with ray, etc.
cc'ing @concretevitamin for another look.
Co-authored-by: Zhanghao Wu <[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.
Awesome, LGTM too @MysteryManav!
Co-authored-by: Zhanghao Wu <[email protected]>
@concretevitamin @Michaelvll commited a last suggestion just now Thanks |
Docs: Added warning in documentation against installing Ray in base environment #3082
Tested (run the relevant ones):
bash format.sh
pytest tests/test_smoke.py
pytest tests/test_smoke.py::test_fill_in_the_name
bash tests/backward_comaptibility_tests.sh