Summary:
Ignore mixed upper-case/lower-case style for now
Fix space between function and its arguments violation
Pull Request resolved: https://github.com/pytorch/pytorch/pull/35574
Test Plan: CI
Differential Revision: D20712969
Pulled By: malfet
fbshipit-source-id: 0012d430aed916b4518599a0b535e82d15721f78
Summary:
---
How does the current code subsume all detections in the deleted `nccl.py`?
- The dependency of `USE_NCCL` on the OS and `USE_CUDA` is handled as dependency options in `CMakeLists.txt`.
- The main NCCL detection happens in [FindNCCL.cmake](8377d4b32c/cmake/Modules/FindNCCL.cmake), which is called by [nccl.cmake](8377d4b32c/cmake/External/nccl.cmake). When `USE_SYSTEM_NCCL` is false, the previous Python code defer the detection to `find_package(NCCL)`. The change in `nccl.cmake` retains this.
- `USE_STATIC_NCCL` in the previous Python code simply changes the name of the detected library. This is done in `IF (USE_STATIC_NCCL)`.
- Now we only need to look at how the lines below line 20 in `nccl.cmake` are subsumed. These lines list paths to header and library directories that NCCL headers and libraries may reside in and try to search these directories for the key header and library files in turn. These are done by `find_path` for headers and `find_library` for the library files in `FindNCCL.cmake`.
* The call of [find_path](https://cmake.org/cmake/help/v3.8/command/find_path.html) (Search for `NO_DEFAULT_PATH` in the link) by default searches for headers in `<prefix>/include` for each `<prefix>` in `CMAKE_PREFIX_PATH` and `CMAKE_SYSTEM_PREFIX_PATH`. Like the Python code, this commit sets `CMAKE_PREFIX_PATH` to search for `<prefix>` in `NCCL_ROOT_DIR` and home to CUDA. `CMAKE_SYSTEM_PREFIX_PATH` includes the standard directories such as `/usr/local` and `/usr`. `NCCL_INCLUDE_DIR` is also specifically handled.
* Similarly, the call of [find_library](https://cmake.org/cmake/help/v3.8/command/find_library.html) (Search for `NO_DEFAULT_PATH` in the link) by default searches for libraries in directories including `<prefix>/lib` for each `<prefix>` in `CMAKE_PREFIX_PATH` and `CMAKE_SYSTEM_PREFIX_PATH`. But it also handles the edge cases intended to be solved in the Python code more properly:
- It only searches for `<prefix>/lib64` (and `<prefix>/lib32`) if it is appropriate on the system.
- It only searches for `<prefix>/lib/<arch>` for the right `<arch>`, unlike the Python code searches for `lib/<arch>` in a generic way (e.g., the Python code searches for `/usr/lib/x86_64-linux-gnu` but in reality systems have `/usr/lib/x86_64-some-customized-name-linux-gnu`, see https://unix.stackexchange.com/a/226180/38242 ).
---
Regarding for relevant issues:
- https://github.com/pytorch/pytorch/issues/12063 and https://github.com/pytorch/pytorch/issues/2877: These are properly handled, as explained in the updated comment.
- https://github.com/pytorch/pytorch/issues/2941 does not changes NCCL detection specifically for Windows (it changed CUDA detection).
- b7e258f81ef61d19b884194cdbcd6c7089636d46 A versioned library detection is added, but the order is reversed: The unversioned library becomes preferred. This is because normally unversioned libraries are linked to versioned libraries and preferred by users, and local installation by users are often unversioned. Like the document of [find_library](https://cmake.org/cmake/help/v3.8/command/find_library.html) suggests:
> When using this to specify names with and without a version suffix, we recommend specifying the unversioned name first so that locally-built packages can be found before those provided by distributions.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/22930
Differential Revision: D16440275
Pulled By: ezyang
fbshipit-source-id: 11fe80743d4fe89b1ed6f96d5d996496e8ec01aa
Summary:
This tests the water for adding back NNPACK in PyTorch, it's a lot better than the fallback THNN versions.
In #6151, we (ezyang and soumith) removed NNPACK support from PyTorch. Of course Maratyszcza might have advice, too. (Or an opinion on the CMake changes.)
The only functional changes are to use NNPack more aggressively on mobile and a .contiguous() to match NNPack's assumption (I stumbled over that while using NNPack for style transfer.)
The CMake changes try to use the NNPack we already have in git.
In terms of lines of code this is a large part of the diff of https://lernapparat.de/pytorch-jit-android/ . As far as I can tell, we don't have MKLDNN on mobile and the native THNN implementation are prohibitively expensive in terms of both CPU and memory.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/15924
Differential Revision: D13709576
Pulled By: ezyang
fbshipit-source-id: f2e287739909451c173abf046588209a7450ca2c
Summary:
This has 4 changes
1) propagate USE_SYSTEM_NCCL. Previously it was ignored and cmake always did a FindPackage
2) respect SCCACHE_DISABLE in our caffe2 sccache wrapper for circleci
3) use SCCACHE_DISABLE when building nccl, because it triggers the same bug as when using CCACHE (already tracked in https://github.com/pytorch/pytorch/issues/13362). This was hidden because we weren't respecting USE_SYSTEM_NCCL, and were never building nccl ourselves in CI
4) In one particular CI configuration (caffe2, cuda 8, cudnn 7), force USE_SYSTEM_NCCL=1. Building the bundled nccl triggers a bug in nvlink. I've done some investigation, but this looks like a tricky, preexisting bug, so rather than hold up this diff I'm tracking it separately in https://github.com/pytorch/pytorch/issues/14486
Pull Request resolved: https://github.com/pytorch/pytorch/pull/14195
Differential Revision: D13237502
Pulled By: anderspapitto
fbshipit-source-id: 1100ac1269c7cd39e2e0b3ba12a56a3ce8977c55
Summary:
I don't have a full analysis, but ccache appears to often fail while
nccl. To work around this, run the NCCL build with CCACHE_DISABLE.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/13340
Differential Revision: D12855467
Pulled By: anderspapitto
fbshipit-source-id: 63eb12183ab9d03dd22090f084688ae6390fe8bd
Summary:
always build nccl from within the main cmake build, rather than via a separate invocation in build_pytorch_libs.sh. Use the existing caffe2 codepaths
Pull Request resolved: https://github.com/pytorch/pytorch/pull/13150
Differential Revision: D12815674
Pulled By: anderspapitto
fbshipit-source-id: a710b6f242d159b9816911a25ee2c4b8c3f855aa
Summary:
- Removed the old nccl file
- Make open-source NCCL a submodule
- CMake to make NCCL itself
NCCL2 now is in the default build.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/12359
Reviewed By: orionr, yns88
Differential Revision: D10219665
Pulled By: teng-li
fbshipit-source-id: 134ff47057512ba617b48bf390c1c816fff3f881
Summary:
- Removed the old nccl file
- Make open-source NCCL a submodule
- CMake to make NCCL itself
NCCL2 now is in the default build.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/12312
Differential Revision: D10190845
Pulled By: teng-li
fbshipit-source-id: 08d42253b774149a66919d194f88b34628c39bae
* Have PyTorch depend on minimal libcaffe2.so instead of libATen.so
* Build ATen tests as a part of Caffe2 build
* Hopefully cufft and nvcc fPIC fixes
* Make ATen install components optional
* Add tests back for ATen and fix TH build
* Fixes for test_install.sh script
* Fixes for cpp_build/build_all.sh
* Fixes for aten/tools/run_tests.sh
* Switch ATen cmake calls to USE_CUDA instead of NO_CUDA
* Attempt at fix for aten/tools/run_tests.sh
* Fix typo in last commit
* Fix valgrind call after pushd
* Be forgiving about USE_CUDA disable like PyTorch
* More fixes on the install side
* Link all libcaffe2 during test run
* Make cuDNN optional for ATen right now
* Potential fix for non-CUDA builds
* Use NCCL_ROOT_DIR environment variable
* Pass -fPIC through nvcc to base compiler/linker
* Remove THCUNN.h requirement for libtorch gen
* Add Mac test for -Wmaybe-uninitialized
* Potential Windows and Mac fixes
* Move MSVC target props to shared function
* Disable cpp_build/libtorch tests on Mac
* Disable sleef for Windows builds
* Move protos under BUILD_CAFFE2
* Remove space from linker flags passed with -Wl
* Remove ATen from Caffe2 dep libs since directly included
* Potential Windows fixes
* Preserve options while sleef builds
* Force BUILD_SHARED_LIBS flag for Caffe2 builds
* Set DYLD_LIBRARY_PATH and LD_LIBRARY_PATH for Mac testing
* Pass TORCH_CUDA_ARCH_LIST directly in cuda.cmake
* Fixes for the last two changes
* Potential fix for Mac build failure
* Switch Caffe2 to build_caffe2 dir to not conflict
* Cleanup FindMKL.cmake
* Another attempt at Mac cpp_build fix
* Clear cpp-build directory for Mac builds
* Disable test in Mac build/test to match cmake
Caffe2 started with an option to use NNPACK pre-installed in the system.
Now this option is mostly legacy, as Caffe2 can include NNPACK in its own build on all platforms.
Due to problems when pre-installed NNPACK is built with different dependencies or compiler options, we decided to remove this option and alwyas build NNPACK with Caffe2.
This change makes Caffe2 always build NNPACK as part of its own build, and updates NNPACK and cpuinfo submodules.
CMake 3.2 is required to properly track dependencies in projects imported as ExternalProject_Add (BUILD_BYPRODUCTS parameter).
Users on Ubuntu 14.04 LTS would need to install and use cmake3 package for configurations. Users of other popular distributions generally have a recent enough CMake package.
* [cmake] Move nccl to modern cmake, and avoid using EXTERNAL_DEPENDENCIES
* [cmake] Move nnpack to modern cmake and avoid using EXTERNAL_DEPENDENCIES.
* [cmake] Move ATen to modern cmake and avoid using EXTERNAL_DEPENDENCIES.
* Move cpufeatures to modern cmake, and avoid using EXTERNAL_DEPENDENCIES
* Finally remove EXTERNAL_DEPENDENCIES.
* Maratyszcza's comments
Summary:
Include six, enum34, and PeachPy as Caffe2 submodules, and use the versions from submodules instead of downloading them during configuration time
Closes https://github.com/caffe2/caffe2/pull/1917
Reviewed By: orionr
Differential Revision: D6938735
Pulled By: Maratyszcza
fbshipit-source-id: 841a6c47a1cd003a19f48f6c256aa4d9eb2cc6e4
Summary:
Original commit changeset: d0c1c7681605
Reverting due to broken OSS build due to this commit
Reviewed By: bddppq
Differential Revision: D6935666
fbshipit-source-id: 955cfeb6d5a4ed265b2e099094cfb5bfe960ff95
Summary:
Include six, enum34, and PeachPy as Caffe2 submodules, and use the versions from submodules instead of downloading them during configuration time
Closes https://github.com/caffe2/caffe2/pull/1901
Differential Revision: D6930731
Pulled By: Maratyszcza
fbshipit-source-id: d0c1c7681605d957de6f51bd24fbb25afc0f282f
Summary:
- Fix path to FXdiv and FP16 dependencies
- Link cpuinfo library
- Pull NNPACK fix for PYTHONPATH handling when launching PeachPy
- Pull cpuinfo fix for cross-compiling on Linux for Android
- Pull cpuinfo fix for CPUINFO_LIBRARY_TYPE support
- Pull cpuinfo fix for iOS builds
Closes https://github.com/caffe2/caffe2/pull/1869
Differential Revision: D6881428
Pulled By: Maratyszcza
fbshipit-source-id: 7b4115daa090096dbd97303503792e7b144fbb43
Summary:
Few people complained in NNPACK repo about broken build on PPC64, as it specifically whitelists supported architecture in its CMakeLists.txt, and refuses to build on unsupported platforms. This commit explicitly disables NNPACK build (as part of Caffe2 build) on unsupported architectures.
Closes https://github.com/caffe2/caffe2/pull/1439
Differential Revision: D6288999
Pulled By: Maratyszcza
fbshipit-source-id: 76c40e9ce882356944b63968df8fd853f21ecd35
Summary:
If this variable is set to a ccache symlink then the NCCL build will
also use the cache. The NCCL build is the slowest component of a cached
build without this change
Closes https://github.com/caffe2/caffe2/pull/1416
Reviewed By: Yangqing
Differential Revision: D6214008
Pulled By: pietern
fbshipit-source-id: e0a90e27de9b1c5a1fdc0e5bad5fb61f9fa924c3
Summary:
NNPACK now supports building with CMake, and its build scripts have advantages over the ones in Caffe2:
- They automatically download all dependencies, no need to keep them in submodules anymore
- They automatically download and setup PeachPy for x86-64 build
- The same scripts are used for server/desktop (Linux, macOS) and mobile (Android/iOS)
- They unblock Caffe2 build with Ninja
Closes https://github.com/caffe2/caffe2/pull/1382
Reviewed By: Yangqing
Differential Revision: D6150723
Pulled By: Maratyszcza
fbshipit-source-id: 7c3e4e3406f60d4cc059e1c8112cb10aa3d75ece
Summary:
This introduces a few things:
- It enables us to create Caffe2Config.cmake that can be used down the road for building dependent libraries, so they do not need to explicitly write FindCaffe2.cmake.
- The config file will automatically figure out transitive dependency of Caffe2 as well as compiler flags.
- This diff also disables the RPATH setting since it is kind of a mess right now. In principle, we should figure out a clearer rpath setting following the typical rpath setting choices (https://cmake.org/Wiki/CMake_RPATH_handling) - I can send a follow up PR to clean this up.
- Minor: removed old gflags ang glog files.
Closes https://github.com/caffe2/caffe2/pull/1354
Reviewed By: dzhulgakov
Differential Revision: D6098014
Pulled By: Yangqing
fbshipit-source-id: cb06c41a7ef60fddb78b24887b6b3e82684b7c6b
Summary:
Use HINTS instead of PATHS for find_library so that you can specify
-DNCCL_ROOT_DIR and it will use this NCCL installation regardless of
what else is installed on your system. Also add a path hint to include
the default base path for NCCL 2 libraries.
Closes https://github.com/caffe2/caffe2/pull/1152
Reviewed By: Yangqing
Differential Revision: D5740053
Pulled By: pietern
fbshipit-source-id: 43f0908a63e8a9b90320dece0bbb558827433b48
Summary:
I closed https://github.com/caffe2/caffe2/pull/736 because one of these variables should be used after all.
Here's how C1 uses this variable: https://github.com/BVLC/caffe/blob/rc5/cmake/Targets.cmake#L116
Without this fix, there is a race condition in the parallel build leading to this error:
```
make[2]: *** No rule to make target `../third_party/NNPACK/lib/libnnpack.a', needed by `caffe2/libCaffe2_CPU.so'.
```
Closes https://github.com/caffe2/caffe2/pull/737
Differential Revision: D5211794
Pulled By: Yangqing
fbshipit-source-id: 9e368f09b01edaf86252727adc6f6cc40d244e29
Summary:
This was found necessary on some CentOS. aaronmarkham
Closes https://github.com/caffe2/caffe2/pull/240
Differential Revision: D4819591
Pulled By: Yangqing
fbshipit-source-id: 40161cd484a2c8d43f26077919ad2762440dde13
Summary:
This also requires a change to cmake/External/nccl.cmake to use the
static NCCL binary instead of the shared object. When the Caffe2/Gloo
build uses the bundled NCCL version it should be packaged up in the
resulting libraries and not cause another runtime dependency on a
library that has to be installed separately.
Closes https://github.com/caffe2/caffe2/pull/218
Differential Revision: D4769926
Pulled By: pietern
fbshipit-source-id: 5c85559992c200d874f4218724823815ffb5adb5
Summary:
Necessary if CXX isn't set when cmake is called. The CXX variable will then be
empty which prevents make from using its own default.
Closes https://github.com/caffe2/caffe2/pull/202
Differential Revision: D4711113
Pulled By: Yangqing
fbshipit-source-id: 895c07044b263ba9b5440453978248506d7ac225
Summary: This fixes build that include caffe2 and change the value of CMAKE_BINARY_DIR to their own binary dir. Allows the generation of protobuf headers/files in particular.
Reviewed By: Yangqing
Differential Revision: D4466126
fbshipit-source-id: eba264094dd2bff07a7f050b95fd2d5525462b09