broadcast for ^

[chengchingwen]

broadcast ^ for CuArray since there is no Base.pow

[vchuravy]

Thank you!

[vchuravy]

bors try

[chengchingwen]

I only add :^ to _cufuncs without literal_pow because I think no one would manually call the literal_pow

[bors]

try

Build succeeded

• ci/gitlab/trying

[vchuravy]

Can you open a PR to CUDAnative with the definition for CUDAnative.pow(x::Union{Float32, Float64}, y::Int64) = CUDAnative.pow(x, Int32(y))? Otherwise this looks GTM.

[maleadt]

I guess this depends on JuliaGPU/CUDAnative.jl#394?
bors try
We should probably conditionalize the test, or use an Int32 exponent to have it work on a released version of CUDAnative.

[chengchingwen]

@maleadt It does depends on JuliaGPU/CUDAnative.jl#394. What about calling Int32 inside the culiteral_pow?
https://github.com/chengchingwen/CuArrays.jl/blob/88695f0f0170a37b7880070c95c9fc4aa2b0e13e/src/broadcast.jl#L41

[bors]

try

Build failed

• ci/gitlab/trying

[chengchingwen]

@maleadt I use Int32 in culiteral_pow so that it didn't need JuliaGPU/CUDAnative.jl#394.

[chengchingwen]

Since this PR was only meant to handle the problem of broadcasting ^, I guess it would better to change as less code as possible and leave the type problem to CUDAnative

[chengchingwen]

@maleadt I also add the diffrule for pow, so now it can work with the ForwardDiff.

[maleadt]

Should we have a test for the ForwardDiff logic?
I'm not too familiar with this code though, so @vchuravy or @MikeInnes please merge if this is OK.

[MikeInnes]

Looks generally good. Testing a bit more comprehensively would be nice, but since there's no ForwardDiff test setup right now AFAIK that may be a big ask. Are we testing the various literal / non-literal power cases?

There's also a merge conflict that needs fixing.

[chengchingwen]

@MikeInnes the conflict is because the master of CuArrays move ForwardDiff to another file, so I just merge the master of CuArrays to this PR. Since the ForwardDiff is now using with the Require, are there any way to test it?

[MikeInnes]

You could add ForwardDiff as a test dependency. Honestly it might just be better to have it as a regular dependency at this point; I would want to check that the other maintainers don't object, though.

[maleadt]

Honestly it might just be better to have it as a regular dependency at this point; I would want to check that the other maintainers don't object, though.

I moved it out to reduce the set of dependencies to test CuArrays on my development system. Requires makes much sense here IMO, wouldn't it be equivalent to add some CI rule to make sure ForwardDiff is installed?

[chengchingwen]

@MikeInnes I could try to add some tests for ForwardDiff with CuArrays, but I'm totally unfamiliar with ForwardDiff. The implementation of pow was tested with Tracker. Could you give me some guide on how to do it?

[chengchingwen]

So I finally find a way to test all the ForwardDiff and also fix some function that are not able to use the diffrule directly (due to calling the native function with literal number/Int64)

[chengchingwen]

@MikeInnes Anything else to do here?

[maleadt]

bors r+

[bors]

Build failed

• ci/gitlab/staging

[chengchingwen]

@maleadt Could you try again, this should pass all the test.

[maleadt]

bors r+

[bors]

Build succeeded

• ci/gitlab/staging