Strided.jl icon indicating copy to clipboard operation
Strided.jl copied to clipboard

Conections with TransmuteDims.jl, TensorCast.jl and CUDA.jl v5.

Open tomsmierz opened this issue 2 years ago • 7 comments

In the pull request (link) in TransmuteDims.jl, which is part of the TensorCast.jl package, about updating dependency for Strided.jl from v1 to v2 there was mentioned some code problems, which prevent this update (most probably problems lies with UnsafeStridedView). This dependency issue prevents compatibility of TensorCast.jl with CUDA.jl v4 and v5.

The author of TransmuteDims.jl ( mcabbott) mentioned that these problems and required changes may be obvious to authors of Strided.jl. If you have time, can you look into it?

tomsmierz avatar Nov 18 '23 12:11 tomsmierz

Hi, I am certainly willing to look into this, if one could point me to what exactly I should be looking for. It will anyway be interesting, because other than dropping the support for the UnsafeStridedViews, there should not have been breaking changes.

Jutho avatar Nov 22 '23 21:11 Jutho

That would be wonderful, sorry I've been snowed under & haven't got around to trying. This file is the all that touches Strided.jl.

Edit: when I make a quick attempt, this line is the problem, as I'm not sure what should replace UnsafeStridedView(pointer(dst), size(dst), strides(dst), 0, identity).

mcabbott avatar Nov 23 '23 21:11 mcabbott

Any updates?

tomsmierz avatar Dec 01 '23 13:12 tomsmierz

Asking again if there are any updates. It is an important issue, required to move to CUDA 5.

tomsmierz avatar Feb 02 '24 10:02 tomsmierz

Thanks for the reminder, I keep forgetting about this one. UnsafeStridedView is completely removed from Strided, because the overhead of dealing with structs that point to Arrays should have been resolved in Julia.

Hence, the whole block

    if isbitstype(T)
        _A = UnsafeStridedView(pointer(A), sz, st, 0, identity)
        _B = UnsafeStridedView(pointer(dst), size(dst), strides(dst), 0, identity)
        _densecopy_strided!(_B, _A)
    else
        # This path may be slower than the default one. Not really tested, either.
        _densecopy_strided!(dst, StridedView(A, sz, st, 0, identity))
    end

should just be replaced with

        _densecopy_strided!(dst, StridedView(A, sz, st, 0, identity))

Also, the _densecopy_strided! implementation:

@inline function _densecopy_strided!(dst, src)
    T = eltype(dst)
    LinearAlgebra.axpby!(one(T), src, zero(T), dst)
end

can actually just be replaced with Base.copy!.

Jutho avatar Feb 02 '24 10:02 Jutho

hey @mcabbott, can you implement these changes?

tomsmierz avatar Feb 05 '24 09:02 tomsmierz

Thanks for the info above.

I was also trying a bit here: https://github.com/mcabbott/TransmuteDims.jl/pull/43 and got tests passing, but perhaps there's a bit more to do.

mcabbott avatar Feb 05 '24 20:02 mcabbott

Can I assume that this can be closed? If not, feel free to reopen or open new issues.

Jutho avatar Mar 20 '25 17:03 Jutho