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

Use proper p2pd binary on macOS #586

Merged
merged 5 commits into from
Aug 27, 2023
Merged

Use proper p2pd binary on macOS #586

merged 5 commits into from
Aug 27, 2023

Conversation

borzunov
Copy link
Member

@borzunov borzunov commented Aug 24, 2023

This is enough for Petals to work (and pass its tests on macOS).

Making hivemind tests work on macOS turns out to be far more difficult due to lots of multiprocessing magic there. I suggest to work on that in a separate PR (see #585).


P2PD_SOURCE_URL = f"https://github.com/learning-at-home/go-libp2p-daemon/archive/refs/tags/{P2PD_VERSION}.tar.gz"
P2PD_BINARY_URL = f"https://github.com/learning-at-home/go-libp2p-daemon/releases/download/{P2PD_VERSION}/"

# The value is sha256 of the binary from the release page
EXECUTABLES = {
Copy link
Member Author

Choose a reason for hiding this comment

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

I made EXECUTABLES when we also wanted to ship p2p-keygen binary. We now moved away from it, since p2p-keygen functionality can be implemented in 3 lines of Python.

@borzunov borzunov changed the title Support macOS p2pd binary Use proper p2pd binary on macOS Aug 24, 2023
@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #586 (57ee262) into master (33a9a41) will decrease coverage by 0.20%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #586      +/-   ##
==========================================
- Coverage   85.45%   85.25%   -0.20%     
==========================================
  Files          81       81              
  Lines        8009     8009              
==========================================
- Hits         6844     6828      -16     
- Misses       1165     1181      +16     

see 5 files with indirect coverage changes

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
if sha256(binary_path) != expected_hash:
binary_url = os.path.join(P2PD_BINARY_URL, executable)
print(f"Downloading {binary_url}")
if sha256(binary_path) != expected_hash:
Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify: do we verify it this way to automatically download the correct binary for a non-Linux system? Would it be more clear if we simply checked that the binary exists?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sometimes devs replace the binary for testing purposes (e.g., to test an update in libp2p-daemon). As far as I understand, this code allows to replace the binary to the standard one when you reinstall hivemind (since the expected behavior in this case is to get a clean installation).

README.md Outdated
@@ -81,8 +81,8 @@ of [Go toolchain](https://golang.org/doc/install) (1.15 or 1.16 are supported).

- __Linux__ is the default OS for which hivemind is developed and tested. We recommend Ubuntu 18.04+ (64-bit), but
other 64-bit distros should work as well. Legacy 32-bit is not recommended.
- __macOS 10.x__ can run hivemind using [Docker](https://docs.docker.com/desktop/mac/install/).
We recommend using [our Docker image](https://hub.docker.com/r/learningathome/hivemind).
- __macOS__ is also supported, but some features may not work properly.
Copy link
Member

@justheuristic justheuristic Aug 25, 2023

Choose a reason for hiding this comment

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

Nit: the phrase "supported, but some features may not work properly" may rise some eyebrows. Consider "macOS is partially supported" or something in the spirit of "most (but not all) of the main features work"

/* not a significant comment, feel free to ignore if you disagree */

Copy link
Member Author

@borzunov borzunov Aug 25, 2023

Choose a reason for hiding this comment

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

Will do

@borzunov borzunov merged commit d90a14d into master Aug 27, 2023
14 checks passed
@borzunov borzunov deleted the macos-binary branch August 27, 2023 00:00
@borzunov borzunov restored the macos-binary branch August 27, 2023 00:00
borzunov added a commit to bigscience-workshop/petals that referenced this pull request Aug 29, 2023
This PR makes both clients and servers work on macOS. Specifically, it:

- Follows learning-at-home/hivemind#586 to run a macOS-compatible `p2pd` binary (both x86-64 and ARM64 are supported)
- Fixes forking issues and tests on macOS, Python 3.10+
- Introduces basic support for serving model blocks on Apple M1/M2 GPUs (torch.mps)
- Increases max number of open files by default (it's not enough on Linux and is really small on macOS)
mryab pushed a commit that referenced this pull request Aug 31, 2023
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