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

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

Open
tomsmierz opened this issue Nov 18, 2023 · 7 comments
Open

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

tomsmierz opened this issue Nov 18, 2023 · 7 comments

Comments

@tomsmierz
Copy link

tomsmierz commented Nov 18, 2023

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?

@Jutho
Copy link
Owner

Jutho commented Nov 22, 2023

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.

@mcabbott
Copy link
Contributor

mcabbott commented Nov 23, 2023

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).

@tomsmierz
Copy link
Author

Any updates?

@tomsmierz
Copy link
Author

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

@Jutho
Copy link
Owner

Jutho commented Feb 2, 2024

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!.

@tomsmierz
Copy link
Author

hey @mcabbott, can you implement these changes?

@mcabbott
Copy link
Contributor

mcabbott commented Feb 5, 2024

Thanks for the info above.

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

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

No branches or pull requests

3 participants