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

fix(vllm): fix up vllm 0.4.2 #1635

Merged
merged 14 commits into from
May 9, 2024
Merged

fix(vllm): fix up vllm 0.4.2 #1635

merged 14 commits into from
May 9, 2024

Conversation

Atry
Copy link
Contributor

@Atry Atry commented May 6, 2024

Contribution checklist (recommended but not always applicable/required):

  • There's an automated test for this change
  • Commit messages or code include references to related issues or PRs (including third parties)
  • Commit messages are conventional - examples from the log include "feat: add changelog files to fixup hook", "fix(contourpy): allow wheel usage", and "test: add sqlalchemy2 test"

@Atry Atry force-pushed the vllm-0.4.1 branch 2 times, most recently from 5feb137 to 9de3bbe Compare May 6, 2024 08:44
@Atry Atry changed the title Upgrade vllm version in tests fix(vllm): Fix up vllm 0.4.2 May 6, 2024
@Atry Atry marked this pull request as ready for review May 6, 2024 08:44
@Atry Atry changed the title fix(vllm): Fix up vllm 0.4.2 fix(vllm): fix up vllm 0.4.2 May 6, 2024
@Atry
Copy link
Contributor Author

Atry commented May 6, 2024

NixOS/nixpkgs#309477 is needed to investigate the circular dependencies between fastapi and fastapi-cli.

@cpcloud
Copy link
Collaborator

cpcloud commented May 6, 2024

@Atry I added an override for vllm-nccl-cu12 that downloads the rather large SO file using pkgs.fetchurl and allows building vllm without the user having to hack in an environment variable, or download things from the internet in a random setup.py script 😅

@Atry
Copy link
Contributor Author

Atry commented May 6, 2024

Copying the so file to a path under tmpdir does not make sense. It would be discarded immediately. Users still need to set the environment variable even when vllm-nccl-cu12 is overrridden.

overrides/default.nix Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Atry Atry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

triton needs setuptools

overrides/default.nix Outdated Show resolved Hide resolved
overrides/default.nix Outdated Show resolved Hide resolved
overrides/default.nix Outdated Show resolved Hide resolved
overrides/default.nix Outdated Show resolved Hide resolved
overrides/default.nix Outdated Show resolved Hide resolved
@Atry Atry requested a review from cpcloud May 9, 2024 06:01
@Atry
Copy link
Contributor Author

Atry commented May 9, 2024

@cpcloud Do you want to merge this PR?

@cpcloud cpcloud merged commit 6aa2f73 into nix-community:master May 9, 2024
160 checks passed
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

2 participants