search

h2oai / h2o4gpu

H2Oai GPU Edition
Apache License 2.0
460 stars 95 forks source link

Llvmremove #678

Closed pseudotensor closed 6 years ago

mdymczyk commented 6 years ago

@pseudotensor looks like llvmlite is required by numba. Looks like we're only using it in one place to push numpy arrays to the GPU, maybe we could remove numba from requirements and substitute it with PyCUDA's to_gpu?

pseudotensor commented 6 years ago

@mdymczyk Hmm, but seems to be issue only for ppc builds. x86 builds went through fine. So I guess numba is getting re-compiled for ppc and that needed llvm.

But originally, I deduced that llvm was only added due to pyenv requirements, which we no longer use.

So I wonder if we can avoid llvm requirement for ppc. Maybe removing numba, but I feel odd that I never added llvm for numbda, only for pyenv.

https://github.com/pyenv/pyenv/wiki/Common-build-problems#requirements

mdymczyk commented 6 years ago

@pseudotensor it looks like on ppc llvm isn't even installed and that's why the Python install of llvmlite fails (it complains about missing LLVM_CONFIG). So I guess that's why we were downloading it and setting up all the env variables (to install it).

On the other hand llvm is required by numba https://github.com/numba/numba/blob/master/requirements.txt#L2 which we have in requirements but only use it here (and in one notebook).

So maybe we can remove numba and use the pycuda stuff I linked in my previous comment.

pseudotensor commented 6 years ago

Ya, I understand your idea. Maybe can do, although maybe can keep it for now. @hemenkapadia modified the conda install to work and included llvm updates (that's why I started poking around since I didn't understand if that was necessary).

hemenkapadia commented 6 years ago

PR671 includes versions of llvmlite, numba and LLVM that compile for both ppc64le wheel and conda, and the LLVM is the same that is used for dai (i think datatable). If we think there is no issue to upgrade LLVM then we can go with whatever is done in PR 671 post your review.

This will atleast help us start release conda package for h2o4gpu to meet the request of customer. Then we can review and trim down unnecessary stuff if needed.

What do you think @pseudotensor and @mdymczyk

pseudotensor commented 6 years ago

I think it's ok if you merge your stuff @hemenkapadia , ignore my concern about llvm. Just make sure it's green :)