LuxDL / Lux.jl

Elegant & Performant Scientific Machine Learning in Julia
https://lux.csail.mit.edu/
MIT License
446 stars 50 forks source link

Add ReverseSequence #698

Closed NeroBlackstone closed 2 weeks ago

NeroBlackstone commented 3 weeks ago

I don't know if this is the correct implementation

NeroBlackstone commented 3 weeks ago

If dims is specified,

  1. AbstractVector{T} and !isbitstype(T) throws an error

Hi, I'm curious why isbitstype() is needed here. For any 1D array, we can't specify dim right? So why we need isbitstype(), it would be nice if you could give an example with specific code. Thank you very much.

NeroBlackstone commented 3 weeks ago

I have updated the implementation according to your guidance, except for the doubtful points above.

NeroBlackstone commented 3 weeks ago

test failed on gpu, I can't reproduce on local since I don't have AMDGPU or CUDA devices...

ludvigk commented 3 weeks ago

I think it fails because Base.Iterators.Reverse{CuArray{Float32, 1, CUDA.Mem.DeviceBuffer}} is not isbitstype. CUDA doesn't like that. Even a minimal example like this fails for that reason:

g = CUDA.zeros(1)
gr = Iterators.reverse(g)
g .* gr
ludvigk commented 3 weeks ago

Using reverse instead of Iterators.reverse on arrays of type GPUArraysCore.AbstractGPUArray should fix this.

NeroBlackstone commented 2 weeks ago

Using reverse instead of Iterators.reverse on arrays of type GPUArraysCore.AbstractGPUArray should fix this.

I got a PC with GPU and test locally.

I found Float64 works well but Int throws errors. Weird...

avik-pal commented 2 weeks ago

Add this layer to the docs, also rebase with main. That will fix the doc build

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.13%. Comparing base (acd924f) to head (dbeb848).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #698 +/- ## ========================================== - Coverage 87.34% 81.13% -6.21% ========================================== Files 50 50 Lines 2505 2513 +8 ========================================== - Hits 2188 2039 -149 - Misses 317 474 +157 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

NeroBlackstone commented 2 weeks ago

I noticed some test failures, should I follow up on this PR? Or leave all the rest to you?

avik-pal commented 2 weeks ago

Github won't let me edit files directly. So if you add the docs to https://github.com/LuxDL/Lux.jl/blob/main/docs/src/api/Lux/layers.md#misc-helper-layers the build will pass