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

Add scatter for CUDA support #1

Merged
merged 13 commits into from
May 2, 2021
Merged

Conversation

yuehhua
Copy link
Member

@yuehhua yuehhua commented Apr 12, 2021

Transfer from FluxML/NNlib.jl#296.

src/scatter.jl Outdated Show resolved Hide resolved
src/NNlibCUDA.jl Outdated Show resolved Hide resolved
src/scatter.jl Outdated Show resolved Hide resolved
src/scatter.jl Outdated Show resolved Hide resolved
@yuehhua
Copy link
Member Author

yuehhua commented Apr 16, 2021

Wait for merge of JuliaGPU/CUDA.jl#842

@yuehhua
Copy link
Member Author

yuehhua commented Apr 20, 2021

Now this PR relies on NNlib#master (or next patch release of v0.7.19) and CUDA#master (or next patch release of v3.0.4).

@maleadt

This comment has been minimized.

@yuehhua

This comment has been minimized.

src/scatter.jl Outdated Show resolved Hide resolved
@maleadt

This comment has been minimized.

@yuehhua
Copy link
Member Author

yuehhua commented May 2, 2021

@maleadt Could you trigger the CI test again? I think it is ready to go.
All tests passed on my PC.

@maleadt
Copy link
Contributor

maleadt commented May 2, 2021

You can always amend and force-push to re-trigger CI.

@CarloLucibello
Copy link
Member

You can also close and reopen to retrigger

@yuehhua
Copy link
Member Author

yuehhua commented May 2, 2021

I need at least NNlib v0.7.19 and CUDA v3.0.4 to pass CI, but buildkite just have the following version.

[052768ef] CUDA v3.0.0
[f6369f11] ForwardDiff v0.10.18
[872c559c] NNlib v0.7.18

As I know, there should be compatible with current version.

@maleadt
Copy link
Contributor

maleadt commented May 2, 2021

Then you need to bump the versions in the Manifest. Buildkite doesn't use specific package versions.

@yuehhua
Copy link
Member Author

yuehhua commented May 2, 2021

Thank you. It works.

@CarloLucibello
Copy link
Member

The compat bounds should also be updated

@yuehhua
Copy link
Member Author

yuehhua commented May 2, 2021

The compat bounds look somewhat like this?

@CarloLucibello CarloLucibello merged commit 94ea8b3 into FluxML:master May 2, 2021
@CarloLucibello
Copy link
Member

And this other little piece is done, thank you for all these efforts!

@maleadt
Copy link
Contributor

maleadt commented May 2, 2021

FWIW, it would have been better to squash and not merge this PR. Now there's 13 commits added to master for a single feature, many of which fail CI (complicating operations like git bisect).

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.

3 participants