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

RFC: add GPU testing #646

Merged
merged 9 commits into from
Aug 1, 2022
Merged

RFC: add GPU testing #646

merged 9 commits into from
Aug 1, 2022

Conversation

mcabbott
Copy link
Member

@mcabbott mcabbott commented Jul 14, 2022

This wants to start adding GPU tests...

  • This uses the fake JLArray from New package: JLArrays v0.1.0 JuliaRegistries/General#64443 so that this can run on github actions / random laptops.
  • Adding @gpu in front of test_rrule(sum, rand(3)) expands to first run the test as before, then check the GPU result. No attempt is made to run all the tests this way.

Xref #617 -- note that there is now GPUArraysCore.jl, so CR can use @allowscalar, and dispatch on AbstractGPUArray, without loading anything heavy.

Xref #645 -- that particular rule needs to be re-written, marked broken here.

@mcabbott mcabbott marked this pull request as draft July 14, 2022 19:55
@mcabbott mcabbott force-pushed the gputest branch 4 times, most recently from 603dd88 to 468b7fd Compare July 21, 2022 22:11
test/test_helpers.jl Outdated Show resolved Hide resolved
Expr(:block, ex, Expr(:macrocall, Symbol("@test_broken"), __source__, ex2)) |> esc
else
Expr(:block, ex, Expr(:macrocall, Symbol("@test"), __source__, ex2)) |> esc
# NB this @test should report a line number in your tests, not here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@oxinabox oxinabox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. Once you attend to the remaining things, I think this can be merged

@mcabbott mcabbott marked this pull request as ready for review August 1, 2022 16:08
@mcabbott mcabbott merged commit 36f3ce7 into JuliaDiff:main Aug 1, 2022
@mcabbott mcabbott deleted the gputest branch August 1, 2022 18:29
@mcabbott mcabbott added the GPU label Aug 2, 2022
@sethaxen
Copy link
Member

sethaxen commented Aug 4, 2022

@mcabbott is there a reason why a release wasn't registered after merging this PR or was it an oversight?

@mcabbott
Copy link
Member Author

mcabbott commented Aug 4, 2022

No strong reason! It doesn't change any functionality, only testing, maybe that's why.

@sethaxen
Copy link
Member

sethaxen commented Aug 4, 2022

No strong reason! It doesn't change any functionality, only testing, maybe that's why.

The PR bumped the minor version though. Should we then make a release with that minor version or save it for the next release?

@sethaxen
Copy link
Member

sethaxen commented Aug 6, 2022

Since this is blocking 2 releases, I'm going to assume the version bump was intentional and release a new version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants