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

Cuda 3.0 support #1571

Merged
merged 18 commits into from
Apr 23, 2021
Merged

Cuda 3.0 support #1571

merged 18 commits into from
Apr 23, 2021

Conversation

DhairyaLGandhi
Copy link
Member

@DhairyaLGandhi DhairyaLGandhi commented Apr 10, 2021

@DhairyaLGandhi
Copy link
Member Author

@jonathan-laurent @maleadt could you confirm that this branch works fine for AlphaZero and CUDA 3. And also whether you've tested the subpackage from FluxML/NNlib.jl#286

@CarloLucibello
Copy link
Member

you can test the NNlibCUDA´s subpackage with

(@v1.6) pkg> add https://github.com/FluxML/NNlib.jl:lib/NNlibCUDA

@CarloLucibello
Copy link
Member

It´s a bit tricky to support both julia 1.5 and julia 1.6 at this point (NNlibCUDA requires julia 1.6). Should we drop julia 1.6 support?

@CarloLucibello
Copy link
Member

the Manifest here is adding CUDA 2.6 actually

@maleadt
Copy link
Collaborator

maleadt commented Apr 12, 2021

Now that we have device overrides, Base.exp etc are GPU compatible, so there's a couple of GPU-specific invocations of exp, log, ... that can go.

@jonathan-laurent
Copy link

jonathan-laurent commented Apr 15, 2021

What exactly should I do to test this? I tried the following without success:

]add Flux#dg/cuda16 NNlib#master CUDA@3.0.3

@DhairyaLGandhi
Copy link
Member Author

You'd need to check out a local copy and instantiate the manifest in the branch. You also need https://github.com/FluxML/NNlibCUDA.jl

@jonathan-laurent
Copy link

AlphaZero.jl works fine with CUDA 3.0 and this branch.

@DhairyaLGandhi
Copy link
Member Author

I think this is good to go, anyone have thoughts?

@CarloLucibello
Copy link
Member

the NNlibCUDA compat bound is missing and the Manifest could be removed. Beside that, looks good!

@DhairyaLGandhi
Copy link
Member Author

Thanks, added the bound, and I'd like to keep the manifest around for now, so I think we are good to go here.

@jonathan-laurent
Copy link

Is the plan to merge this in the next minor release of Flux? (I imagine this cannot be part of a patch release if you are dropping support for Julia <1.6).

@DhairyaLGandhi
Copy link
Member Author

Yeah, I was considering merging and releasing it now. Its allowed within semver to limit Julia versions, and we aren't breaking any Flux API so we are good. Any downstream breaking changes would need depending on CUDA explicitly so we are covered with a patch release.

@DhairyaLGandhi
Copy link
Member Author

DhairyaLGandhi commented Apr 22, 2021

ref FluxML/NNlib.jl#316

Its not a blocker, but good to have it be complete. On 1.6 (which is what Flux will bump Julia to), we should see the correct bounds.

@DhairyaLGandhi DhairyaLGandhi merged commit 509b21a into master Apr 23, 2021
@DhairyaLGandhi DhairyaLGandhi deleted the dg/cuda16 branch April 23, 2021 13:08
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

Successfully merging this pull request may close these issues.

4 participants