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.

Sign up for GitHub

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

Jump to bottom

Add CUDA tests for stateless layers #906

Closed
wants to merge 1 commit into from
Closed

Add CUDA tests for stateless layers #906

wants to merge 1 commit into from

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Oct 23, 2019

A lot of @test_broken here because of CUDAnative log fighting and scalar getindex being disallowed.

@kshyatt
Copy link
Contributor Author

kshyatt commented Oct 23, 2019

bors try

@bors
Copy link
Contributor

bors bot commented Oct 23, 2019

🔒 Permission denied

Existing reviewers: click here to make kshyatt a reviewer

MikeInnes
MikeInnes reviewed Oct 24, 2019
View reviewed changes
test/cuda/cuda.jl
cx = gpu(x)
scx = σ.(cx)
cy = gpu(y)
@test_broken Flux.binarycrossentropy.(σ.(x), y) ≈ Flux.binarycrossentropy.(scx, cy)
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why you split out scx here, is it just a style thing or does it change the bug/behaviour somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was trying to see if the sigma was causing the compilation error, or the log.

@MikeInnes
Copy link
Member

MikeInnes commented Oct 24, 2019 •
edited
Loading

Hoping this will let you try (and feel free to merge if you are happy with it):

bors d+

@bors
Copy link
Contributor

bors bot commented Oct 24, 2019

✌️ kshyatt can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@kshyatt
Copy link
Contributor Author

kshyatt commented Oct 24, 2019

bors try

bors bot added a commit that referenced this pull request Oct 24, 2019
@bors
Try #906:
85ada45
@bors
Copy link
Contributor

bors bot commented Oct 24, 2019

try

Build failed

@DhairyaLGandhi
Copy link
Member

Failure on 1.0 seems network related

@DhairyaLGandhi
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Nov 5, 2019
@bors
Try #906:
ffc36d0
@bors
Copy link
Contributor

bors bot commented Nov 5, 2019

try

Build succeeded

@DhairyaLGandhi
Copy link
Member

bors try

Since #940 was merged, I think the normalise is what we miss currently

bors bot added a commit that referenced this pull request Nov 26, 2019
@bors
Try #906:
7ff3bb1
@bors
Copy link
Contributor

bors bot commented Nov 26, 2019

try

Build failed

@DhairyaLGandhi
Copy link
Member

Yup, seems like it

@DhairyaLGandhi
Copy link
Member

@maleadt can say more, but I think now we don't want to compare GPU Arrays with regular ones.

@CarloLucibello
Copy link
Member

@kshyatt if you don't mind I'll add these tests to #1264

@ToucheSir
Copy link
Member

#1264 was merged with non-broken gpu losses, so can this be closed?

@DhairyaLGandhi
Copy link
Member

Yup, thanks for nudging in the right direction @kshyatt !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MikeInnes MikeInnes MikeInnes left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kshyatt @MikeInnes @DhairyaLGandhi @CarloLucibello @ToucheSir