Skip to content

Conversation

@michel2323
Copy link
Collaborator

@michel2323 michel2323 commented Sep 4, 2025

This brings oneAPI on par with AMDGPU.

Two minor issues:

  • For AMDGPU and oneAPI we don't test an FFT solver. Actually, the FFT has not yet landed in oneAPI, but it should soon.
  • Having oneAPI, CUDA, and AMDGPU as dependencies in the tests potentially restricts the dependencies and does not represent realistic setups where users only load one GPU package. It would be better to add only one of the packages when testing, for example, CUDA, etc...

@glwagner
Copy link
Member

glwagner commented Sep 4, 2025

I think we can achieve that second goal. This requires moving using AMDGPU to test_amdgpu.jl right? Is anything else needed? Maybe we would also need a second initialization step for the AMD tests (since now using AMDGPU happens during initialization) --- not sure

@michel2323 michel2323 marked this pull request as draft September 4, 2025 15:46
@michel2323
Copy link
Collaborator Author

michel2323 commented Sep 4, 2025

What about CUDA.jl? Do you want to keep that package as a fixed dependency of the tests and only move AMDGPU and oneAPI out? Eventually, maybe every GPU backend should be treated equally. Not sure that's already the case though...

@glwagner
Copy link
Member

glwagner commented Sep 4, 2025

The main difference now I guess is that we have a lot of CUDA resources for CI but only enough AMD / Intel resources for super simple smoke tests.

But you're right, the idea I proposed will not work in a future where we run the full suite of GPU tests on all devices... 🤔

using InteractiveUtils
using oneAPI
using oneAPI.oneMKL: oneSparseMatrixCSR
using oneAPI.oneMKL.FFT
Copy link
Collaborator

Choose a reason for hiding this comment

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

@michel2323 Did you tested it with Oceananigans.jl?

Comment on lines +126 to +128
@inline UT.getdevice(one::GPUVar, i) = oneAPI.device(one)
@inline UT.getdevice(one::GPUVar) = oneAPI.device(one)
@inline UT.switch_device!(dev::oneAPI.ZeDevice) = oneAPI.device!(dev)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@inline UT.getdevice(one::GPUVar, i) = oneAPI.device(one)
@inline UT.getdevice(one::GPUVar) = oneAPI.device(one)
@inline UT.switch_device!(dev::oneAPI.ZeDevice) = oneAPI.device!(dev)

after #4738 this is not needed

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.

5 participants