search

JuliaGPU / CLBLAS.jl

CLBLAS integration for Julia
Apache License 2.0
22 stars 14 forks source link

Sd/v05 #25

Closed SimonDanisch closed 7 years ago

SimonDanisch commented 7 years ago

Jeez, we really need to switch to defining unsafe_convert methods etc...

SimonDanisch commented 7 years ago

@maleadt @vchuravy can we have Tim CI? :)

maleadt commented 7 years ago

@SimonDanisch not sure why it didn't update... but I've activated CI, build on master succeeds https://ci.maleadt.net/buildbot/julia/builders/CLBLAS.jl%3A%20Julia%200.5%20%28x86-64%29/builds/1 but fails on this PR https://ci.maleadt.net/buildbot/julia/builders/CLBLAS.jl%3A%20Julia%200.5%20%28x86-64%29/builds/2

EDIT: nvm, I know why it didn't push the status. Let me fix it.

SimonDanisch commented 7 years ago

I use the prebuild binaries installed with apt-get

maleadt commented 7 years ago

clblas binaries on my CI are the ones from GitHub too (Debian stable doesn't offer them).

SimonDanisch commented 7 years ago

I'd like to leave the failures for another PR, since it should still be a strict improvement and it seems as if tests haven't been passing previously....

dfdx commented 7 years ago

No objections from my side. I believe we should currently pay more attention to GPUArrays anyway, the future of CLBLAS heavily depends on it. I expect we will still link to libclblas in some way, but in the end bindings and installation procedure might be quite different.

SimonDanisch commented 7 years ago

Great! Although I'm wondering a bit what's going on here... Were the tests actually passing at any point (locally) ?

dfdx commented 7 years ago

I checked CLBLAS's master a week ago on my Ubuntu box and it worked perfectly well. I can check this PR's branch in the evening.

dfdx commented 7 years ago

Tests pass fine on this branch with CUDA and GeForce GTX 960M given that Complex64 is excluded from tested types. Since this is a known and documented issue, I believe we can exclude this type and merge the PR.

SimonDanisch commented 7 years ago

Okay, I think this should be ready to go!

vchuravy commented 7 years ago

Tests for gemv? Otherwise LGTM

SimonDanisch commented 7 years ago

yeah they bring me a bit in a code duplication issue, since I now started to test them in GPUArrays... I guess the way forward is to rip all the high level stuff out of CLBLAS and do testing etc in GPUArrays

vchuravy commented 7 years ago

We still need to have some minimal tests here. But that shouldn't hold up this PR.

On Mon, 3 Apr 2017, 19:31 Simon, notifications@github.com wrote:

yeah they bring me a bit in a code duplication issue, since I now started to test them in GPUArrays... I guess the way forward is to rip all the high level stuff out of CLBLAS and do testing etc in GPUArrays

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/JuliaGPU/CLBLAS.jl/pull/25#issuecomment-291106058, or mute the thread https://github.com/notifications/unsubscribe-auth/AAI3aupZQVnV8BBpc_1Pd9tbnGI78WMeks5rsMqGgaJpZM4MFXQA .

SimonDanisch commented 7 years ago

sweet! :)