Repository navigation
Commit 730b77a
authored
Build PyTorch wheel via PEP 517 without poisoning the CI image (pytorch#21685)
Forward fix for pytorch#21562.
pytorch#21562 moved the PyTorch wheel build from `setup.py bdist_wheel` to
`python -m build --wheel --no-isolation`. That direction is correct and
is kept here. The problem was that it installed the PEP 517 build
requirements permanently into the image's conda environment, and two
different things in that install changed how every later build in the
image behaves. Both broke main.
### 1. All 17 RISC-V jobs failed
scikit-build-core ships a setuptools plugin that registers itself as the
`build_ext` command class. Once it is present in the environment, any
later `pip install --no-build-isolation` picks it up. The tokenizers
build is exactly that, so its CMake configure started running through
scikit-build-core, which turns on C++20 module dependency scanning. The
image compiler cannot produce the scan output:
```
[299/312] Generating CXX dyndep file CMakeFiles/pytorch_tokenizers_cpp.dir/CXX.dd
FAILED: CMakeFiles/pytorch_tokenizers_cpp.dir/src/python_bindings.cpp.o
cc1plus: error: to generate dependencies you must specify either '-M' or '-MM'
```
### 2. The wheel was built without LAPACK
The same install also put a pip `cmake` wheel into the environment,
which lands on PATH ahead of the image's own cmake. The two are the same
version but not interchangeable: the pip wheel does not search the conda
environment, so `FindMKL` stopped finding the MKL libraries that are
installed there and PyTorch was configured with MKL and LAPACK off. 12
tests in `extension/llm/modules/test/test_turboquant_kv_cache.py` then
failed with:
```
RuntimeError: Calling torch.geqrf on a CPU tensor requires compiling PyTorch with LAPACK.
```
### Fix
Install the build requirements into a throwaway venv instead of into the
image, and invoke the build through that venv's interpreter by absolute
path. The venv is created with `--system-site-packages`, so PyTorch
still builds against the image's numpy and toolchain and still reuses
sccache. The venv is deleted once the wheel is built, so neither the
scikit-build-core plugin nor the pip cmake survives into the image, and
the image's own cmake stays in front.
### Evidence
From the docker image build log, PyTorch's configure summary:
| | pytorch#21562, as merged | this PR |
| --- | --- | --- |
| cmake actually used | `.../site-packages/cmake/data/bin/cmake` |
`/opt/conda/envs/py_3.10/bin/cmake` |
| `USE_MKL` | `OFF` | `ON` |
| `USE_LAPACK` | `0` | `1` |
CI on this PR:
- `test-riscv`: 17 of 17 green. On main it is 17 of 17 red.
- `test-lora-multimethod-linux`: green. On main it is red.
### Note on the jobs that did not finish
`unittest / linux`, `unittest-editable / linux` and `test-lora-linux`
were all cancelled at 1 hour 31 minutes, which is the 90 minute job cap.
Any PR that touches `.ci/docker/` forces a full image rebuild that
consumes roughly 30 minutes of that budget, so these three cannot
complete on a PR of this shape. That is also why the LAPACK problem was
not visible on pytorch#21562 before it merged. The `USE_LAPACK` line in the
image build log above is the direct evidence for that half of the fix,
since it is read at configure time rather than at test time.
### Relationship to the revert
This branch contains two commits, a revert of pytorch#21562 followed by the
corrected version. The net diff against main is a single file,
`.ci/docker/common/install_pytorch.sh`, and squash merge collapses them
into one commit.
This supersedes pytorch#21684, which is a straight revert of pytorch#21562. If this
lands, pytorch#21684 should be closed without merging. Do not merge pytorch#21684
after this one, since it would restore the previous file wholesale and
undo this fix.1 parent 48741ac commit 730b77a
1 file changed
Lines changed: 19 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
53 | 53 | | |
54 | 54 | | |
55 | 55 | | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
64 | 75 | | |
65 | 76 | | |
66 | 77 | | |
| |||
0 commit comments