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

python3Packages.pytorch: fix Darwin build by disabling GCD #94750

Merged
merged 2 commits into from Aug 8, 2020

Conversation

danieldk
Copy link
Contributor

@danieldk danieldk commented Aug 5, 2020

Motivation for this change

Request: could someone with Darwin ofborg privileges trigger a build of python37Packages.pytorch and python38Packages.pytorch? My MacBook is not fast enough that I want to attempt a full build of PyTorch to test this.

PyTorch 1.6.0 has updated the vendored pthreadpool library, which has recently
added support for Grand Central Dispatch. Unfortunately, it uses functionality
(DISPATCH_APPLY_AUTO) that is only available since macOS 10.13, whereas we are
still using 10.12 libraries.

We can't directly pass through option to vendored libraries, since the setup.py
scripts creates/filters the options that are passed to CMake. So, instead, this
adds a small patch that disables the GCD functionality in pthreadpool.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jonringer
Copy link
Contributor

@GrahamcOfBorg build python37Packages.pytorch

PyTorch 1.6.0 has updated the vendored pthreadpool library, which has recently
added support for Grand Central Dispatch. Unfortunately, it uses functionality
(DISPATCH_APPLY_AUTO) that is only available since macOS 10.13, whereas we are
still using 10.12 libraries.

We can't directly pass through option to vendored libraries, since the setup.py
scripts creates/filters the options that are passed to CMake. So, instead, this
adds a small patch that disables the GCD functionality in pthreadpool.
@danieldk danieldk force-pushed the pytorch-darwin-fix branch 2 times, most recently from 02fca59 to fddf279 Compare August 6, 2020 09:45
@danieldk
Copy link
Contributor Author

danieldk commented Aug 6, 2020

Thanks @jonringer ! Turns out pthreadpool attempts to use GCD anyway in a header, even if it's configured not to in CMake. So, I have updated the patch to fix this.

I also found that install_name_tool was still called on the dev output rather than lib. I have fixed that as well.

Hopefully it will build now. It would be nice if someone with Darwin privileges could trigger the build again.

The library name fixup was attempted on the dev output, whereas it should be
applied to the lib output.
@danieldk
Copy link
Contributor Author

danieldk commented Aug 7, 2020

Builds successfully on my MacBook now:

❯ nix-shell -p 'with import ./default.nix {}; python37.withPackages (ps: with ps; [ pytorch ])'
>>> torch.randn((10,)) @ torch.randn((10,))
tensor(6.7712)

So, this is ready for review.

@jonringer
Copy link
Contributor

@GrahamcOfBorg build python37Packages.pytorch

@danieldk
Copy link
Contributor Author

danieldk commented Aug 8, 2020

Timed out 😢.

@FRidh FRidh merged commit bc7b404 into NixOS:master Aug 8, 2020
@danieldk danieldk deleted the pytorch-darwin-fix branch August 8, 2020 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants