feature[cartesian]: Add support for HIP/ROCm #1278

stubbiali · 2023-06-28T12:16:53Z

Adaptations to generate GridTools/DaCe code targeting AMD GPUs. Complemented by GridTools/gridtools#1759.

egparedes

Not a review, just a couple of suggestions

egparedes · 2023-06-29T08:45:09Z

src/gt4py/storage/utils.py

@@ -243,6 +245,18 @@ def allocate_gpu(
    if device_field.ndim > 0:
        device_field = device_field[tuple(slice(0, s, None) for s in shape)]

+    if gt_config.GT4PY_USE_HIP:
+        # HIP/ROCm lack support for __cuda_array_interface__
+        device_field.__hip_array_interface__ = {


Shouldn't this be called __cuda_array_interface__ to avoid any changes in the gridtools bindings?

I agree, unless things have changed, it seems that the community just always uses the cuda_array_interface name even for AMD device buffers. (e.g. most importantly cupy)

I fully agree that __cuda_array_interface__ should be the way to go, but that's not feasible at the moment. CuPy does not allow to either read or overload __cuda_array_interface__. See this issue and links therein: cupy/cupy#7431. This said, my solution is clearly a hack. DLPack seems the solution the community is converging to, but I must said I'm not very familiar with it, so I need to dig into it first.

My understanding of cupy/cupy#7431 is that this is some particular case where you would want to wrap cpp side allocated data in a cupy buffer. Have you tried and ran into the error? Last time we ran AMD a while back we used __cuda_array_interface__ happily to pass cupy arrays to stencils.

Yes, I tried and the GridTools bindings threw the exception AttributeError: HIP/ROCm does not support cuda array interface. Same happens if you try to read __cuda_array_interface__ from within Python. But I cannot exclude I'm missing something important.

+1 for DLPack.
I've seen accelerated adoption lately. Is it on the radar @egparedes to add it ?
Feels not urgent, but it's something we could contribute later down the road

If this error is there, is it safe to just work around it in this way? There must be a reason why it is not supported.

Again, I'm not an expert here, but it's very likely that this workaround only works in simple scenarios, as ours (e.g. using a single stream).

+1 for DLPack. I've seen accelerated adoption lately. Is it on the radar @egparedes to add it ? Feels not urgent, but it's something we could contribute later down the road

@FlorianDeconinck Yes, we're looking into supporting DLPack. There is no timeline yet, but hopefully not far in the future.

egparedes · 2023-06-29T09:01:19Z

src/gt4py/cartesian/config.py

@@ -32,6 +32,8 @@

 CUDA_HOST_CXX: Optional[str] = os.environ.get("CUDA_HOST_CXX", None)

+GT4PY_USE_HIP: int = int(os.environ.get("GT4PY_USE_HIP", 0))


There should be a way to autodetect if cupy has been compiled with HIP. This is a naive suggestion to do it using dlpack, but probably there is a simpler way using some esoteric cupy API endpoint (maybe cp.show_config()?).

Suggested change

GT4PY_USE_HIP: int = int(os.environ.get("GT4PY_USE_HIP", 0))

if "GT4PY_USE_HIP" in os.environ:

GT4PY_USE_HIP: bool = bool(int(os.environ["GT4PY_USE_HIP"]))

else:

# Autodetect cupy with ROCm/HIP using DLPack device type

# Reference: https://data-apis.org/array-api/latest/API_specification/generated/array_api.array.__dlpack_device__.html#array_api.array.__dlpack_device__

try:

import cupy as _cp

GT4PY_USE_HIP = _cp.empty((1,)).__dlpack_device__()[0] == 10

del _cp

except Exception:

GT4PY_USE_HIP = False

I partially followed your suggestion using cupy.cuda.get_hipcc_path().

gronerl

Only a small comment.
I wonder if there is a way at CSCS to run something on AMD as part of CI.

src/gt4py/cartesian/backend/pyext_builder.py

FlorianDeconinck · 2023-06-30T12:55:59Z

If this needs an extra set of tests NASA/NOAA can wiggle some AMD systems, hit me up.

(Great to see this!)

Light-weight PR to make the Python bindings work with AMD GPU buffers. Complementary to GridTools/gt4py#1278. --------- Co-authored-by: Hannes Vogt <hannes@havogt.de>

havogt · 2023-07-18T15:06:02Z

cscs-ci run

havogt

nvcc needs -isystem=

src/gt4py/cartesian/backend/pyext_builder.py

havogt · 2023-07-19T11:32:25Z

cscs-ci run

stubbiali · 2023-07-25T14:21:18Z

src/gt4py/cartesian/backend/pyext_builder.py

+            "-isystem{}".format(gt_include_path),
+            "-isystem{}".format(gt_config.build_settings["boost_include_path"]),


hipcc (clang) requires -isystem [dir].

havogt · 2023-07-26T13:47:53Z

cscs-ci run

Light-weight PR to make the Python bindings work with AMD GPU buffers. Complementary to GridTools/gt4py#1278. --------- Co-authored-by: Hannes Vogt <hannes@havogt.de>

stubbiali added 3 commits June 23, 2023 00:10

Initial adaptations to support HIP/ROCm.

b447bf4

Add comment.

26ae5cf

Rename nvcc as cuda.

5892d8f

stubbiali mentioned this pull request Jun 28, 2023

Python: Add support for HIP/ROCm buffers GridTools/gridtools#1759

Merged

stubbiali added 2 commits June 28, 2023 14:28

run isort hook.

e9e214a

Revise extra_compile_args.

36935cc

egparedes reviewed Jun 29, 2023

View reviewed changes

gronerl requested changes Jun 30, 2023

View reviewed changes

src/gt4py/cartesian/backend/pyext_builder.py Outdated Show resolved Hide resolved

Fuse branches.

ea392ee

Set GT4PY_USE_HIP programmatically.

17368a7

gronerl approved these changes Jul 4, 2023

View reviewed changes

havogt requested changes Jul 19, 2023

View reviewed changes

src/gt4py/cartesian/backend/pyext_builder.py Outdated Show resolved Hide resolved

src/gt4py/cartesian/backend/pyext_builder.py Outdated Show resolved Hide resolved

Apply suggestions from code review

d8a148c

Fix compiler flags.

59c95e4

stubbiali commented Jul 25, 2023

View reviewed changes

havogt approved these changes Aug 3, 2023

View reviewed changes

havogt merged commit e7e403f into GridTools:main Aug 3, 2023
26 checks passed

stubbiali mentioned this pull request Oct 24, 2023

Refactor Python bindings ghex-org/GHEX#151

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature[cartesian]: Add support for HIP/ROCm #1278

feature[cartesian]: Add support for HIP/ROCm #1278

stubbiali commented Jun 28, 2023 •

edited

Loading

egparedes left a comment

egparedes Jun 29, 2023 •

edited

Loading

gronerl Jun 30, 2023

stubbiali Jun 30, 2023 •

edited

Loading

gronerl Jun 30, 2023

stubbiali Jun 30, 2023 •

edited

Loading

FlorianDeconinck Jun 30, 2023

gronerl Jun 30, 2023

stubbiali Jun 30, 2023

egparedes Jul 4, 2023 •

edited

Loading

egparedes Jun 29, 2023 •

edited

Loading

stubbiali Jun 30, 2023

gronerl left a comment

FlorianDeconinck commented Jun 30, 2023

havogt commented Jul 18, 2023

havogt left a comment

havogt commented Jul 19, 2023

stubbiali Jul 25, 2023

havogt commented Jul 26, 2023

		@@ -32,6 +32,8 @@

		CUDA_HOST_CXX: Optional[str] = os.environ.get("CUDA_HOST_CXX", None)

		GT4PY_USE_HIP: int = int(os.environ.get("GT4PY_USE_HIP", 0))

-GT4PY_USE_HIP: int = int(os.environ.get("GT4PY_USE_HIP", 0))
+if "GT4PY_USE_HIP" in os.environ:
+    GT4PY_USE_HIP: bool = bool(int(os.environ["GT4PY_USE_HIP"]))
+else:
+    # Autodetect cupy with ROCm/HIP using DLPack device type
+    # Reference: https://data-apis.org/array-api/latest/API_specification/generated/array_api.array.__dlpack_device__.html#array_api.array.__dlpack_device__
+    try:
+        import cupy as _cp
+        GT4PY_USE_HIP = _cp.empty((1,)).__dlpack_device__()[0] == 10
+        del _cp
+    except Exception:
+        GT4PY_USE_HIP = False

		"-isystem{}".format(gt_include_path),
		"-isystem{}".format(gt_config.build_settings["boost_include_path"]),

feature[cartesian]: Add support for HIP/ROCm #1278

feature[cartesian]: Add support for HIP/ROCm #1278

Conversation

stubbiali commented Jun 28, 2023 • edited Loading

egparedes left a comment

Choose a reason for hiding this comment

egparedes Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

gronerl Jun 30, 2023

Choose a reason for hiding this comment

stubbiali Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

gronerl Jun 30, 2023

Choose a reason for hiding this comment

stubbiali Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

FlorianDeconinck Jun 30, 2023

Choose a reason for hiding this comment

gronerl Jun 30, 2023

Choose a reason for hiding this comment

stubbiali Jun 30, 2023

Choose a reason for hiding this comment

egparedes Jul 4, 2023 • edited Loading

Choose a reason for hiding this comment

egparedes Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

stubbiali Jun 30, 2023

Choose a reason for hiding this comment

gronerl left a comment

Choose a reason for hiding this comment

FlorianDeconinck commented Jun 30, 2023

havogt commented Jul 18, 2023

havogt left a comment

Choose a reason for hiding this comment

havogt commented Jul 19, 2023

stubbiali Jul 25, 2023

Choose a reason for hiding this comment

havogt commented Jul 26, 2023

stubbiali commented Jun 28, 2023 •

edited

Loading

egparedes Jun 29, 2023 •

edited

Loading

stubbiali Jun 30, 2023 •

edited

Loading

stubbiali Jun 30, 2023 •

edited

Loading

egparedes Jul 4, 2023 •

edited

Loading

egparedes Jun 29, 2023 •

edited

Loading