gamma, lgamma, digamma work with GPU #294

xukai92 · 2018-03-31T22:33:33Z

Related issues:
#290
#292

Related PR:
#291

deps/cuda1.jl

CarloLucibello · 2018-04-01T03:43:27Z

in order to pass tests, we should:

add a REQUIRE file in the test directory containing the single line SpecialFunctions. Even better, I'd like SpecialFunctions to be a proper dependence of the package, if @denizyuret agrees. It is a lean and well mantained package.
define the derivative of trigamma(x) as poligamma(2,x) or just avoid testing testing its grad in test/unary.jl

.

xukai92 · 2018-04-03T03:49:59Z

How should I "define the derivative of trigamma(x) as poligamma(2,x)" in Knet.jl? I think that's in AutoGrad.jl isn't it?

CarloLucibello · 2018-04-03T04:32:22Z

hmhm, looks like trigamma derivative is already defined in autograd, but polygamma(2,x) has to be defined for KnetArray. So maybe add something along the line of (just a guess):

    J=broadcast_func("polygamma")
    for S in (32,64)
        T = Symbol("Float$S")
        F = "polygamma_impl_$S"
        @eval begin
            function $J(n::Int, x::KnetArray{$T})
                y = similar(x)
                @knet8($F,(Cint, Cint,Ptr{$T},Ptr{$T}), n, length(y), x, y)
                return y
            end
        end
    end

denizyuret · 2018-04-03T04:48:53Z

We can make SpecialFunctions a requirement for AutoGrad.

…

On Tue, Apr 3, 2018, 07:32 Carlo Lucibello ***@***.***> wrote: hmhm, looks like trigamma derivative is already defined in autograd, but poligamma(2,x) has to be defined for KnetArray — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#294 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvNpn8-AMG11-bcNUjzy7KwmRMSw6ANks5tkvtYgaJpZM4TCosn> .

CarloLucibello · 2018-04-03T13:54:59Z

@xukai92 if you don't need trigamma's derivative for KnetArray, the easiest way out (once you fix the problem with importing SpecialFunctions) is to just skip the test in test/unary.jl

@testset "unary" begin
    broken_grads = [trigamma] 
    for f in unary_fns
        f in broken_grads && continue
        ....

xukai92 · 2018-09-02T16:49:35Z

@CarloLucibello Just come back to work on this PR. I merged with master and it seems everything passes (even I didn't exclude the test as suggested by you long time ago).

CarloLucibello · 2018-09-02T20:17:40Z

I tried this pr locally and it works fine for gamma, lgamma, digamma and also trigamma's derivatives for KnetArrays. We should merge this.

denizyuret · 2018-09-02T21:23:56Z

I get these errors when I include("test/unary.jl"), is this expected? Some seem to be gradcheck errors, some isapprox errors comparing with cpu for digamma and trigamma.

xukai92 · 2018-09-02T22:08:06Z

I didn't get errors when I run the complete test on my local. Looking at you gist, it seems that some outputs seem to correct but still reported? E.g.

(f, t, n) = (SpecialFunctions.digamma, Float64, (2, 1))
unary: Test Failed at /home/ec2-user/.julia/dev/Knet/test/unary.jl:39
  Expression: isapprox(cy, Array(gy))
   Evaluated: isapprox([-1.8009; -1.25649], [-1.8009; -1.25649])
Stacktrace:
 [1] macro expansion at /home/ec2-user/.julia/dev/Knet/test/unary.jl:36 [inlined]
 [2] macro expansion at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Test/src/Test.jl:1083 [inlined]
 [3] top-level scope at /home/ec2-user/.julia/dev/Knet/test/unary.jl:6

PS: I'm not very familiar with Knet's test scripts.

denizyuret · 2018-09-02T22:19:50Z

The isapprox errors may be due to small floating differences between the cpu and gpu, the number of digits displayed probably does not reflect the difference. This is not a big deal, we can call isapprox with a larger rtol. Did include("test/unary.jl") pass on your side with all gradients etc? (You can uncomment the @show lines to see what it is testing)

…

On Sun, Sep 2, 2018 at 6:08 PM Kai Xu ***@***.***> wrote: I didn't get errors when I run the complete test on my local. Looking at you gist, it seems that some outputs seem to correct but still reported? E.g. (f, t, n) = (SpecialFunctions.digamma, Float64, (2, 1)) unary: Test Failed at /home/ec2-user/.julia/dev/Knet/test/unary.jl:39 Expression: isapprox(cy, Array(gy)) Evaluated: isapprox([-1.8009; -1.25649], [-1.8009; -1.25649]) Stacktrace: [1] macro expansion at /home/ec2-user/.julia/dev/Knet/test/unary.jl:36 [inlined] [2] macro expansion at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Test/src/Test.jl:1083 [inlined] [3] top-level scope at /home/ec2-user/.julia/dev/Knet/test/unary.jl:6 PS: I'm not very familiar with Knet's test scripts. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#294 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvNpkLEv-Bb9C6EcTiMXMflxz3X17e4ks5uXFbHgaJpZM4TCosn> .

xukai92 · 2018-09-02T22:36:59Z

Oh yes - I also have these errors. We should I do?

xukai92 · 2018-09-02T23:08:22Z

Should I set separate rtol for these functions or something?

denizyuret · 2018-09-02T23:55:12Z

We can fix rtol, but what about the gradient errors? They look more serious, or do you not get them?

…

On Sun, Sep 2, 2018 at 7:08 PM Kai Xu ***@***.***> wrote: Should I set separate rtol for these functions or something? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#294 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvNphT1eKgqSqIzwHISC6w_9tMnX0mSks5uXGTmgaJpZM4TCosn> .

denizyuret · 2018-09-03T01:13:45Z

In particular Float32 gradients for digamma/trigamma do not work for me:

julia> f(x) = sum(digamma.(x))
f (generic function with 1 method)

julia> grad(f)(rand(Float32,1))
1-element Array{Float32,1}:
 6.1312017

julia> grad(f)(ka(rand(Float32,1)))
1-element KnetArray{Float32,1}:
 NaN

julia> grad(f)(ka(rand(Float64,1)))
1-element KnetArray{Float64,1}:
 2.3219245818432324

xukai92 · 2018-09-11T00:40:22Z

Thanks for pointing this out. I found that I didn't implement the float and double version in two separate functions correctly. I know how to fix that in C but I don't know how to link them to the same Julia function in ops.jl.

Suppose I have gamma_impl_32 and gamma_impl_64 as the float and double versions in C, how should I link them to the gamma function in Julia?

xukai92 · 2018-09-11T00:57:55Z

Seems that simply using the double version can handle both.

julia> f(x) = sum(digamma.(x))
f (generic function with 1 method)

julia> rs_32 = rand(Float32, 1)
1-element Array{Float32,1}:
 0.35235214

julia> grad(f)(rs_32)
1-element Array{Float32,1}:
 9.129284

julia> grad(f)(ka(rs_32))
1-element KnetArray{Float32,1}:
 9.129284

julia> rs_64 = rand(Float64, 1)
1-element Array{Float64,1}:
 0.6063909020012128

julia> grad(f)(rs_64)
1-element Array{Float64,1}:
 3.573492967407559

julia> grad(f)(ka(rs_64))
1-element KnetArray{Float64,1}:
 3.573492967407558

Is it OK to do so?

xukai92 · 2019-01-04T22:51:14Z

@CarloLucibello Can we merge this?

CarloLucibello · 2019-01-07T06:19:51Z

Where is the code for the kernels coming from? We should credit the author and avoid issues with licenses.

Also, it's not clear what is happening here with the 32/64 issue. Is the kernel being executed at 64 bit precision also when using float 32 knet arrays? If so, we should fix this

xukai92 · 2019-01-07T13:31:21Z

Where is the code for the kernels coming from? We should credit the author and avoid issues with licenses.

It's based on https://github.com/rachtsingh/lgamma, which is again ported from https://bitbucket.org/eigen/eigen/overview. How should I cite it?

Also, it's not clear what is happening here with the 32/64 issue. Is the kernel being executed at 64 bit precision also when using float 32 knet arrays? If so, we should fix this

Let me try to solve this

xukai92 · 2019-03-26T04:40:22Z

deps/gamma.jl

@@ -0,0 +1,242 @@
+# Gamma family
+# Acknowledgement:


@CarloLucibello I added a comment to acknowledge the original files. Is it OK?

xukai92 · 2019-03-26T04:41:40Z

deps/cuda1.jl

+include("gamma.jl")
+print(fp,cuda1gammafamily())
+
+function cuda1src(f, j=f, ex="$f(xi)"; seperate_impl=false, BLK=256, THR=256)


@denizyuret I amended the cuda1src function to support functions which have different CUDA kernel implementation for float and double. Before the kernel is assumed to be same. Does it look good?

@CarloLucibello This float v.s. double issues is solved by this.

xukai92 · 2019-03-26T04:43:09Z

@CarloLucibello I resolved the comments we had before. Please see my notes on changes.

xukai92 · 2019-03-26T04:45:15Z

deps/gamma.jl

+    factorial *= (i + 1);
+  }
+
+  $T s = n % 2 == 0 ? -$one_str : $one_str;


Just for people who wonder why we had numerical errors before: this line was coded as s = powf(-1.0, n + 1) in the original code for float. This causes numerical errors somehow. I changed it to avoid using unnecessary power function to avoid this.

xukai92 · 2019-04-01T11:53:54Z

@CarloLucibello Can you take a look at the my updates and see if it's good now? Thanks!

CarloLucibello · 2019-04-01T14:36:14Z

test/REQUIRE

@@ -0,0 +1 @@
+SpecialFunctions


I think we don't need this, since SpecialFunctions is already in the REQUIRE of the package

CarloLucibello · 2019-04-01T14:57:30Z

thanks for your patience and for the good work!

xukai92 · 2019-04-01T14:58:52Z

Many thanks for your suggestions on improving codes in this PR!

gamma, lgamma, digamma work with GPU

6477f94

CarloLucibello reviewed Apr 1, 2018

View reviewed changes

deps/cuda1.jl Outdated Show resolved Hide resolved

CarloLucibello mentioned this pull request Apr 17, 2018

add gamma and lgamma for knet array #291

Closed

xukai92 mentioned this pull request Sep 1, 2018

CUDA gradient support for lgamma, lbeta and lgamma functions FluxML/Flux.jl#383

Closed

merge with master

8496c9c

denizyuret added the enhancement label Sep 2, 2018

using double only

a37ed79

using double only

cdb2b55

xukai92 added 3 commits March 26, 2019 00:23

Merge remote-tracking branch 'upstream/master' into gamma-gpu

8a473b3

ignore unnecessary file

07b9aba

split gamma impl from cuda1

a590c59

xukai92 added 5 commits March 26, 2019 00:52

fix test in local

3afbe53

rename gamma_impl.jl -> gamma.jl

0e8c812

resolve 32 and 64 mapping for gamma impl

c62646f

fix polygamma bug in c

85fad3d

add acknowledgement

b588ac0

xukai92 commented Mar 26, 2019

View reviewed changes

CarloLucibello reviewed Apr 1, 2019

View reviewed changes

remove unnecessary REQUIRE for test

ba52198

CarloLucibello approved these changes Apr 1, 2019

View reviewed changes

CarloLucibello merged commit 7a97d17 into denizyuret:master Apr 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gamma, lgamma, digamma work with GPU #294

gamma, lgamma, digamma work with GPU #294

xukai92 commented Mar 31, 2018 •

edited

CarloLucibello commented Apr 1, 2018 •

edited

xukai92 commented Apr 3, 2018

CarloLucibello commented Apr 3, 2018 •

edited

denizyuret commented Apr 3, 2018 via email

CarloLucibello commented Apr 3, 2018 •

edited

xukai92 commented Sep 2, 2018

CarloLucibello commented Sep 2, 2018

denizyuret commented Sep 2, 2018

xukai92 commented Sep 2, 2018

denizyuret commented Sep 2, 2018 via email

xukai92 commented Sep 2, 2018

xukai92 commented Sep 2, 2018

denizyuret commented Sep 2, 2018 via email

denizyuret commented Sep 3, 2018

xukai92 commented Sep 11, 2018 •

edited

xukai92 commented Sep 11, 2018

xukai92 commented Jan 4, 2019

CarloLucibello commented Jan 7, 2019

xukai92 commented Jan 7, 2019

xukai92 Mar 26, 2019

xukai92 Mar 26, 2019

xukai92 Mar 26, 2019

xukai92 commented Mar 26, 2019

xukai92 Mar 26, 2019

xukai92 commented Apr 1, 2019

CarloLucibello Apr 1, 2019

xukai92 Apr 1, 2019

CarloLucibello commented Apr 1, 2019

xukai92 commented Apr 1, 2019

gamma, lgamma, digamma work with GPU #294

gamma, lgamma, digamma work with GPU #294

Conversation

xukai92 commented Mar 31, 2018 • edited

CarloLucibello commented Apr 1, 2018 • edited

xukai92 commented Apr 3, 2018

CarloLucibello commented Apr 3, 2018 • edited

denizyuret commented Apr 3, 2018 via email

CarloLucibello commented Apr 3, 2018 • edited

xukai92 commented Sep 2, 2018

CarloLucibello commented Sep 2, 2018

denizyuret commented Sep 2, 2018

xukai92 commented Sep 2, 2018

denizyuret commented Sep 2, 2018 via email

xukai92 commented Sep 2, 2018

xukai92 commented Sep 2, 2018

denizyuret commented Sep 2, 2018 via email

denizyuret commented Sep 3, 2018

xukai92 commented Sep 11, 2018 • edited

xukai92 commented Sep 11, 2018

xukai92 commented Jan 4, 2019

CarloLucibello commented Jan 7, 2019

xukai92 commented Jan 7, 2019

xukai92 Mar 26, 2019

Choose a reason for hiding this comment

xukai92 Mar 26, 2019

Choose a reason for hiding this comment

xukai92 Mar 26, 2019

Choose a reason for hiding this comment

xukai92 commented Mar 26, 2019

xukai92 Mar 26, 2019

Choose a reason for hiding this comment

xukai92 commented Apr 1, 2019

CarloLucibello Apr 1, 2019

Choose a reason for hiding this comment

xukai92 Apr 1, 2019

Choose a reason for hiding this comment

CarloLucibello commented Apr 1, 2019

xukai92 commented Apr 1, 2019

xukai92 commented Mar 31, 2018 •

edited

CarloLucibello commented Apr 1, 2018 •

edited

CarloLucibello commented Apr 3, 2018 •

edited

CarloLucibello commented Apr 3, 2018 •

edited

xukai92 commented Sep 11, 2018 •

edited