copied the old rnn.jl->rnncompat.jl for Flux compatibility #738

denizyuret · 2021-02-26T19:21:15Z

No description provided.

maleadt · 2021-03-01T07:11:10Z

Rebased on top of the compat branch, this is for 1.5 only, right? master will be a breaking release (3.0) anyway.

denizyuret · 2021-03-01T09:23:27Z

Rebased on top of the compat branch, this is for 1.5 only, right? master will be a breaking release (3.0) anyway.

This is up to Flux people, Knet does not use the interface in rnncompat.jl. I'd rather move Flux specific code out of CUDA/lib/cudnn into their respective packages.

codecov · 2021-03-01T10:52:24Z

Codecov Report

Merging #738 (5c0c942) into compat (d69be98) will decrease coverage by 1.15%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           compat     #738      +/-   ##
==========================================
- Coverage   79.10%   77.94%   -1.16%     
==========================================
  Files         123      124       +1     
  Lines        6705     6807     +102     
==========================================
+ Hits         5304     5306       +2     
- Misses       1401     1501     +100

Impacted Files	Coverage Δ
lib/cudnn/CUDNN.jl	`44.18% <ø> (ø)`
lib/cudnn/rnncompat.jl	`0.00% <0.00%> (ø)`
src/pool/binned.jl	`89.56% <0.00%> (+0.86%)`	⬆️
lib/cudadrv/devices.jl	`97.36% <0.00%> (+2.63%)`	⬆️

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 d69be98...5c0c942. Read the comment docs.

CarloLucibello · 2021-03-01T19:38:11Z

yes compat is enough, thanks @denizyuret

maleadt · 2021-03-02T06:53:38Z

IIUC the compat branch should now be ready for a release. @CarloLucibello, could you test?

denizyuret · 2021-03-03T05:49:07Z

yes compat is enough, thanks @denizyuret

No problem @CarloLucibello. There are a few compat files in CUDA/lib/cudnn that will probably be deprecated in the next major CUDA.jl release - if you want to keep using them in NNlib/Flux please copy them there:

rnncompat.jl (to be replaced with the new rnn interface in rnn.jl)
batchnorm.jl (to be replaced with the new normalization.jl interface)
nnlib.jl (a better place for this could be NNlib or an intermediate NNlibCUDA package)

maleadt · 2021-03-03T06:51:54Z

There are a few compat files in CUDA/lib/cudnn that will probably be deprecated in the next major CUDA.jl release

FWIW, the next release is bound to be a breaking one, CUDA.jl 3.0. So now is the time maybe to remove those? The alternative is to release another breaking release some time after that, but that may make people unhappy too.

CarloLucibello · 2021-03-03T07:35:16Z

it makes sense to remove the NNlib dependence already in 3.0. We'll create a NNlibCUDA.jl package as a subpackage of NNlib.
@DhairyaLGandhi @darsnack @ToucheSir

DhairyaLGandhi · 2021-03-03T08:07:48Z

We have a bunch of benefits from testing with the cuda system and checking with cudnn has been helpful as well.

darsnack · 2021-03-03T13:48:04Z

Is it not possible to do a similar test set up with a NNlibCUDA.jl?

We've been talking a bit on Zulip about this with @jpsamaroo. Seems like separating out backend code and glue code into contained packages might be nice when we add support for AMD GPUs. Would also be nicer for dependency management and bug fix releases.

CarloLucibello · 2021-03-09T05:10:51Z

@denizyuret I run Flux's test on the compat branch here and got this error

R = RNN: Error During Test at /home/carlo/.julia/packages/Flux/goUGu/test/cuda/curnn.jl:4
  Got exception outside of a @test
  UndefVarError: DropoutDesc not defined
  Stacktrace:
   [1] CUDA.CUDNN.RNNDesc{Float32}(::CUDA.CUDNN.cudnnRNNMode_t, ::Int64, ::Int64; layers::Int64) at /home/carlo/.julia/packages/CUDA/omtO5/lib/cudnn/rnncompat.jl:46
   [2] RNNDesc at /home/carlo/.julia/packages/CUDA/omtO5/lib/cudnn/rnncompat.jl:43 [inlined]
   [3] RNNDesc at /home/carlo/.julia/packages/Flux/goUGu/src/cuda/curnn.jl:13 [inlined]
   [4] desc(::Flux.RNNCell{typeof(tanh),CuArray{Float32,2},CuArray{Float32,1}}) at /home/carlo/.julia/packages/Flux/goUGu/src/cuda/curnn.jl:20
   [5] adjoint at /home/carlo/.julia/packages/Flux/goUGu/src/cuda/curnn.jl:70 [inlined]
   [6] _pullback at /home/carlo/.julia/packages/ZygoteRules/OjfTt/src/adjoint.jl:57 [inlined]
   [7] adjoint at /home/carlo/.julia/packages/Zygote/KpME9/src/lib/lib.jl:188 [inlined]
   [8] adjoint(::Zygote.Context, ::typeof(Core._apply_iterate), ::typeof(iterate), ::Flux.RNNCell{typeof(tanh),CuArray{Float32,2},CuArray{Float32,1}}, ::Tuple{CuArray{Float32,1}}, ::Tuple{CuArray{Float32,1}}) at ./none:0
   [9] _pullback at /home/carlo/.julia/packages/ZygoteRules/OjfTt/src/adjoint.jl:57 [inlined]
   [10] Recur at /home/carlo/.julia/packages/Flux/goUGu/src/layers/recurrent.jl:36 [inlined]
   [11] _pullback(::Zygote.Context, ::Flux.Recur{Flux.RNNCell{typeof(tanh),CuArray{Float32,2},CuArray{Float32,1}}}, ::CuArray{Float32,1}) at /home/carlo/.julia/packages/Zygote/KpME9/src/compiler/interface2.jl:0
   [12] #166 at /home/carlo/.julia/packages/Flux/goUGu/test/cuda/curnn.jl:7 [inlined]
   [13] _pullback(::Zygote.Context, ::var"#166#168"{CuArray{Float32,1}}, ::Flux.Recur{Flux.RNNCell{typeof(tanh),CuArray{Float32,2},CuArray{Float32,1}}}) at /home/carlo/.julia/packages/Zygote/KpME9/src/compiler/interface2.jl:0
   [14] _pullback(::Function, ::Flux.Recur{Flux.RNNCell{typeof(tanh),CuArray{Float32,2},CuArray{Float32,1}}}) at /home/carlo/.julia/packages/Zygote/KpME9/src/compiler/interface.jl:33
   [15] pullback(::Function, ::Flux.Recur{Flux.RNNCell{typeof(tanh),CuArray{Float32,2},CuArray{Float32,1}}}) at /home/carlo/.julia/packages/Zygote/KpME9/src/compiler/interface.jl:39
   [16] gradient(::Function, ::Flux.Recur{Flux.RNNCell{typeof(tanh),CuArray{Float32,2},CuArray{Float32,1}}}) at /home/carlo/.julia/packages/Zygote/KpME9/src/compiler/interface.jl:48
   [17] top-level scope at /home/carlo/.julia/packages/Flux/goUGu/test/cuda/curnn.jl:7
   [18] top-level scope at /build/julia/src/julia-1.5.3/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1190
   [19] include(::String) at ./client.jl:457
   [20] top-level scope at /home/carlo/.julia/packages/Flux/goUGu/test/cuda/runtests.jl:13
   [21] include(::String) at ./client.jl:457
   [22] top-level scope at /home/carlo/.julia/packages/Flux/goUGu/test/runtests.jl:39
   [23] top-level scope at /build/julia/src/julia-1.5.3/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1115
   [24] top-level scope at /home/carlo/.julia/packages/Flux/goUGu/test/runtests.jl:38
   [25] include(::String) at ./client.jl:457
   [26] top-level scope at none:6

denizyuret · 2021-03-09T13:10:23Z

This may be a simple fix: try copying the old https://github.com/JuliaGPU/CUDA.jl/blob/v2.6.1/lib/cudnn/dropout.jl into rnncompat.jl.

copied the old rnn.jl->rnncompat.jl for Flux compatibility

denizyuret mentioned this pull request Feb 26, 2021

Backport new CUDNN interface to 1.5 #635

Closed

maleadt added the backport label Mar 1, 2021

maleadt changed the base branch from master to compat March 1, 2021 07:06

maleadt changed the base branch from compat to master March 1, 2021 07:07

maleadt force-pushed the dy/rnncompat branch from 8b06266 to 6f28cda Compare March 1, 2021 07:08

maleadt changed the base branch from master to compat March 1, 2021 07:09

Copy the old rnn.jl->rnncompat.jl for Flux compatibility

5c0c942

maleadt force-pushed the dy/rnncompat branch from 6f28cda to 5c0c942 Compare March 1, 2021 07:10

maleadt removed the backport label Mar 1, 2021

maleadt merged commit b30ac5f into JuliaGPU:compat Mar 1, 2021

CarloLucibello mentioned this pull request Mar 6, 2021

create NNlibCUDA sub-package FluxML/NNlib.jl#286

Merged

maleadt added a commit that referenced this pull request Mar 16, 2021

Merge pull request #738 from denizyuret/dy/rnncompat

7166e05

copied the old rnn.jl->rnncompat.jl for Flux compatibility

avik-pal mentioned this pull request Jul 9, 2021

Accumulate doesn't work on >=4 dim Arrays with dims <= ndims(A) - 3 #1039

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

copied the old rnn.jl->rnncompat.jl for Flux compatibility #738

copied the old rnn.jl->rnncompat.jl for Flux compatibility #738

denizyuret commented Feb 26, 2021

maleadt commented Mar 1, 2021

denizyuret commented Mar 1, 2021

codecov bot commented Mar 1, 2021 •

edited

Loading

CarloLucibello commented Mar 1, 2021

maleadt commented Mar 2, 2021

denizyuret commented Mar 3, 2021

maleadt commented Mar 3, 2021

CarloLucibello commented Mar 3, 2021

DhairyaLGandhi commented Mar 3, 2021

darsnack commented Mar 3, 2021 •

edited

Loading

CarloLucibello commented Mar 9, 2021

denizyuret commented Mar 9, 2021

copied the old rnn.jl->rnncompat.jl for Flux compatibility #738

copied the old rnn.jl->rnncompat.jl for Flux compatibility #738

Conversation

denizyuret commented Feb 26, 2021

maleadt commented Mar 1, 2021

denizyuret commented Mar 1, 2021

codecov bot commented Mar 1, 2021 • edited Loading

Codecov Report

CarloLucibello commented Mar 1, 2021

maleadt commented Mar 2, 2021

denizyuret commented Mar 3, 2021

maleadt commented Mar 3, 2021

CarloLucibello commented Mar 3, 2021

DhairyaLGandhi commented Mar 3, 2021

darsnack commented Mar 3, 2021 • edited Loading

CarloLucibello commented Mar 9, 2021

denizyuret commented Mar 9, 2021

codecov bot commented Mar 1, 2021 •

edited

Loading

darsnack commented Mar 3, 2021 •

edited

Loading