-
Notifications
You must be signed in to change notification settings - Fork 169
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
Conversation
|
||
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 = { |
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.
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.
Codecov Report
@@ 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 |
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: |
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.
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?
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.
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. |
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.
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 */
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.
Will do
Co-authored-by: Max Ryabinin <[email protected]>
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)
(cherry picked from commit d90a14d)
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).