crflynn / skgrf

scikit-learn compatible Python bindings for grf (generalized random forests) C++ random forest library
https://skgrf.readthedocs.io/en/stable/
GNU General Public License v3.0
30 stars 6 forks source link

Issue with numpy<1.20 #75

Closed awooddoughty closed 2 years ago

awooddoughty commented 3 years ago

First, wanted to say thank you so much for building this! I'd been using grf with rpy for awhile, but am so happy to be away from the R dependency!

Second, I'm having an issue with the py38 wheel with numpy<1.20. While I can install skgrf, when I try to use it numpy complains about ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 88 from C header, got 80 from PyObject. Upgrading to numpy >= 1.20 fixes the problem. Unfortunately I also need to install tensorflow, which has a strong dependency on numpy<1.20 (thanks google).

My assumption is that this is caused by skgrf being compiled with numpy>=1.20 (I've been able to use the py36 wheels successfully, since those would have been built on numpy<1.20). I don't know how hard it is to build/maintain wheels that use numpy<1.20 or if you have other suggestions for how best to proceed.

I have tried installing from source and building from main, but pip was having trouble with both and it's beyond my abilities to debug exactly what was wrong

So I'm wondering if you have thoughts on the best way around this.

Steps to reproduce: 1) Fresh env with py3.8 2) pip install numpy==1.19.5 skgrf 3) python; from skgrf import grf – ValueError 4) pip uninstall -y numpy 5) pip install numpy==1.20 6) python; from skgrf import grf – Succeeds

crflynn commented 3 years ago

I think you're right in that we probably need to do the builds with a lower version of numpy. I don't believe numpy is pinned on builds currently but I will experiment with setting it to an older version when I get a chance.

crflynn commented 3 years ago

Looks like scikit-learn sets numpy as a build requirement using a meta-package called oldest-supported-numpy rather than numpy. That might be what we need here.

crflynn commented 3 years ago

@awooddoughty can you try the latest release 0.2.1?

awooddoughty commented 2 years ago

Yeah, that seems to work! Thank you!