Skip to content

BUG: NumPy main breaking SciPy main build on Mac ARM64 (bisected on f2py change) #25286

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.

Sign up for GitHub

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

Jump to bottom
Closed
tylerjereddy opened this issue Nov 30, 2023 · 6 comments · Fixed by #25287 · May be fixed by scipy/scipy#19615
Closed

BUG: NumPy main breaking SciPy main build on Mac ARM64 (bisected on f2py change) #25286

tylerjereddy opened this issue Nov 30, 2023 · 6 comments · Fixed by #25287 · May be fixed by scipy/scipy#19615
Labels
00 - Bug component: numpy.f2py

Comments

@tylerjereddy
Copy link
Contributor

@HaoZeke sorry if this is duplicated somewhere, feel free to close if so. There's a lot of f2py movement I think, I bisected a failure of SciPy main to build on ARM MacOS (M2 max) to this very recent commit:

be0589e36a902859eb350fb10edb43879afd6150 is the first bad commit
commit be0589e36a902859eb350fb10edb43879afd6150
Author: Rohit Goswami <rog32@hi.is>
Date:   Thu Nov 30 00:59:53 2023 +0000

    BUG: Fix module name bug in signature files [urgent] [f2py] (#25267)
    
    * TST: Add one for gh-25263
    
    Co-authored-by: jmrohwer <jmrohwer@users.noreply.github.com>
    
    * BUG: Handle modules correctly for F77
    
    Closes gh-25263
    
    ---------
    
    Co-authored-by: jmrohwer <jmrohwer@users.noreply.github.com>

 numpy/f2py/f2py2e.py            | 14 +++++++++++---
 numpy/f2py/tests/test_f2py2e.py | 16 ++++++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)
@tylerjereddy
Copy link
Contributor Author

I used gfortran 12.1.0, in case that matters.

@HaoZeke
Copy link
Member

HaoZeke commented Nov 30, 2023

Thanks for the report @tylerjereddy, could you share the exact error? That one shouldn't have affected existing bindings at all..

@tylerjereddy
Copy link
Contributor Author

I've pasted it below the fold:

[368/1606] Compiling C object scipy/linalg/_flapack.cpython-311-darwin.so.p/meson-generated_..__flapackmodule.c.o
FAILED: scipy/linalg/_flapack.cpython-311-darwin.so.p/meson-generated_..__flapackmodule.c.o 
cc -Iscipy/linalg/_flapack.cpython-311-darwin.so.p -Iscipy/linalg -I../scipy/linalg -I../../../python_venvs/py_311_numpy_dev/lib/python3.11/site-packages/numpy/_core/include -I../../../python_venvs/py_311_numpy_dev/lib/python3.11/site-packages/numpy/f2py/src -I/opt/homebrew/Cellar/openblas/0.3.24/include -I/Library/Frameworks/Python.framework/Versions/3.11/include/python3.11 -fvisibility=hidden -fcolor-diagnostics -Wall -Winvalid-pch -std=c99 -O2 -g -Wno-unused-but-set-variable -Wno-unused-function -Wno-conversion -Wno-misleading-indentation -DNPY_NO_DEPRECATED_API=NPY_1_9_API_VERSION -Wno-empty-body -MD -MQ scipy/linalg/_flapack.cpython-311-darwin.so.p/meson-generated_..__flapackmodule.c.o -MF scipy/linalg/_flapack.cpython-311-darwin.so.p/meson-generated_..__flapackmodule.c.o.d -o scipy/linalg/_flapack.cpython-311-darwin.so.p/meson-generated_..__flapackmodule.c.o -c scipy/linalg/_flapackmodule.c
scipy/linalg/_flapackmodule.c:14001:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T',"*trans=='N'||*trans=='T'","1st keyword trans","sgels:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14001:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T',"*trans=='N'||*trans=='T'","1st keyword trans","sgels:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14188:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T',"*trans=='N'||*trans=='T'","1st keyword trans","dgels:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14188:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T',"*trans=='N'||*trans=='T'","1st keyword trans","dgels:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14375:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='C',"*trans=='N'||*trans=='C'","1st keyword trans","cgels:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14375:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='C',"*trans=='N'||*trans=='C'","1st keyword trans","cgels:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14562:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='C',"*trans=='N'||*trans=='C'","1st keyword trans","zgels:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14562:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='C',"*trans=='N'||*trans=='C'","1st keyword trans","zgels:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14728:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T',"*trans=='N'||*trans=='T'","1st keyword trans","sgels_lwork:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14728:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T',"*trans=='N'||*trans=='T'","1st keyword trans","sgels_lwork:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14858:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T',"*trans=='N'||*trans=='T'","1st keyword trans","dgels_lwork:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14858:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T',"*trans=='N'||*trans=='T'","1st keyword trans","dgels_lwork:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14989:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='C',"*trans=='N'||*trans=='C'","1st keyword trans","cgels_lwork:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:14989:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='C',"*trans=='N'||*trans=='C'","1st keyword trans","cgels_lwork:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:15121:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='C',"*trans=='N'||*trans=='C'","1st keyword trans","zgels_lwork:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:15121:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='C',"*trans=='N'||*trans=='C'","1st keyword trans","zgels_lwork:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:33656:17: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T'||*trans=='C',"*trans=='N'||*trans=='T'||*trans=='C'","1st keyword trans","sgttrs:trans='%c'",trans) {
                ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:33656:30: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T'||*trans=='C',"*trans=='N'||*trans=='T'||*trans=='C'","1st keyword trans","sgttrs:trans='%c'",trans) {
                             ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
scipy/linalg/_flapackmodule.c:33656:43: error: indirection requires pointer operand ('int' invalid)
    CHECKSCALAR(*trans=='N'||*trans=='T'||*trans=='C',"*trans=='N'||*trans=='T'||*trans=='C'","1st keyword trans","sgttrs:trans='%c'",trans) {
                                          ^~~~~~
scipy/linalg/_flapackmodule.c:148:11: note: expanded from macro 'CHECKSCALAR'
    if (!(check)) {\
          ^~~~~
fatal error: too many errors emitted, stopping now [-ferror-limit=]

@HaoZeke
Copy link
Member

HaoZeke commented Nov 30, 2023 •
edited
Loading

Thanks a ton. Can reproduce locally on Linux as well. Should have a fix ASAP.

EDIT: This is a weird one, and can't be fixed by a revert of #25267 either, since the behavior was dead wrong since #25181, though it was working in 1.26.1 so that's still a pretty narrow range.

HaoZeke added a commit to HaoZeke/numpy that referenced this issue Nov 30, 2023
@HaoZeke
BUG: Handle .pyf.src again
9240eba 
Closes numpygh-25286
@HaoZeke HaoZeke mentioned this issue Nov 30, 2023
Merged
@HaoZeke
Copy link
Member

HaoZeke commented Dec 1, 2023 •
edited
Loading

Some of these are trivial to fix (bad spacing for the #defines, others less so.)

To debug this:

spin run $SHELL
cd scipy
rm -rf build/scipy/linalg/_flapackmodule.c && python dev.py build

Iteratively commenting things out from the flapack.pyf.src files:

    include 'flapack_user.pyf.src'
!    include 'flapack_gen.pyf.src'
!    include 'flapack_gen_banded.pyf.src'
!    include 'flapack_gen_tri.pyf.src'
!    include 'flapack_sym_herm.pyf.src'
!    include 'flapack_pos_def.pyf.src'
!    include 'flapack_pos_def_tri.pyf.src'
    include 'flapack_other.pyf.src'

And so on.

At-least part of this stems from the character handling changes (#19388), the CHECKSCALAR issues are essentially for things like side in <>larf which is a character (typedef char character) but with CHECKSCALAR tests against side[0] which isn't valid.

These are discussed slightly in #19388 (comment). Need to figure out why there's a regression.

It is rather painful to debug because of the size of the generated bindings (and the complete lack of relevant test coverage here), but am chipping away at it.

@HaoZeke HaoZeke mentioned this issue Dec 1, 2023
Draft
@HaoZeke
Copy link
Member

HaoZeke commented Dec 1, 2023

OK, here is the root cause:

python module _char_handling_test
    interface

    subroutine test_char_input(trans, info)

        callstatement (*f2py_func)(&trans, &info)
        callprotoargument char*, int*

        character, intent(in), check(*trans=='N'||*trans=='T'||*trans=='C') :: trans = 'N'
        integer intent(out) :: info

    end subroutine test_char_input

    end interface

end python module _char_handling_test

With:

subroutine test_char_input(trans, info)
    character, intent(in) :: trans
    integer, intent(out) :: info
    if (trans == 'N') then
        info = 1
    else if (trans == 'T') then
        info = 2
    else if (trans == 'C') then
        info = 3
    else
        info = -1
    end if

end subroutine test_char_input

Which used to work for distutils (e.g. on 1.26.2):

python -c "import _char_handling_test as cht; print(cht.test_char_input('T'))"
debug-capi:Python C/API function _char_handling_test.test_char_input(trans='N')
debug-capi:character trans='N':input,optional,scalar
debug-capi:trans='T'
debug-capi:Checking `trans=='N'||trans=='T'||trans=='C''
debug-capi:int info=:output,hidden,scalar
debug-capi:info=0
debug-capi:Fortran subroutine `test_char_input(&trans,&info)'
debug-capi:trans='T'
debug-capi:info=2
debug-capi:Building return value.
debug-capi:Python C/API function _char_handling_test.test_char_input: successful.
debug-capi:Freeing memory.
2

However, there was a minor bug in meson, a couple actually (the --lower flag was too aggressive). Rather more importantly, on main, even distutils now fails to generate the correct binding:

creating /tmp/tmpejt952p2/tmp
creating /tmp/tmpejt952p2/tmp/tmpejt952p2
creating /tmp/tmpejt952p2/tmp/tmpejt952p2/src.linux-x86_64-3.9
INFO: compile options: '-DNPY_DISABLE_OPTIMIZATION=1 -I/tmp/tmpejt952p2/src.linux-x86_64-3.9 -I/home/rgoswami/Git/Github/ScientificPython/NumPy/numpy/build-install/usr/lib/python3.9/site-packages/numpy/_core/include -I/home/rgoswami/micromamba/envs/numpy-dev/include/python3.9 -c'
INFO: x86_64-conda-linux-gnu-cc: /tmp/tmpejt952p2/src.linux-x86_64-3.9/_char_handling_testmodule.c
INFO: x86_64-conda-linux-gnu-cc: /tmp/tmpejt952p2/src.linux-x86_64-3.9/fortranobject.c
/tmp/tmpejt952p2/src.linux-x86_64-3.9/_char_handling_testmodule.c: In function 'f2py_rout__char_handling_test_test_char_input':
/tmp/tmpejt952p2/src.linux-x86_64-3.9/_char_handling_testmodule.c:242:17: error: invalid type argument of unary '*' (have 'int')
  242 |     CHECKSCALAR(*trans=='N'||*trans=='T'||*trans=='C',"*trans=='N'||*trans=='T'||*trans=='C'","1st keyword trans","test_char_input:trans='%c'",trans) {
      |                 ^~~~~~
/tmp/tmpejt952p2/src.linux-x86_64-3.9/_char_handling_testmodule.c:86:11: note: in definition of macro 'CHECKSCALAR'
   86 |     if (!(check)) {\
      |           ^~~~~
/tmp/tmpejt952p2/src.linux-x86_64-3.9/_char_handling_testmodule.c:242:30: error: invalid type argument of unary '*' (have 'int')
  242 |     CHECKSCALAR(*trans=='N'||*trans=='T'||*trans=='C',"*trans=='N'||*trans=='T'||*trans=='C'","1st keyword trans","test_char_input:trans='%c'",trans) {
      |                              ^~~~~~
/tmp/tmpejt952p2/src.linux-x86_64-3.9/_char_handling_testmodule.c:86:11: note: in definition of macro 'CHECKSCALAR'
   86 |     if (!(check)) {\
      |           ^~~~~
/tmp/tmpejt952p2/src.linux-x86_64-3.9/_char_handling_testmodule.c:242:43: error: invalid type argument of unary '*' (have 'int')
  242 |     CHECKSCALAR(*trans=='N'||*trans=='T'||*trans=='C',"*trans=='N'||*trans=='T'||*trans=='C'","1st keyword trans","test_char_input:trans='%c'",trans) {
      |                                           ^~~~~~
/tmp/tmpejt952p2/src.linux-x86_64-3.9/_char_handling_testmodule.c:86:11: note: in definition of macro 'CHECKSCALAR'
   86 |     if (!(check)) {\
      |           ^~~~~

Which, once fixed, will let scipy build again. Though I still think its a good idea to upgrade the flapack bindings.

charris pushed a commit to charris/numpy that referenced this issue Dec 23, 2023
@HaoZeke @charris
BUG: Handle .pyf.src again
b6df150 
Closes numpygh-25286
@charris charris mentioned this issue Dec 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
00 - Bug component: numpy.f2py
Projects
None yet
Milestone
No milestone
2 participants
@HaoZeke @tylerjereddy