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

Remove extra step in M1 installation instructions #36029

Merged
merged 3 commits into from
Jun 30, 2023

Conversation

pcmoritz
Copy link
Contributor

@pcmoritz pcmoritz commented Jun 2, 2023

Why are these changes needed?

After #35932, nothing special is needed any more on M1.

Related issue number

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 added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • 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 :(

@pcmoritz pcmoritz requested a review from a team as a code owner June 2, 2023 19:48
Copy link
Contributor

Choose a reason for hiding this comment

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

I was able to successfully install in a new conda environment on my laptop, but it may be helpful to verify through a completely fresh M1 (VM?) to confirm if it just works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this out on a fresh M1 now :)

Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>
@pcmoritz pcmoritz changed the title Remove M1 installation instructions Remove extra step in M1 installation instructions Jun 29, 2023
@@ -272,7 +272,7 @@ You can install and use Ray C++ API as follows.
M1 Mac (Apple Silicon) Support
------------------------------

Ray has experimental support for machines running Apple Silicon (such as M1 macs).
Ray supports machines running Apple Silicon (such as M1 macs).
Multi-node clusters are untested. To get started with local Ray development:

#. Install `miniforge <https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-MacOSX-arm64.sh>`_.
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually do you know if these miniforge instructions are still needed? Was this primarily for installing grpcio?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is just a convenient way to get things up and running, that's why I kept it. When I was trying stuff out, these instructions helped to get a good Python distribution installed, that's why I put it back. But no strong opinions here :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see. Reading over this section again now, I feel that there shouldn't be any more unique differences from the other operating systems which are covered by the Official Releases section.

Any thoughts on recommending using conda across different operating systems, rather than just for M1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, we probably shouldn't be in the business of recommending Python distributions to people. On M1 there is a little more ambiguity since e.g. you could use an x86 distribution and Rosetta (which grpc actually recommended for a while) and some of the non-conda distributions might be less well supported.

I'll merge the PR now because it strictly improves things, but happy to remove this section altogether in a follow up PR :)

@pcmoritz pcmoritz merged commit dabf2aa into ray-project:master Jun 30, 2023
@pcmoritz pcmoritz deleted the m1-installation-instructions branch June 30, 2023 04:34
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
After ray-project#35932, nothing special is needed any more on M1.

---------

Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>
Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
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.

2 participants