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

Add rocm support #3

Merged
merged 4 commits into from
Apr 14, 2023
Merged

Add rocm support #3

merged 4 commits into from
Apr 14, 2023

Conversation

rprospero
Copy link
Contributor

This PR adds support for AMD cards. I've done my best not to touch any of the NVIDIA material, but I don't have an NVIDIA card to test it on. Obviously, that kind of test will need to be performed before merging.

Running under AMD requires a couple of extra changes to the webui-user.sh file (mainly disabling the CUDA check which will obviously fail). I'm working on a more substantial PR which will enable full app support (so that everything is a single nix run github:virchau13/automattic1111-webui-nix or nix run github:virchau13/automattic1111-webui-nix#rocm

@virchau13
Copy link
Owner

Yay, someone actually did it! Thank you for the PR! I'll test this and merge it once I get back to my (Nvidia) PC.

One preliminary comment: you should probably change shell.nix also so that it doesn't error with the new argument format of impl.nix.

Just a note on nix run support: one of my goals for this project was to avoid tracking upstream unless absolutely necessary. I'd prefer if any changes you make don't involve something that has to be changed with every push to upstream (such as a fetchFromGitHub call, a submodule in this repository, or a direct embed in this repository). Of course there would be times when the deps would break, and then I'd have to go and fix it, but the idea was that that would be pretty rare (and it turns out I was right, I haven't had to do it even once :P.) I would very much prefer a change that adds a new setupDiffusion shell function that patches the upstream repository, or something similar to that, so that I don't have to go and manually update anything unless e.g. the patch breaks.

Again, thank you for the PR!

@rprospero
Copy link
Contributor Author

Good catch on the shell.nix file. I've got it working again and even added an option for using nix-shell with AMD support (nix-shell --arg isCUDA false).

As for the nix run bits, I'm fully on-board with your goal of not tracking upstream. I'm setting it up so that you would always get the most recent upstream release, even if we never touch the repo again (barring major breaking changes from upstream).

@virchau13 virchau13 merged commit 91760ee into virchau13:master Apr 14, 2023
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.

2 participants