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: replace pip with uv by Astral.sh #952

Merged
merged 19 commits into from
Mar 15, 2024
Merged

CI: replace pip with uv by Astral.sh #952

merged 19 commits into from
Mar 15, 2024

Conversation

carmocca
Copy link
Contributor

@carmocca carmocca commented Feb 25, 2024

author: @Andrei-Aksionov

Hello there 👋

This PR replace pip with blazingly fast uv by Astral.sh.

Couple of notes:

  1. Decided to not use cache since uv is so fast, that the process of retrieving and unpacking stored cache slows down things a bit.
  2. UV_HTTP_TIMEOUT is increased to 500 seconds, since sometimes uv might hang during package download. But when it works, it downloads them really fast.
  3. As for right now it's not possible to do uv pip install ., so the workaround with installing packages in an editable mode (which is supported) is used here.

@carmocca carmocca self-assigned this Feb 25, 2024
@Andrei-Aksionov
Copy link
Collaborator

It's of course blazingly fast, but it doesn't support all the functionality of the pip.
For instance it cannot install requirements-all.txt:

(lit-gpt-uv) ➜  lit-gpt git:(autogptq_integration) uv pip install -r requirements-all.txt                                                                                                                                   INSERT
error: Unsupported requirement in requirements-all.txt at position 658
  Caused by: URL requirement must be preceded by a package name. Add the name of the package before the URL (e.g., `package_name @ https://...`).
git+https://github.com/EleutherAI/lm-evaluation-harness.git@115206dc89dad67b8beaa90051fb52db77f0a529
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I see that it's developing in a quite intensive manner, so I think we can revisit it in a month or so, but right now it's not a drop-in replacement for pip.

@Andrei-Aksionov
Copy link
Collaborator

It doesn't know list command, only freeze.

(lit-gpt-uv) ➜  lit-gpt git:(autogptq_integration) uv pip list                                                                                                                                                              INSERT
error: unrecognized subcommand 'list'

  tip: a similar subcommand exists: 'install'

Usage: uv pip [OPTIONS] <COMMAND>

For more information, try '--help'.

@Andrei-Aksionov
Copy link
Collaborator

Andrei-Aksionov commented Mar 5, 2024

On my local machine, with all the packages being cached, uv reduced installation time from 67 seconds to 17.
On a CI install all dependencies step is reduced from 45 seconds to 28 seconds. Is it enough to justify the change?
Update: it's not a fair comparison (on a CI) since pip uses cache and uv doesn't. That needs to be fixed. actions/setup-python#818

But it throws some unexpected errors.
I'll revisit it when I have time. In the meantime, let's keep it as a Draft.

@carmocca carmocca assigned Andrei-Aksionov and unassigned carmocca Mar 12, 2024
@Andrei-Aksionov
Copy link
Collaborator

Not yet ready for integration, but close.

The biggest issues:

  • doesn't support uv pip install ., but you can install the package in edit mode by doing: uv pip install -e .. Not a hugely criminal workaround, yet not what I would like to do in a CI.
  • sometimes might hang during package download. But when it works, it does it really fast.

I'll try it one more time in a week.

@carmocca
Copy link
Contributor Author

Honestly I would be fine merging it now that it's working at least

@Andrei-Aksionov Andrei-Aksionov changed the title Try Astral uv for CI CI: replace pip with uv by Astral.sh Mar 15, 2024
@Andrei-Aksionov Andrei-Aksionov marked this pull request as ready for review March 15, 2024 15:31
@carmocca carmocca merged commit fd4b8a2 into main Mar 15, 2024
8 checks passed
@carmocca carmocca deleted the carmocca/uv branch March 15, 2024 15:41
rasbt pushed a commit that referenced this pull request Mar 18, 2024
@charliermarsh
Copy link

By the way, we now support uv pip install . without -e, in the latest release.

@carmocca
Copy link
Contributor Author

carmocca commented Mar 25, 2024

Thanks for the heads up Charlie. I just gave it a try in #1187 but non-editable requires --prerelease=allow since we have a prerelease direct dependency.

But adding this makes dependencies of our dependencies also install prereleases. This in turn installs the current numpy prerelease (2.0.0b1) which itself doesn't work with the latest torch:

A module that was compiled using NumPy 1.x cannot be run in
NumPy 2.0.0b1 as it may crash. To support both 1.x and 2.x
versions of NumPy, modules must be compiled against NumPy 2.0.
If you are a user of the module, the easiest solution will be to
either downgrade NumPy or update the failing module (if available).
Traceback (most recent call last):  File "<string>", line 1, in <module>
  File "/home/runner/work/litgpt/litgpt/litgpt/__init__.py", line 6, in <module>
    from litgpt.model import GPT  # needs to be imported before config
  File "/home/runner/work/litgpt/litgpt/litgpt/model.py", line 12, in <module>
    import torch
  File "/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/site-packages/torch/__init__.py", line 1477, in <module>
    from .functional import *  # noqa: F403
  File "/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/site-packages/torch/functional.py", line 9, in <module>
    import torch.nn.functional as F
  File "/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/site-packages/torch/nn/__init__.py", line 1, in <module>
    from .modules import *  # noqa: F403
  File "/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/site-packages/torch/nn/modules/__init__.py", line 35, in <module>
    from .transformer import TransformerEncoder, TransformerDecoder, \
  File "/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/site-packages/torch/nn/modules/transformer.py", line 20, in <module>
    device: torch.device = torch.device(torch._C._get_default_device()),  # torch.device('cpu'),
/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/site-packages/torch/nn/modules/transformer.py:20: UserWarning: Failed to initialize NumPy: module compiled against ABI version 0x1000009 but this version of numpy is 0x2000000 (Triggered internally at ../torch/csrc/utils/tensor_numpy.cpp:84.)

Is there a way to set --prerelease= only for our direct dependencies? Maybe --prerelease=allow-direct?

@charliermarsh
Copy link

Ah yeah, we do allow prereleases for direct dependencies, but we treat . and -e . a little differently... For editables, we treat any dependency of an editable as "direct"; but for direct reference dependencies (like . or https://...), we consider that the direct dependency, and any of its dependencies are transitive. I think we can improve this: astral-sh/uv#2643

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.

3 participants