search

xmlyqing00 / Cholmod-Scikit-Sparse-Windows

Set up cholmod and scikit-sparse python package on Windows.
GNU General Public License v3.0
37 stars 11 forks source link

Bugfix #3

Closed stnoh closed 4 years ago

stnoh commented 4 years ago

I recently tried your repository and suffered from the similar problem in #1 .
After some checking, I found that lapack-blas DLLs (libblas.dll, ...) were missing at the installation.
I guess, these DLLs were located in your system path but they weren't in the other guys system.

Anyway, the simplistic solution of #1 is just copy lapack-blas DLLs (libblas.dll, ...) to sksparse directory after the installation, but I think that it is quite annoying. Here I complement setup.py by using data_files option to copy DLLs at the installation (0329c1e).

Replacing Cython (d49d172) is just my preference, but I believe it is reasonable since this repository only aims to support Windows.

xmlyqing00 commented 4 years ago

Thank you @stnoh ! Good job! DLLs parts are clear.

I'm not familiar with Cython. Is it another compile option? Could you explain more what benefits does Cython bring in this package? Maybe writing some sentences in the README.md could be better. Thank you!

stnoh commented 4 years ago

AFAIK, Cython is a kind porting option from C to Python. Since C is binary-dependent, Linux version Cython won't match with Windows'. I know there are few other options for C-to-Python porting (pybind11, for example), but I do not want to change many things from the original one...
Anyway, here it is used for creating sksparse/cholmod.cp36-win_amd64.pyd (You can check it from egg file) from scikit-sparse-0.4.4/sksparse/cholmod.c . CHOLMOD is built by Python, but it still wants to use binary version of lapack-blas DLLs. This was the reason of DLL problem in #1 .

xmlyqing00 commented 4 years ago

I see. It's clear. Thank you for your contributions.