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

Add '' to cmake common_paths #12298

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Conversation

zasdfgbnm
Copy link
Contributor

PyTorch is a project built with cmake, and provides cmake packages for applications to link with it. The cmake prefix of PyTorch can be found by running:

python -c 'import torch.utils; print(torch.utils.cmake_prefix_path)'

you will see something like below from the above command:

/home/gaoxiang/.virtualenvs/nvfuser/lib/python3.11/site-packages/torch/share/cmake

Inspecting this directory:

❯ tree /home/gaoxiang/.virtualenvs/nvfuser/lib/python3.11/site-packages/torch/share/cmake
/home/gaoxiang/.virtualenvs/nvfuser/lib/python3.11/site-packages/torch/share/cmake
├── ATen
│   └── ATenConfig.cmake
├── Caffe2
│   ├── Caffe2Config.cmake
│   ├── Caffe2Targets.cmake
│   ├── Caffe2Targets-release.cmake
│   ├── FindCUDAToolkit.cmake
│   ├── FindCUSPARSELT.cmake
│   ├── Modules_CUDA_fix
│   │   ├── FindCUDA.cmake
│   │   ├── FindCUDNN.cmake
│   │   └── upstream
│   │       ├── CMakeInitializeConfigs.cmake
│   │       ├── FindCUDA
│   │       │   ├── make2cmake.cmake
│   │       │   ├── parse_cubin.cmake
│   │       │   ├── run_nvcc.cmake
│   │       │   └── select_compute_arch.cmake
│   │       ├── FindCUDA.cmake
│   │       ├── FindPackageHandleStandardArgs.cmake
│   │       └── FindPackageMessage.cmake
│   └── public
│       ├── cuda.cmake
│       ├── gflags.cmake
│       ├── glog.cmake
│       ├── LoadHIP.cmake
│       ├── mkl.cmake
│       ├── mkldnn.cmake
│       ├── protobuf.cmake
│       └── utils.cmake
├── Tensorpipe
│   ├── TensorpipeTargets.cmake
│   └── TensorpipeTargets-release.cmake
└── Torch
    ├── TorchConfig.cmake
    └── TorchConfigVersion.cmake

9 directories, 28 files

However, meson currently filters this directory out by _preliminary_find_check. As a result, doing

torch_dep = dependency('Torch')

will fail, even if you set cmake_prefix_path with the value returned by PyTorch.

Possibly related issues:
https://stackoverflow.com/questions/68884434/libtorch-c-meson-dependency
#9740
https://discuss.pytorch.org/t/libtorch-meson-build/139648

@eli-schwartz
Copy link
Member

The current commit message states the mechanical nature of the change: "add such and such string to XXX". It would be best to explain in the commit message why this change is useful.

I'm no cmake expert but I think what it's doing is allowing you to pass the value of the share/cmake/ directory as cmake_prefix_path although normally that directory is supposed to be set to the base directory which contains "lib" and "include" and "share/cmake" subdirectories.

Am I understanding correctly?

@zasdfgbnm
Copy link
Contributor Author

@eli-schwartz Yes, your understanding is correct. I will update commit message later.

This is to allow passing the path share/cmake/ as cmake_prefix_path.
This is a case supported by cmake and is relied on by PyTorch.

The cmake prefix of PyTorch can be found by running:
python -c 'import torch.utils; print(torch.utils.cmake_prefix_path)'

you will see something like below from the above command:
/home/gaoxiang/.virtualenvs/nvfuser/lib/python3.11/site-packages/torch/share/cmake
Inspecting this directory:

❯ tree /home/gaoxiang/.virtualenvs/nvfuser/lib/python3.11/site-packages/torch/share/cmake
/home/gaoxiang/.virtualenvs/nvfuser/lib/python3.11/site-packages/torch/share/cmake
├── ATen
│   └── ATenConfig.cmake
├── Caffe2
│   ├── Caffe2Config.cmake
│   ├── Caffe2Targets.cmake
│   ├── Caffe2Targets-release.cmake
│   ├── FindCUDAToolkit.cmake
│   ├── FindCUSPARSELT.cmake
│   ├── Modules_CUDA_fix
│   │   ├── FindCUDA.cmake
│   │   ├── FindCUDNN.cmake
│   │   └── upstream
│   │       ├── CMakeInitializeConfigs.cmake
│   │       ├── FindCUDA
│   │       │   ├── make2cmake.cmake
│   │       │   ├── parse_cubin.cmake
│   │       │   ├── run_nvcc.cmake
│   │       │   └── select_compute_arch.cmake
│   │       ├── FindCUDA.cmake
│   │       ├── FindPackageHandleStandardArgs.cmake
│   │       └── FindPackageMessage.cmake
│   └── public
│       ├── cuda.cmake
│       ├── gflags.cmake
│       ├── glog.cmake
│       ├── LoadHIP.cmake
│       ├── mkl.cmake
│       ├── mkldnn.cmake
│       ├── protobuf.cmake
│       └── utils.cmake
├── Tensorpipe
│   ├── TensorpipeTargets.cmake
│   └── TensorpipeTargets-release.cmake
└── Torch
    ├── TorchConfig.cmake
    └── TorchConfigVersion.cmake

9 directories, 28 files

However, meson currently filters this directory out by `_preliminary_find_check`. As a result, doing
torch_dep = dependency('Torch')
will fail, even if you set `cmake_prefix_path` with the value returned by PyTorch.

Possibly related issues:
https://stackoverflow.com/questions/68884434/libtorch-c-meson-dependency
mesonbuild#9740
https://discuss.pytorch.org/t/libtorch-meson-build/139648
@zasdfgbnm
Copy link
Contributor Author

Commit message updated

@tristan957 tristan957 added this to the 1.3.0 milestone Sep 28, 2023
@tristan957
Copy link
Contributor

Meson has a cmake_prefix_path option or --cmake-prefix-path on the command line. I feel like setting that to the output of python -c 'import torch.utils; print(torch.utils.cmake_prefix_path)' makes more sense to me. This could be achieved with a machine file pretty easily.

@tristan957
Copy link
Contributor

Alternatively writing a custom dependency lookup for pytorch might be even better?

@eli-schwartz
Copy link
Member

@tristan957 afaict that's what this PR does. It allows you to set the meson option to that value and have it work -- previously, the value of torch.utils.cmake_prefix_path was a value that cmake would search but meson would reject.

@zasdfgbnm
Copy link
Contributor Author

Meson has a cmake_prefix_path option or --cmake-prefix-path on the command line. I feel like setting that to the output of python -c 'import torch.utils; print(torch.utils.cmake_prefix_path)' makes more sense to me. This could be achieved with a machine file pretty easily.

I am actually setting --cmake-prefix-path to python -c 'import torch.utils; print(torch.utils.cmake_prefix_path)' and it does not work. And this is exactly why I write this PR.

@tristan957
Copy link
Contributor

Ah ok. Now I get it. Thanks for explaining a little bit more.

@tristan957
Copy link
Contributor

@eli-schwartz are we good to merge it?

@eli-schwartz eli-schwartz merged commit 6402f53 into mesonbuild:master Sep 29, 2023
34 checks passed
@zasdfgbnm zasdfgbnm deleted the empty-cmake- branch September 29, 2023 21:27
@nirbheek nirbheek modified the milestones: 1.3.0, 1.2.3 Oct 11, 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.

4 participants