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

[CI/Build] PEP 517/518 improvements #4791

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dtrifiro
Copy link

@dtrifiro dtrifiro commented May 13, 2024

This PR attempts to improve a few aspects of the vLLM build process with the aim of moving towards a full PEP-517/PEP-518 compliant build.

  1. Use pypa/build to build the vllm wheel, moving away from setup.py bdist_wheel
  2. Add --no-build-isolation flags when running pip install .
  3. bump setuptools minimum version to 61

Note that both when building through python -m build and when installing through pip install ., build dependencies are installed before the package is actually built, thus build isolation (which implies the creation of a new virtualenv with build dependencies) is not required. This should speed up the build process a bit.

Since we can define build dependencies in pyproject.toml, in the future we could get rid of --no-isolation and avoid having to manually manage build dependencies in requirements-build.txt (although that does help a bit with caching in Dockerfiles).

If this is merged, I can spend some time getting rid of the requirements*.txt files, moving those into setup.py (and/or possibly package extras), in order to be able to build/install with a single command. I would appreciate some thoughts on the matter.

this avoids re-creating an isolated virtualenv with build dependencies
when pip installing the package
@dtrifiro dtrifiro changed the title [CI/Build] PEP518 improvements [CI/Build] PEP 517/518 improvements May 13, 2024
@robertgshaw2-neuralmagic
Copy link
Collaborator

@andy-neuma PTAL

@mgoin
Copy link
Collaborator

mgoin commented May 21, 2024

Is the --no-isolation flag required for pip install -e . to generally work with this PR? I would like to resolve this before landing since I consider that a breaking change for the current flow to build from source.

@dtrifiro
Copy link
Author

Is the --no-isolation flag required for pip install -e . to generally work with this PR?

pip install -e will work regardless of --no-isolation. The issue with building without --no-isolation is that you're always creating a fresh virtual environment with the build dependencies (torch in particular, which is 700MB), which increases the build/install times:

$ pip list | grep torch
torch                             2.3.0
$ pip install -v -e .
Using pip 23.0.1 from /mnt/data/vllm/.venv/lib/python3.11/site-packages/pip (python 3.11)
Obtaining file:///mnt/data/vllm
  Running command pip subprocess to install build dependencies
  Collecting cmake>=3.21
    Using cached cmake-3.29.3-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (26.7 MB)
  Collecting ninja
    Using cached ninja-1.11.1.1-py2.py3-none-manylinux1_x86_64.manylinux_2_5_x86_64.whl (307 kB)
  Collecting packaging
    Using cached packaging-24.0-py3-none-any.whl (53 kB)
  Collecting setuptools>=49.4.0
    Using cached setuptools-70.0.0-py3-none-any.whl (863 kB)
  Collecting torch==2.3.0
    Using cached torch-2.3.0-cp311-cp311-manylinux1_x86_64.whl (779.2 MB)
    ...

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.

None yet

3 participants