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

ROCm in WSL #346

Merged
merged 8 commits into from
Nov 21, 2024
Merged

ROCm in WSL #346

merged 8 commits into from
Nov 21, 2024

Conversation

HPPinata
Copy link
Contributor

This is a first draft. Feedback on the clarity of descriptions, level of detail and questions left open is welcome.

The WSL-Patch needs a bit more testing to make sure it runs reliably.
flash_attn integration is also flaky as of now.

Memory usage, as usual on Linux systems is suboptimal, WSL memory overhead makes this even more obvious.

@HPPinata HPPinata force-pushed the ROCm-WSL branch 2 times, most recently from 9d5dd3a to 28bc634 Compare November 18, 2024 17:32
@HPPinata HPPinata marked this pull request as ready for review November 18, 2024 17:34
@HPPinata
Copy link
Contributor Author

HPPinata commented Nov 18, 2024

Consider merging #347 before this, since this PR has been tested on top of it.

@HPPinata HPPinata force-pushed the ROCm-WSL branch 3 times, most recently from 13a8649 to 8898789 Compare November 20, 2024 22:24
Copy link
Member

@tazlin tazlin left a comment

Choose a reason for hiding this comment

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

I'm OK with this as is; the functional change to the scripts should only affect WSL and the doc changes can be iterated on if needed. I can't step-by-step confirm that it works as reported, as I don't have access to an AMD machine, but it looks reasonable and I'd rather merge now so the git history doesn't keep diverging.

@tazlin tazlin merged commit b23b866 into Haidra-Org:main Nov 21, 2024
@HPPinata
Copy link
Contributor Author

HPPinata commented Nov 21, 2024

I've tested a blind copy/paste of the commands from the wsl unregister stage to getting a worker up and running (with the only exceptions being a copy/paste of the bridgeData.yaml and models directory to avoid redownload) so it should work (Tested on Windows 11, with the 10.2024 drivers and a supported RDNA3 Card).
Any compatibility issues beyond that will need to be reported by users, I think that should be fine for something explicitly stated to be experimental.

Initially activating WSL is hard to test if it has been installed before, but if we get any questions around that it's just a Readme change to correct. The library patch should continue to work unless the "recommended" way to deal with this changes upstream or there are incompatible Driver/Kernel upgrades.

@tazlin
Copy link
Member

tazlin commented Nov 21, 2024

Agreed on all.

Thanks again for your work on this and all your other initiatives.

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