umm, not sure what did I break here... anyone might know?

Codecov Report

Merging #1106 (79fc651) into master (098106d) will increase coverage by 0.00%.
The diff coverage is 80.00%.

@@           Coverage Diff           @@
##           master    #1106   +/-   ##
=======================================
  Coverage   79.46%   79.47%           
=======================================
  Files         118      118           
  Lines        7919     7925    +6     
=======================================
+ Hits         6293     6298    +5     
- Misses       1626     1627    +1     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 098106d...79fc651. Read the comment docs.

Nice! Does the existing indexing function just work, or how do you want to use this type on device?

Typically I put device-side implementations in src/device, e.g., src/device/sparse.jl because it's not really tied to CUSPARSE.

And you didn't break anything, if you push a newer commit Buildkite is currently configured to cancel previous builds.

Nice! Does the existing indexing function just work, or how do you want to use this type on device?

I mainly just want to make it work when the kernel argument is a CuSparseMatrixCSC/R without getting the complain of not being a bitstype, for example when I try to write the following kernel,

function thread_layout(H)
    if size(H, 2) < 256
        threads = size(H, 2)
        nblocks = 1
    else
        threads = 256
        nblocks = ceil(Int, size(H, 2) / 256)
    end
    return threads, nblocks
end

function update_term_kernel(H, t, subspace_v)
    row = (blockIdx().x - 1) * blockDim().x + threadIdx().x

    if row > length(H.rowPtr) - 1
        return
    end

    @inbounds for k in H.rowPtr[row]:H.rowPtr[row+1]-1
        col = H.colVal[k]
        lhs = subspace_v[col]
        rhs = subspace_v[row]

        H.nzVal[k] = RydbergEmulator.term_value(t, lhs, rhs, col, row)
    end
    return
end

function RydbergEmulator.update_term!(H::CuSparseMatrixCSR, t::AbstractTerm, subspace_v::CuVector)
    LinearAlgebra.checksquare(H) == length(subspace_v) || error("given matrix size does not match subspace size")
    threads, nblocks = thread_layout(H)
    @cuda threads=threads blocks=nblocks update_term_kernel(H, t, subspace_v)
    return H
end

because CuSparseMatrixCSC or CuSparseMatrixCSR is not bitstype, thus I will get

ERROR: LoadError: GPU compilation of kernel update_term_kernel(CuSparseMatrixCSR{ComplexF32}, Hamiltonian{Tuple{RydInteract{Float64, StaticArrays.SVector{36, RydAtom{2, Int64}}}, XTerm{Float64, Nothing}, NTerm{Float64}}}, CuDeviceVector{Int64, 1}) failed
KernelError: passing and using non-bitstype argument

Argument 2 to your kernel function is of type CuSparseMatrixCSR{ComplexF32}, which is not isbits:
  .rowPtr is of type CuArray{Int32, 1, B} where B which is not isbits.
    .storage is of type Union{Nothing, CUDA.ArrayStorage{B}} where B which is not isbits.
  .colVal is of type CuArray{Int32, 1, B} where B which is not isbits.
    .storage is of type Union{Nothing, CUDA.ArrayStorage{B}} where B which is not isbits.
  .nzVal is of type CuArray{ComplexF32, 1, B} where B which is not isbits.
    .storage is of type Union{Nothing, CUDA.ArrayStorage{B}} where B which is not isbits.

so I think I will need to let adapt convert the CuSparseMatrixCSR type to a bitstype that contains CuDeviceVector only.

I'm trying to include some tests, but how should I include the tests in runtests.jl? I'm a bit lost on the test suite...

ah it seems the test file is added automatically, this test suite tool in CUDA looks quite nice! is there a package for this already? mind me stealing this feature to a package?

ah it seems the test file is added automatically, this test suite tool in CUDA looks quite nice! is there a package for this already? mind me stealing this feature to a package?

It's quite annoying to figure a good API to extract this into a package, so I didn't bother. Feel free to take the code, it's based on Base's runtests.jl. If you're not using the CUDA/NVML-specific stuff, oneAPI.jl has a simpler version of this runner you might want to copy instead.

I mainly just want to make it work when the kernel argument is a CuSparseMatrixCSC/R without getting the complain of not being a bitstype

Ah OK, so you're manually using the internal fields. We should probably add a note/TODO then explaining that the type currently doesn't have any user-facing functionality implemented, in case people expect to be able to pass this to a kernel and use getindex or nonzeros etc.

Ah OK, so you're manually using the internal fields. We should probably add a note/TODO then explaining that the type currently doesn't have any user-facing functionality implemented, in case people expect to be able to pass this to a kernel and use getindex or nonzeros etc.

OK, yeah, I'm not sure about the APIs for kernel programming regarding to the CSC/CSR struct, in my case I mainly want to use the internal fields directly to make an efficient implementation, and manually using 3 array pointers seems annoying. I think I'll add some docstring too.

actually I'm not sure where should I add the note/TODO, maybe I should put them in the documentation?

I pushed some changes (mostly decoupling from CUSPARSE, we want the generic implementation to be really generic) & clean-ups (e.g. don't use cudaconvert but use adapt). Should be good to go if this suits your needs, but ultimately we'd want some GPU-compatible device functionality too.

Ah thanks! Yeah this looks good to me! I think I can use the CSR directly now. Would this appear in a patch release soon?

This is pretty much a feature, and I wasn't planning on creating a new minor release just yet (it's been only a week). You can't use the master branch for the time being?

Yeah I was going to depend on a new version in another package but it's not in a hurry. So no problem then. I'll just use the master branch for now