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: ROCm build #817

Merged
merged 12 commits into from
Dec 5, 2024
Merged

fix: ROCm build #817

merged 12 commits into from
Dec 5, 2024

Conversation

Naomiusearch
Copy link
Contributor

I have no idea what to put here so...

Pozdrawiam z rodzinką :3

Copy link
Member

@AlpinDale AlpinDale left a comment

Choose a reason for hiding this comment

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

LGTM, just a few nitpicks

sudo patch $ROCM_PATH/lib/llvm/lib/clang/*/include/__clang_hip_cmath.h ./patches/amd.patch
Copy link
Member

Choose a reason for hiding this comment

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

What happens if there's multiple versions installed here? Will it return the first one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there shouldn't be more than one version, though I can just make it check rocm version and patch clang 17 for rocm 6.1 and clang 18 for 6.2 if that's better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nvm, I'm too stupid for that. It should be fine as is anyway

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@Naomiusearch
Copy link
Contributor Author

@AlpinDale can it be merged?

@AlpinDale
Copy link
Member

@Naomiusearch sorry for the delay. I forgot to mention: I tested this PR a bit more and it seems to break AWQ/GPTQ marlin entirely. I planned to redo this PR but I got distracted. I'll finish it up today.

@AlpinDale
Copy link
Member

I tested this some more and AWQ needs more work to be fixed. Can you undo the changes related to gptq_marlin? We can merge the PR then.

@AlpinDale AlpinDale changed the title Some fixes fix: ROCm build Nov 27, 2024
@AlpinDale AlpinDale merged commit 4f9fea4 into PygmalionAI:main Dec 5, 2024
5 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.

2 participants