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

Missing adapt for sparse and CUDABackend #2459

Closed
hexaeder opened this issue Aug 5, 2024 · 6 comments
Closed

Missing adapt for sparse and CUDABackend #2459

hexaeder opened this issue Aug 5, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@hexaeder
Copy link

hexaeder commented Aug 5, 2024

Describe the bug

There is a difference between adapting a sparse matrix to CuArray vs CUDABackend. I would expect both to return a CuSparseMatrixCSC.

To reproduce

using Pkg
pkg"activate --temp"
pkg"add SparseArrays, CUDA, Adapt"

using SparseArrays, CUDA, Adapt, CUDA.CUSPARSE

A = sprand(Float32, 10, 10, 0.1)
adapt(CUDABackend(), A) isa SparseMatrixCSC # true
adapt(CuArray, A) isa CuSparseMatrixCSC     #true
Manifest.toml
(jl_5A56jY) pkg> st -m
Status `/tmp/jl_5A56jY/Manifest.toml`
  [621f4979] AbstractFFTs v1.5.0
  [79e6a3ab] Adapt v4.0.4
  [a9b6321e] Atomix v0.1.0
  [ab4f0b2a] BFloat16s v0.5.0
  [fa961155] CEnum v0.5.0
  [052768ef] CUDA v5.4.3
  [1af6417a] CUDA_Runtime_Discovery v0.3.4
  [3da002f7] ColorTypes v0.11.5
  [5ae59095] Colors v0.12.11
  [34da2185] Compat v4.15.0
  [a8cc5b0e] Crayons v4.1.1
  [9a962f9c] DataAPI v1.16.0
  [a93c6f00] DataFrames v1.6.1
  [864edb3b] DataStructures v0.18.20
  [e2d170a0] DataValueInterfaces v1.0.0
  [e2ba6199] ExprTools v0.1.10
  [53c48c17] FixedPointNumbers v0.8.5
  [0c68f7d7] GPUArrays v10.3.0
  [46192b85] GPUArraysCore v0.1.6
  [61eb1bfa] GPUCompiler v0.26.7
  [842dd82b] InlineStrings v1.4.2
  [41ab1584] InvertedIndices v1.3.0
  [82899510] IteratorInterfaceExtensions v1.0.0
  [692b3bcd] JLLWrappers v1.5.0
  [63c18a36] KernelAbstractions v0.9.22
  [929cbde3] LLVM v8.1.0
  [8b046642] LLVMLoopInfo v1.0.0
  [b964fa9f] LaTeXStrings v1.3.1
  [1914dd2f] MacroTools v0.5.13
  [e1d29d7a] Missings v1.2.0
  [5da4648a] NVTX v0.3.4
  [bac558e1] OrderedCollections v1.6.3
  [2dfb63ee] PooledArrays v1.4.3
  [aea7be01] PrecompileTools v1.2.1
  [21216c6a] Preferences v1.4.3
  [08abe8d2] PrettyTables v2.3.2
  [74087812] Random123 v1.7.0
  [e6cf234a] RandomNumbers v1.5.3
  [189a3867] Reexport v1.2.2
  [ae029012] Requires v1.3.0
  [6c6a2e73] Scratch v1.2.1
  [91c51154] SentinelArrays v1.4.5
  [a2af1166] SortingAlgorithms v1.2.1
  [90137ffa] StaticArrays v1.9.7
  [1e83bf80] StaticArraysCore v1.4.3
  [10745b16] Statistics v1.11.1
  [892a3eda] StringManipulation v0.3.4
  [3783bdb8] TableTraits v1.0.1
  [bd369af6] Tables v1.12.0
  [a759f4b9] TimerOutputs v0.5.24
  [013be700] UnsafeAtomics v0.2.1
  [d80eeb9a] UnsafeAtomicsLLVM v0.1.5
  [4ee394cb] CUDA_Driver_jll v0.9.1+1
  [76a88914] CUDA_Runtime_jll v0.14.1+0
  [9c1d0b0a] JuliaNVTXCallbacks_jll v0.2.1+0
  [dad2f222] LLVMExtra_jll v0.0.31+0
  [e98f9f5b] NVTX_jll v3.1.0+2
  [0dad84c5] ArgTools v1.1.2
  [56f22d72] Artifacts v1.11.0
  [2a0f44e3] Base64 v1.11.0
  [ade2ca70] Dates v1.11.0
  [f43a241f] Downloads v1.6.0
  [7b1f6079] FileWatching v1.11.0
  [9fa8497b] Future v1.11.0
  [b77e0a4c] InteractiveUtils v1.11.0
  [4af54fe1] LazyArtifacts v1.11.0
  [b27032c2] LibCURL v0.6.4
  [76f85450] LibGit2 v1.11.0
  [8f399da3] Libdl v1.11.0
  [37e2e46d] LinearAlgebra v1.11.0
  [56ddb016] Logging v1.11.0
  [d6f4376e] Markdown v1.11.0
  [ca575930] NetworkOptions v1.2.0
  [44cfe95a] Pkg v1.11.0
  [de0858da] Printf v1.11.0
  [3fa0cd96] REPL v1.11.0
  [9a3f8284] Random v1.11.0
  [ea8e919c] SHA v0.7.0
  [9e88b42a] Serialization v1.11.0
  [6462fe0b] Sockets v1.11.0
  [2f01184e] SparseArrays v1.11.0
  [f489334b] StyledStrings v1.11.0
  [fa267f1f] TOML v1.0.3
  [a4e569a6] Tar v1.10.0
  [8dfed614] Test v1.11.0
  [cf7118a7] UUIDs v1.11.0
  [4ec0a83e] Unicode v1.11.0
  [e66e0078] CompilerSupportLibraries_jll v1.1.1+0
  [deac9b47] LibCURL_jll v8.6.0+0
  [e37daf67] LibGit2_jll v1.7.2+0
  [29816b5a] LibSSH2_jll v1.11.0+1
  [c8ffd9c3] MbedTLS_jll v2.28.6+0
  [14a3606d] MozillaCACerts_jll v2023.12.12
  [4536629a] OpenBLAS_jll v0.3.27+1
  [bea87d4a] SuiteSparse_jll v7.7.0+0
  [83775a58] Zlib_jll v1.2.13+1
  [8e850b90] libblastrampoline_jll v5.10.1+0
  [8e850ede] nghttp2_jll v1.59.0+0
  [3f19e933] p7zip_jll v17.4.0+2

Version info

Details on Julia:

# please post the output of:
julia> versioninfo()
Julia Version 1.11.0-rc2
Commit 34c3a63147b (2024-07-29 06:24 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 16 × AMD Ryzen 9 8945HS w/ Radeon 780M Graphics
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, znver3)
Threads: 16 default, 0 interactive, 8 GC (on 16 virtual cores)
Environment:
  JULIA_NUM_THREADS = auto

Details on CUDA:

julia> CUDA.versioninfo()
CUDA runtime 12.5, artifact installation
CUDA driver 12.5
NVIDIA driver 555.99.0

CUDA libraries:
- CUBLAS: 12.5.3
- CURAND: 10.3.6
- CUFFT: 11.2.3
- CUSOLVER: 11.6.3
- CUSPARSE: 12.5.1
- CUPTI: 2024.2.1 (API 23.0.0)
- NVML: 12.0.0+555.52.1

Julia packages:
- CUDA: 5.4.3
- CUDA_Driver_jll: 0.9.1+1
- CUDA_Runtime_jll: 0.14.1+0

Toolchain:
- Julia: 1.11.0-rc2
- LLVM: 16.0.6

1 device:
  0: ������������������������������������������������������������ (sm_89, 7.444 GiB / 7.996 GiB available)
@hexaeder hexaeder added the bug Something isn't working label Aug 5, 2024
@vchuravy
Copy link
Member

vchuravy commented Aug 5, 2024

Hm these are the only adapt rules defined for the CUDABackend

Adapt.adapt_storage(::CUDABackend, a::Array) = Adapt.adapt(CuArray, a)
Adapt.adapt_storage(::CUDABackend, a::CuArray) = a
Adapt.adapt_storage(::KA.CPU, a::CuArray) = convert(Array, a)

@maleadt
Copy link
Member

maleadt commented Aug 6, 2024

Is adapt(::Backend, x) even a public interface right now? It's mentioned nowhere in the docs, and also doesn't seem to be used by KA.jl at a quick glance, so I don't think you can expect the operation to be fleshed out.

@vchuravy
Copy link
Member

vchuravy commented Aug 6, 2024

I added it in JuliaGPU/KernelAbstractions.jl#364 inspired by DiffEqGPU and JuliaGPU/KernelAbstractions.jl#353

Ideally it would just mean the same as CuArray

@maleadt
Copy link
Member

maleadt commented Aug 6, 2024

Right, but as it stands it's not part of the public API for KA.jl users, so @hexaeder I would recommend just using adapt(CuArray, x) which is documented and tested. If you think adapt(::Backend, x) should become a thing, please open an issue on KA.jl and we can consider extending the behavior of the adaptors that back-ends need to implement (right now, it's explicitly only Array <-> GPUArray).

@maleadt maleadt closed this as completed Aug 6, 2024
@hexaeder
Copy link
Author

hexaeder commented Aug 6, 2024

Allright, I'll change it in my code! I probably stumbled over it by accident. adapt(backend, x) seems natural, and since it worked for simple cases, I just assumed that it is part of the official interface without rechecking the documentation. Sorry for the noise!

@vchuravy
Copy link
Member

vchuravy commented Aug 7, 2024

I just assumed that it is part of the official interface without rechecking the documentation. Sorry for the noise!

It's a fine assumption. I should have added it to the docs at the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants