Skip to content

BLD: revert function() -> #define for 3 npymath functions#22679

Merged
rgommers merged 1 commit intonumpy:mainfrom
mattip:issue22673
Nov 27, 2022
Merged

BLD: revert function() -> #define for 3 npymath functions#22679
rgommers merged 1 commit intonumpy:mainfrom
mattip:issue22673

Conversation

@mattip
Copy link
Copy Markdown
Member

@mattip mattip commented Nov 27, 2022 •
edited by rgommers
Loading

It turns out scipy is cross-compiling for macos-arm64 on macos-x86_64 hardware, and using the numpy macos-x86_64 wheel to get libnpymath.a This causes a linker warning but not an error. At runtime, when using NumPy 1.24, importing scipy.special fails because functions are missing: we changed the external functions to defines when refactoring npymath.

This PR exports the missing functions. Closes #22673. Going forward, SciPy should fix its build, but that is too late for the released wheels.

@mattip mattip added the 36 - Build Build related PR label Nov 27, 2022
@rgommers rgommers added this to the 1.24.0 release milestone Nov 27, 2022
@mattip mattip added the 09 - Backport-Candidate PRs tagged should be backported label Nov 27, 2022
@mattip
Copy link
Copy Markdown
Member Author

mattip commented Nov 27, 2022 •
edited
Loading

@Carreau once the build is finished, there will be new wheel artifacts here. Could you make sure they work with scipy on your arm64 machine?

Edit: sorry, the wheels will be here

@Carreau
Copy link
Copy Markdown
Contributor

Carreau commented Nov 27, 2022

Yep, that works for me.

$ python
Python 3.10.0 | packaged by conda-forge | (default, Nov 20 2021, 02:27:15) [Clang 11.1.0 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from scipy.spatial.distance import euclidean
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/bussonniermatthias/miniforge3/envs/dl/lib/python3.10/site-packages/scipy/spatial/__init__.py", line 111, in <module>
    from ._geometric_slerp import geometric_slerp
  File "/Users/bussonniermatthias/miniforge3/envs/dl/lib/python3.10/site-packages/scipy/spatial/_geometric_slerp.py", line 9, in <module>
    from scipy.spatial.distance import euclidean
  File "/Users/bussonniermatthias/miniforge3/envs/dl/lib/python3.10/site-packages/scipy/spatial/distance.py", line 121, in <module>
    from ..special import rel_entr
  File "/Users/bussonniermatthias/miniforge3/envs/dl/lib/python3.10/site-packages/scipy/special/__init__.py", line 649, in <module>
    from . import _ufuncs
ImportError: dlopen(/Users/bussonniermatthias/miniforge3/envs/dl/lib/python3.10/site-packages/scipy/special/_ufuncs.cpython-310-darwin.so, 0x0002): symbol not found in flat namespace (_npy_asinh)
>>>
$
$ pip install ~/Downloads/cp310-macosx/numpy-1.25.0.dev0+22.gf7217e9bf-cp310-cp310-macosx_11_0_arm64.whl
Processing /Users/bussonniermatthias/Downloads/cp310-macosx/numpy-1.25.0.dev0+22.gf7217e9bf-cp310-cp310-macosx_11_0_arm64.whl
Installing collected packages: numpy
  Attempting uninstall: numpy
    Found existing installation: numpy 1.24.0rc1
    Uninstalling numpy-1.24.0rc1:
      Successfully uninstalled numpy-1.24.0rc1
Successfully installed numpy-1.25.0.dev0+22.gf7217e9bf
$ python
Python 3.10.0 | packaged by conda-forge | (default, Nov 20 2021, 02:27:15) [Clang 11.1.0 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from scipy.spatial.distance import euclidean
>>>

@rgommers rgommers changed the title BUILD: revert function() -> #define for 3 npymath functions BLD: revert function() -> #define for 3 npymath functions Nov 27, 2022
rgommers
rgommers approved these changes Nov 27, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @mattip! And thanks @Carreau for testing.

I also went back and checked older SciPy releases:

  • 1.8.1 works, the other symbols that turned into defines are not used,
  • 1.7.3 has an upper bound so it cannot be used from wheels together with numpy 1.24. And on conda-forge, this issue isn't visible.

So in it goes.

@rgommers rgommers merged commit 57f704a into numpy:main Nov 27, 2022
@charris charris mentioned this pull request Nov 30, 2022
Merged
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Nov 30, 2022
@charris charris removed this from the 1.24.0 release milestone Nov 30, 2022
rgommers added a commit to rgommers/numpy that referenced this pull request Dec 14, 2022
@rgommers
BLD: fix issue in npymath on macOS arm64 in the Meson build
0a6463b 
This was due to a cross-merge conflict. numpygh-22679 added a new file
to the setup.py build, while the Meson build got merged. It's
a file that only does anything on arm64 macOS, hence it wasn't
noticed in CI.

Addresses a "missing `npy_asinh`" problem discussed in numpygh-22796

[skip azp]
[skip circle]
[skip cirrus]
@rgommers rgommers mentioned this pull request Dec 14, 2022
Merged
@mattip mattip deleted the issue22673 branch December 27, 2022 16:51
seberg added a commit to seberg/numpy that referenced this pull request Jan 5, 2023
@seberg
MAINT: Move export for scipy arm64 helper into main module
2b77db3 
This is a follow up to numpygh-22679 which addressed numpygh-22673.

The main thing is that we want the functions to be available after
importing NumPy, so they need to be part of multiarray.
However, `npymath` is a static library, so the symbols are not really
exported there.  The former PR did actually work in practice but this
seems like it is technically the right place?

For some reason, I had to add nextafter to be able to do:

    from scipy.spatial.distance import euclidean

with the SciPy 1.9.3 wheels.  SciPy test collection works with this for
the 1.9.3 wheel, so this should be all the symbols hopefully.
@seberg seberg mentioned this pull request Jan 5, 2023
Merged
seberg added a commit to seberg/numpy that referenced this pull request Jan 5, 2023
@seberg
MAINT: Move export for scipy arm64 helper into main module
d0a613c 
This is a follow up to numpygh-22679 which addressed numpygh-22673.

The main thing is that we want the functions to be available after
importing NumPy, so they need to be part of multiarray.
However, `npymath` is a static library, so the symbols are not really
exported there.  The former PR did actually work in practice but this
seems like it is technically the right place?

For some reason, I had to add nextafter to be able to do:

    from scipy.spatial.distance import euclidean

with the SciPy 1.9.3 wheels.  SciPy test collection works with this for
the 1.9.3 wheel, so this should be all the symbols hopefully.
charris pushed a commit to charris/numpy that referenced this pull request Jan 8, 2023
@seberg @charris
MAINT: Move export for scipy arm64 helper into main module
e337d58 
This is a follow up to numpygh-22679 which addressed numpygh-22673.

The main thing is that we want the functions to be available after
importing NumPy, so they need to be part of multiarray.
However, `npymath` is a static library, so the symbols are not really
exported there.  The former PR did actually work in practice but this
seems like it is technically the right place?

For some reason, I had to add nextafter to be able to do:

    from scipy.spatial.distance import euclidean

with the SciPy 1.9.3 wheels.  SciPy test collection works with this for
the 1.9.3 wheel, so this should be all the symbols hopefully.
charris pushed a commit to charris/numpy that referenced this pull request Jan 8, 2023
@seberg @charris
MAINT: Move export for scipy arm64 helper into main module
a14ce74 
This is a follow up to numpygh-22679 which addressed numpygh-22673.

The main thing is that we want the functions to be available after
importing NumPy, so they need to be part of multiarray.
However, `npymath` is a static library, so the symbols are not really
exported there.  The former PR did actually work in practice but this
seems like it is technically the right place?

For some reason, I had to add nextafter to be able to do:

    from scipy.spatial.distance import euclidean

with the SciPy 1.9.3 wheels.  SciPy test collection works with this for
the 1.9.3 wheel, so this should be all the symbols hopefully.
@charris charris mentioned this pull request Jan 8, 2023
Merged
charris pushed a commit to charris/numpy that referenced this pull request Jan 9, 2023
@seberg @charris
MAINT: Move export for scipy arm64 helper into main module
8d6d4aa 
This is a follow up to numpygh-22679 which addressed numpygh-22673.

The main thing is that we want the functions to be available after
importing NumPy, so they need to be part of multiarray.
However, `npymath` is a static library, so the symbols are not really
exported there.  The former PR did actually work in practice but this
seems like it is technically the right place?

For some reason, I had to add nextafter to be able to do:

    from scipy.spatial.distance import euclidean

with the SciPy 1.9.3 wheels.  SciPy test collection works with this for
the 1.9.3 wheel, so this should be all the symbols hopefully.
@queirozfcom
Copy link
Copy Markdown

queirozfcom commented Dec 11, 2023 •
edited
Loading

Running into this problem again with numpy 1.25.2 on a Mac M1, Python 3.9 via conda

Possible regression?

image

@rgommers
Copy link
Copy Markdown
Member

rgommers commented Dec 11, 2023

Could you please open a new issue @queirozfcom with as much detail about versions as possible? You're using a venv on top of conda it looks like, and not sure where SciPy is coming from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rgommers rgommers rgommers approved these changes

Assignees

No one assigned

Labels

36 - Build Build related PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BUG: scipy.special import broken with numpy 1.24.0rc1

5 participants

@mattip @Carreau @queirozfcom @rgommers @charris