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

vllm: init at v0.3.1 with rocm support #283842

Merged
merged 2 commits into from
Feb 18, 2024

Conversation

CertainLach
Copy link
Member

@CertainLach CertainLach commented Jan 25, 2024

Description of changes

Depends on:

I am not very familiar with python packaging in nixos (Though, anything is better than requirements.txt)

Couple of changes were made to fix build due to conflicting dependencies:

  • It now depends on torch instead of torch-bin, because plain torch dependency is propagated somewhere, together with duplicate openai-triton package
  • xformers and openai-triton were also broken for me due to usage of different torch

I think the only working way here is to use nixpkgs.config.torchSupport/nixpkgs.config.cudaSupport, as overriding torch dependency doesn't work very well in this case.

It also builds with CUDA for me, though I haven't properly tested it in runtime

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@mweinelt
Copy link
Member

Flask/Werkzeug can't go into master and needs a lot more work on reverse dependencies.

@mweinelt mweinelt marked this pull request as draft January 25, 2024 23:34
@happysalada
Copy link
Contributor

vllm as is still needed some work and wasn't working properly with cuda. If you've got it working for rocm, we could merge an early version with that only. (if you ever want to submit a PR for vllm, you don't need to credit me at all, you can just copy paste what you feel is useful and put me as a maintainer).
transformers is already at 4.37.1 on master.
there are some elements that could be merged relatively quickly, but I feel it would best be to break it down into smaller PRs that are easier to test and review.

@CertainLach
Copy link
Member Author

Managed to get it working on Cuda with this PR:
#285495

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 17, 2024
@CertainLach CertainLach changed the title vllm: init at 2024-01-25 with rocm support vllm: init at v0.3.1 with rocm support Feb 17, 2024
@CertainLach CertainLach changed the base branch from staging to master February 17, 2024 13:03
@github-actions github-actions bot removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 17, 2024
@CertainLach
Copy link
Member Author

gpuTargets
else
# vllm supports less gpu targets than rocm clr, supported target list is taken from ROCM_SUPPORTED_ARCHS in setup.py
lib.lists.intersectLists rocmPackages.clr.gpuTargets ["gfx90a" "gfx908" "gfx906" "gfx1030" "gfx1100"]
Copy link
Member Author

@CertainLach CertainLach Feb 17, 2024

Choose a reason for hiding this comment

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

Looks like vllm setup.py calculates the intersection by itself now, this needs to be removed.

@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Feb 17, 2024
@CertainLach
Copy link
Member Author

CertainLach commented Feb 17, 2024

Tested with CUDA, it works.
ROCm testing (on top of patches mentioned earlier, + couple of fixups on top of them) in progress.

Edit: retested with cuda, i was testing on base branch without #285249, I have patched dependencies for torch 2.2.0.

@CertainLach CertainLach marked this pull request as ready for review February 17, 2024 16:56
@CertainLach
Copy link
Member Author

Works on ROCm.

@happysalada happysalada merged commit 53312e4 into NixOS:master Feb 18, 2024
23 checks passed
@happysalada
Copy link
Contributor

@CertainLach thank you very much for this!

@CertainLach CertainLach deleted the vllm-init-rocm branch February 18, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants