8000 Update Eigen submodule to fix build on macOS with aarch64 by maxhgerlach · Pull Request #3619 · horovod/horovod · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update Eigen submodule to fix build on macOS with aarch64 #3619

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

Merged
merged 2 commits into from
Jul 26, 2022

Conversation

maxhgerlach
Copy link
Collaborator
@maxhgerlach maxhgerlach commented Jul 25, 2022

Checklist before submitting

  • Did you read the contributor guide?
  • Did you update the docs?
  • Did you write any tests to validate this change?
  • Did you update the CHANGELOG, if this change affects users?

Description

On my computer (macOS 12.4, M1 Pro) this fixes building Horovod with PyTorch. Note that the problem does not occur if one builds with TensorFlow, too.

We need to make sure to build with a version of Eigen that's recent enough to include the commit https://gitlab.com/libeigen/eigen/-/commit/fd1dcb6b45a2c797ad4c4d6cc7678ee70763b4ed; else we get duplicate symbol errors. The latest release 3.4.0 includes the fix, which is why I opted for that version: https://gitlab.com/libeigen/eigen/-/tags/3.4.0 (commit hash 3147391d).

When we build with TensorFlow, we don't use Eigen from the submodule, but we use the version of the library that ships with the TF package. For that reason I believe that this change will not affect anything running in the CI: As far as I know, there, we always build with all three frameworks TensorFlow, PyTorch, and MXNet.

Fixes #3605 and #3587.

Review process to land

  1. All tests and other checks must succeed.
  2. At least one member of the technical steering committee must review and approve.
  3. If any member of the technical steering committee requests changes, they must be addressed.

Signed-off-by: Max H. Gerlach <git@maxgerlach.de>
Signed-off-by: Max H. Gerlach <git@maxgerlach.de>
@maxhgerlach maxhgerlach linked an issue Jul 25, 2022 that may be closed by this pull request
@github-actions
Copy link

Unit Test Results

     957 files  +  30       957 suites  +30   10h 8m 53s ⏱️ + 8m 54s
     781 tests ±    0       737 ✔️ ±    0       44 💤 ±    0  0 ±0 
20 765 runs  +746  14 782 ✔️ +466  5 983 💤 +280  0 ±0 

Results for commit 1f10f59. ± Comparison against base commit f17ae0f.

@github-actions
Copy link

Unit Test Results (with flaky tests)

  1 091 files  +     78    1 091 suites  +78   10h 44m 24s ⏱️ + 25m 18s
     781 tests ±       0       737 ✔️ ±       0       44 💤 ±    0  0 ±0 
23 513 runs  +1 468  16 514 ✔️ +1 018  6 999 💤 +450  0 ±0 

Results for commit 1f10f59. ± Comparison against base commit f17ae0f.

@maxhgerlach maxhgerlach merged commit da336f5 into horovod:master Jul 26, 2022
leewyang pushed a commit to leewyang/horovod that referenced this pull request Aug 5, 2022
Signed-off-by: Max H. Gerlach <git@maxgerlach.de>
Signed-off-by: Lee Yang <leey@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

aarch64 build requires newer eigen Installation fails with PyTorch on Mac M1
2 participants
0