8000 Fail to pickle `SplineTransformer` with `scipy==1.15.0rc1` · Issue #30512 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

Fail to pickle SplineTransformer with scipy==1.15.0rc1 #30512

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.

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

Closed
FBruzzesi opened this issue Dec 19, 2024 · 8 comments
Closed

Fail to pickle SplineTransformer with scipy==1.15.0rc1 #30512

FBruzzesi opened this issue Dec 19, 2024 · 8 comments
Labels

Comments

@FBruzzesi
Copy link

Describe the bug

Spotted in scikit-lego, running check_estimators_pickle fails with SplineTransformer and readonly_memmap=True.

cc: @koaning

Steps/Code to Reproduce

from sklearn.utils.estimator_checks import check_estimators_pickle
from sklearn.preprocessing import SplineTransformer


check_estimators_pickle(
    name="hello",
    estimator_orig=SplineTransformer(),
    readonly_memmap=True,
)

Expected Results

Not to raise

Actual Results

Traceback (most recent call last):
  File "/home/fbruzzesi/open-source/scikit-lego/t.py", line 5, in <module>
    check_estimators_pickle(
  File "/home/fbruzzesi/open-source/scikit-lego/.venv-pre/lib/python3.10/site-packages/sklearn/utils/_testing.py", line 147, in wrapper
    return fn(*args, **kwargs)
  File "/home/fbruzzesi/open-source/scikit-lego/.venv-pre/lib/python3.10/site-packages/sklearn/utils/estimator_checks.py", line 2354, in check_estimators_pickle
    unpickled_result = getattr(unpickled_estimator, method)(X)
  File "/home/fbruzzesi/open-source/scikit-lego/.venv-pre/lib/python3.10/site-packages/sklearn/utils/_set_output.py", line 319, in wrapped
    data_to_wrap = f(self, X, *args, **kwargs)
  File "/home/fbruzzesi/open-source/scikit-lego/.venv-pre/lib/python3.10/site-packages/sklearn/preprocessing/_polynomial.py", line 1036, in transform
    f_min, f_max = spl(xmin), spl(xmax)
  File "/home/fbruzzesi/open-source/scikit-lego/.venv-pre/lib/python3.10/site-packages/scipy/interpolate/_bsplines.py", line 523, in __call__
    _dierckx.evaluate_spline(self.t, cc.reshape(cc.shape[0], -1),
ValueError: Expected a 1-dim C contiguous array  of dtype = 12( got 12 )

Versions

System:
    python: 3.10.12 (main, Nov  6 2024, 20:22:13) [GCC 11.4.0]
   machine: Linux-5.15.133.1-microsoft-standard-WSL2-x86_64-with-glibc2.35

Python dependencies:
      sklearn: 1.6.0
          pip: 24.1
   setuptools: None
        numpy: 2.2.0
        scipy: 1.15.0rc1
       Cython: None
       pandas: 2.2.3
   matplotlib: None
       joblib: 1.4.2
threadpoolctl: 3.5.0

Built with OpenMP: True

threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 12
         prefix: libscipy_openblas
       filepath: /home/fbruzzesi/open-source/scikit-lego/.venv-pre/lib/python3.10/site-packages/numpy.libs/libscipy_openblas64_-6bb31eeb.so
        version: 0.3.28
threading_layer: pthreads
   architecture: Haswell

       user_api: blas
   internal_api: openblas
    num_threads: 12
         prefix: libscipy_openblas
       filepath: /home/fbruzzesi/open-source/scikit-lego/.venv-pre/lib/python3.10/site-packages/scipy.libs/libscipy_openblas-68440149.so
        version: 0.3.28
threading_layer: pthreads
   architecture: Haswell

       user_api: openmp
   internal_api: openmp
    num_threads: 12
         prefix: libgomp
       filepath: /home/fbruzzesi/open-source/scikit-lego/.venv-pre/lib/python3.10/site-packages/scikit_learn.libs/libgomp-a34b3233.so.1.0.0
        version: None
@FBruzzesi FBruzzesi added Bug Needs Triage Issue requires triage labels Dec 19, 2024
@lesteve
Copy link
Member
lesteve commented Dec 20, 2024

Hmmm no suggestion off the top of my head, my wild guess would be that it is a scipy regression when using memmap. If that's indeed the case it would be very useful to report it to scipy before they do their 1.15 release.

@FBruzzesi
Copy link
Author
FBruzzesi commented Dec 20, 2024

Thanks @lesteve , I will try to make a minimal reproducible example for them and report the issue before the release.

Edit: scipy#22143 to track its status

@lesteve
Copy link
Member
lesteve commented Dec 20, 2024

Nice, thanks a lot!

Side-comment: I would have expected that we run this as part of our CI. Since we run against all development versions of our dependencies including scipy we should have caught this a while ago, this is quite surprising.

@lesteve
Copy link
Member
lesteve commented Dec 20, 2024

It looks like we are xfailing the relevant test:

pytest sklearn/tests/test_common.py -k 'check_estimators_pickle and Spline' -ra

At the end of the output:

XFAIL sklearn/tests/test_common.py::test_estimators[SplineTransformer()-check_estimators_pickle(readonly_memmap=True)] - Current Scipy implementation of _bsplines does notsupport const memory views.
XPASS sklearn/tests/test_common.py::test_estimators[SplineTransformer()-check_estimators_pickle] - Current Scipy implementation of _bsplines does notsupport const memory views.

So 🤔 looks like it did not work originally in scipy #25624 and then was fixed in scipy scipy/scipy#18153 but we never reenabled the test. And now scipy 1.15dev breaks it again.

What a wonderful world we live in 😅

@lesteve
Copy link
Member
lesteve commented Dec 20, 2024

It looks like we are xfailing the relevant test:

I opened #30515 to un-xfail the SplineTransformer check_estimators_pickle common test

@FBruzzesi
Copy link
Author
FBruzzesi commented Dec 20, 2024

Thanks @lesteve , you saved the day both here and in the scipy discussion! Big kudos! Feel free to close this issue whenever you feel right. Hopefully the fix will happen in scipy before the release 🤞🏼

@lesteve
Copy link
Member
lesteve commented Dec 20, 2024

Thanks for the report and helping with putting together a reproducer!

@lesteve
Copy link
Member
lesteve commented Jan 4, 2025

Closed by #30515, thanks for the report and the help @FBruzzesi!

That allowed to pick up a scipy small bug before the scipy 1.15 release and improve our CI by enabling the test again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants
0