Skip to content

[Feature] Updating dgl ROCm support#20

Merged
jamesETsmith merged 11 commits intoROCm:developfrom
gcapodagAMD:develop
Mar 24, 2026
Merged

[Feature] Updating dgl ROCm support#20
jamesETsmith merged 11 commits intoROCm:developfrom
gcapodagAMD:develop

Conversation

@gcapodagAMD
Copy link
Copy Markdown

@gcapodagAMD gcapodagAMD commented Mar 17, 2026

Description

Updating the support for ROCm

I have been in touch with @diptorupd who is aware of this PR. This comes from having to include updates to build dgl from source with ROCm support, to enable users at a hackathon in the UK.

Checklist

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
  • I've leverage the tools to beautify the python and c++ code.
  • The PR is complete and small, read the Google eng practice (CL equals to PR) to understand more about small PR. In DGL, we consider PRs with less than 200 lines of core code change are small (example, test and documentation could be exempted).
  • All changes have test coverage
  • Code is well-documented
  • To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
  • Related issue is referred in this PR
  • If the PR is for a new model/paper, I've updated the example index here.

Changes

@gcapodagAMD gcapodagAMD changed the title [$Feature] Updating dgl ROCm support [Feature] Updating dgl ROCm support Mar 17, 2026
@diptorupd diptorupd requested a review from jamesETsmith March 17, 2026 17:58
@diptorupd
Copy link
Copy Markdown

Adding @jamesETsmith as a reviewer

@jamesETsmith
Copy link
Copy Markdown
Collaborator

Thanks for this @gcapodagAMD! There's some overlap between this and #16. I'm just getting back from leave and I'm trying clean up #16 now. As soon as that's landed, I'll take a look at this.

In the meantime, could you tell us how you were building from source? What versions of rocm, python, and pytorch you were using?

@gcapodagAMD
Copy link
Copy Markdown
Author

of course @jamesETsmith it was ROCm 7.2.0, Python3.10 on Ubuntu 22.04 and PyTorch 2.9.1.
I think we should also remove the path here: https://github.com/gcapodagAMD/dgl/blob/c61cc21bc30a983485ab343c8e4f5450b451d0a6/tests/scripts/task_unit_test_rocm.sh#L22 to allow users to specify where they have built the dgl libs

sorry, it looks like my latest commit still did not satisfy the linting requirements

@gcapodagAMD
Copy link
Copy Markdown
Author

ok linting passing now

@gcapodagAMD
Copy link
Copy Markdown
Author

gcapodagAMD commented Mar 18, 2026

@jamesETsmith I compared what is here with what is in the PR you mentioned and it looks like the files in graphbolt/src/cuda/extension have been modified exactly in the same way. Cool :)

@gcapodagAMD
Copy link
Copy Markdown
Author

also this one graphbolt/src/cuda/cooperative_minibatching_utils.cu

Comment thread python/dgl/_ffi/_cython/function.pxi
Comment thread include/dgl/hip/hip_extensions/amd_warp_primitives.h
Comment thread graphbolt/src/cnumpy.h Outdated
Comment thread CMakeLists.txt Outdated
Comment thread graphbolt/CMakeLists.txt Outdated
Comment thread README.md
Comment thread README.md Outdated
Copy link
Copy Markdown
Author

@gcapodagAMD gcapodagAMD left a comment

Choose a reason for hiding this comment

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

let me know if you are happy with the changes or would like to request more, thanks

Comment thread include/dgl/hip/hip_extensions/amd_warp_primitives.h
Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
Comment thread README.md Outdated
Comment thread include/dgl/hip/hip_extensions/amd_warp_primitives.h
Copy link
Copy Markdown
Author

@gcapodagAMD gcapodagAMD left a comment

Choose a reason for hiding this comment

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

I think I will be addressing all the left over comments with the next commit, thanks @jamesETsmith and @GMNGeoffrey

Copy link
Copy Markdown
Author

@gcapodagAMD gcapodagAMD left a comment

Choose a reason for hiding this comment

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

should be all good now

Copy link
Copy Markdown
Collaborator

@jamesETsmith jamesETsmith left a comment

Choose a reason for hiding this comment

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

Just a few minor comments left

Comment thread script/install_graphbolt_deps.sh Outdated
Comment thread script/install_graphbolt_deps.sh
Comment thread include/dgl/hip/hip_extensions/amd_warp_primitives.h Outdated
@jamesETsmith jamesETsmith merged commit 9a3bfc1 into ROCm:develop Mar 24, 2026
1 check 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.

4 participants