Skip to content
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

[BUG] incompatibility with CPython main branch #4499

Closed
tacaswell opened this issue Dec 16, 2021 · 10 comments
Closed

[BUG] incompatibility with CPython main branch #4499

tacaswell opened this issue Dec 16, 2021 · 10 comments

Comments

@tacaswell
Copy link
Contributor

Describe the bug

When trying to build the scipy main with

✘ 19:01:47 $ cython --version
Cython version 3.0.0a9
(bleeding) ~/s/o/scipy main|✓
✔ 19:03:33 $ python --version
Python 3.11.0a3+

you get the following error:

  In file included from /home/tcaswell/.virtualenvs/bleeding/lib/python3.11/site-packages/numpy/core/include/numpy/ndarraytypes.h:1960,
                   from /home/tcaswell/.virtualenvs/bleeding/lib/python3.11/site-packages/numpy/core/include/numpy/ndarrayobject.h:12,
                   from /home/tcaswell/.virtualenvs/bleeding/lib/python3.11/site-packages/numpy/core/include/numpy/arrayobject.h:5,
                   from scipy/optimize/_highs/cython/src/_highs_wrapper.cxx:871:
  /home/tcaswell/.virtualenvs/bleeding/lib/python3.11/site-packages/numpy/core/include/numpy/npy_1_7_deprecated_api.h:17:2: warning: #warning "Using deprecated NumPy API, disable it with " "#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
     17 | #warning "Using deprecated NumPy API, disable it with " \
        |  ^~~~~~~
  scipy/optimize/_highs/cython/src/_highs_wrapper.cxx:23979:1: error: too many initializers for ‘PyTypeObject’ {aka ‘_typeobject’}
  23979 | };
        | ^
  scipy/optimize/_highs/cython/src/_highs_wrapper.cxx:24131:1: error: too many initializers for ‘PyTypeObject’ {aka ‘_typeobject’}
  24131 | };
        | ^
  scipy/optimize/_highs/cython/src/_highs_wrapper.cxx:24453:1: error: too many initializers for ‘PyTypeObject’ {aka ‘_typeobject’}
  24453 | };
        | ^
  scipy/optimize/_highs/cython/src/_highs_wrapper.cxx:24624:1: error: too many initializers for ‘PyTypeObject’ {aka ‘_typeobject’}
  24624 | };
        | ^
  scipy/optimize/_highs/cython/src/_highs_wrapper.cxx:30583:1: error: too many initializers for ‘PyTypeObject’ {aka ‘_typeobject’}
  30583 | };
        | ^
  error: Command "g++ -pthread -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -DCMAKE_BUILD_TYPE="Release" -DHiGHSRELEASE -DIPX_ON=ON -DHIGHS_GITHASH="n/a" -DHIGHS_COMPILATION_DATE="2021-12-15" -DHIGHS_VERSION_MAJOR=1 -DHIGHS_VERSION_MINOR=0 -DHIGHS_VERSION_PATCH=0 -DHIGHS_DIR="/home/tcaswell/source/other_source/scipy/scipy/optimize/_highs" -DCMAKE_BUILD_TYPE="Release" -DHiGHSRELEASE -DIPX_ON=ON -DHIGHS_GITHASH="n/a" -DHIGHS_COMPILATION_DATE="2021-12-15" -DHIGHS_VERSION_MAJOR=1 -DHIGHS_VERSION_MINOR=0 -DHIGHS_VERSION_PATCH=0 -DHIGHS_DIR="/home/tcaswell/source/other_source/scipy/scipy/optimize/_highs" -UOPENMP -UEXT_PRESOLVE -USCIP_DEV -UHiGHSDEV -UOSI_FOUND -Iscipy/optimize/_highs/src -Iscipy/optimize/_highs/cython/src -Iscipy/optimize/_highs/src/lp_data -Iscipy/optimize/_highs/src/io -Iscipy/optimize/_highs/src/ipm/ipx/include -Iscipy/optimize/_highs/src/ipm/ipx/include -Iscipy/optimize/_highs/src/ipm/basiclu/include -I/home/tcaswell/.virtualenvs/bleeding/lib/python3.11/site-packages/numpy/core/include -Ibuild/src.linux-x86_64-3.11/numpy/distutils/include -I/home/tcaswell/.virtualenvs/bleeding/include -I/home/tcaswell/.pybuild/bleeding/include/python3.11 -c scipy/optimize/_highs/cython/src/_highs_wrapper.cxx -o build/temp.linux-x86_64-3.11/scipy/optimize/_highs/cython/src/_highs_wrapper.o -MMD -MF build/temp.linux-x86_64-3.11/scipy/optimize/_highs/cython/src/_highs_wrapper.o.d -std=c++14 -msse -msse2 -msse3" failed with exit status 1

The generate c++ that causes this problem is:

static PyTypeObject __pyx_type___pyx_memoryview = {
  PyVarObject_HEAD_INIT(0, 0)
  "scipy.optimize._highs.cython.src._highs_wrapper.""memoryview", /*tp_name*/
  sizeof(struct __pyx_memoryview_obj), /*tp_basicsize*/
  0, /*tp_itemsize*/
  __pyx_tp_dealloc_memoryview, /*tp_dealloc*/
  #if PY_VERSION_HEX < 0x030800b4
  0, /*tp_print*/
  #endif
  #if PY_VERSION_HEX >= 0x030800b4
  0, /*tp_vectorcall_offset*/
  #endif
  0, /*tp_getattr*/
  0, /*tp_setattr*/
  #if PY_MAJOR_VERSION < 3
  0, /*tp_compare*/
  #endif
  #if PY_MAJOR_VERSION >= 3
  0, /*tp_as_async*/
  #endif
  __pyx_memoryview___repr__, /*tp_repr*/
  0, /*tp_as_number*/
  &__pyx_tp_as_sequence_memoryview, /*tp_as_sequence*/
  &__pyx_tp_as_mapping_memoryview, /*tp_as_mapping*/
  0, /*tp_hash*/
  0, /*tp_call*/
  __pyx_memoryview___str__, /*tp_str*/
  0, /*tp_getattro*/
  0, /*tp_setattro*/
  &__pyx_tp_as_buffer_memoryview, /*tp_as_buffer*/
  Py_TPFLAGS_DEFAULT|Py_TPFLAGS_HAVE_VERSION_TAG|Py_TPFLAGS_CHECKTYPES|Py_TPFLAGS_HAVE_NEWBUFFER|Py_TPFLAGS_BASETYPE|Py_TPFLAGS_HAVE_GC, /*tp_flags*/
  0, /*tp_doc*/
  __pyx_tp_traverse_memoryview, /*tp_traverse*/
  __pyx_tp_clear_memoryview, /*tp_clear*/
  0, /*tp_richcompare*/
  0, /*tp_weaklistoffset*/
  0, /*tp_iter*/
  0, /*tp_iternext*/
  __pyx_methods_memoryview, /*tp_methods*/
  0, /*tp_members*/
  __pyx_getsets_memoryview, /*tp_getset*/
  0, /*tp_base*/
  0, /*tp_dict*/
  0, /*tp_descr_get*/
  0, /*tp_descr_set*/
  #if !CYTHON_USE_TYPE_SPECS
  0, /*tp_dictoffset*/
  #endif
  0, /*tp_init*/
  0, /*tp_alloc*/
  __pyx_tp_new_memoryview, /*tp_new*/
  0, /*tp_free*/
  0, /*tp_is_gc*/
  0, /*tp_bases*/
  0, /*tp_mro*/
  0, /*tp_cache*/
  0, /*tp_subclasses*/
  0, /*tp_weaklist*/
  0, /*tp_del*/
  0, /*tp_version_tag*/
  #if PY_VERSION_HEX >= 0x030400a1
  0, /*tp_finalize*/
  #endif
  #if PY_VERSION_HEX >= 0x030800b1
  0, /*tp_vectorcall*/
  #endif
  #if PY_VERSION_HEX >= 0x030800b4 && PY_VERSION_HEX < 0x03090000
  0, /*tp_print*/
  #endif
  #if PY_VERSION_HEX >= 0x030B00A2
  0, /*tp_inline_values_offset*/
  #endif
  #if CYTHON_COMPILING_IN_PYPY && PYPY_VERSION_NUM+0 >= 0x06000000
  0, /*tp_pypy_flags*/
  #endif
};
#endif

To Reproduce
Code to reproduce the behaviour:

Try to install the development branch of scipy with the development branch of CPython, numpy and cython.

Expected behavior
A clear and concise description of what you expected to happen.

Clean build.

Environment (please complete the following information):

  • OS: [e.g. Linux, Windows, macOS] Linux (arch)
  • Python version [e.g. 3.8.4]: main branch ~2021-12-14
  • Cython version [e.g. 0.29.18] : master branch ~2021-12-14

I have not bisected this yet, but plan to.

@tacaswell
Copy link
Contributor Author

python/cpython@8319114 / #4499 is the commit in CPython that I bisected this issue back.

@scoder
Copy link
Contributor

scoder commented Dec 16, 2021 via email

@da-woods
Copy link
Contributor

So it's f5aa00b to revert on 0.29.x and 751532a to revert on master.

It looks like this change was in Python 3.11.0a3 but GitHub actions hasn't yet got that version

@scoder
Copy link
Contributor

scoder commented Dec 16, 2021

I reverted the two commits. It's ok to live with the C compiler warning for now.

@tacaswell
Copy link
Contributor Author

Searching for python/cpython@8319114 in the CPython repo you also get a hit on python/cpython#30122 (I think only because in one of the comments they link to code snippets targeting this merge commit).

However, why trying to sort out why that PR came up in search, I noted a commit message

"add to what's new, because this change breaks things like cython"

so I suspect more changes are coming.

@TeamSpen210
Copy link
Contributor

That change is the simplification of the exc_info data - the type and traceback is now just derived from the instance, and if they need to be unnormalised it's set to a tuple.

@da-woods
Copy link
Contributor

FWIW I did a quick check and both Cython master and 0.29.x can build themselves on Python 3.11.0a3. Doesn't guarantee that there aren't any bugs of course (for that I think we have to wait until its added to GitHub actions) but it gives some reassurance.

I don't think it's realistic for us to try to support the in progress versions between alpha releases though

@scoder
Copy link
Contributor

scoder commented Dec 17, 2021

python/cpython#30122

That's a nice improvement, and if it stays in CPython, then we should generate proper code for it. Most of the three-exc-fields access that we do deals with curexc, which does not change (apparently). But we also have a couple of places where we touch the three fields in exc_info. Let's wait a bit for this to settle and then be prepared to adapt. 3.11b1 is still half a year away. (And post-beta rollbacks are not unheard of.)

@iritkatriel
Copy link

That change is the simplification of the exc_info data - the type and traceback is now just derived from the instance, and if they need to be unnormalised it's set to a tuple.

That was the original thought, but then it turned out that exc_info is actually always normalised so the type and traceback field are redundant. The raised exception curexc may not be normalized, so we are leaving it as it is with 3 values, but the caught exception (exc_info) is normalized.

I had a look at the cython code a few weeks ago, and it seems that all you do with exc_info is to copy it over in a few places. It should be easy to update that, just put the copy of the type and traceback fields under #if older version. I can try to help (but I need to get my cython test setup working first).

@iritkatriel
Copy link

Sorry I only saw #4500 after I wrote the previous comment. You're clearly on top of things.

@scoder scoder closed this as completed Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants