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.

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

MAINT: Move export for scipy arm64 helper into main module #22972

Merged
merged 2 commits into from Jan 9, 2023
Merged

MAINT: Move export for scipy arm64 helper into main module #22972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d6d4aa
MAINT: Move export for scipy arm64 helper into main module
seberg Jan 5, 2023
118a01a
restore declaration of npy_nextafter
mattip Jan 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 4 additions & 10 deletions numpy/core/include/numpy/npy_math.h
View file
Open in desktop
Expand Up @@ -184,29 +184,23 @@ NPY_INPLACE double npy_atan2(double x, double y);
#define npy_fmod fmod
#define npy_floor floor
#define npy_expm1 expm1
#define npy_log1p log1p
#define npy_acosh acosh
#define npy_asinh asinh
#define npy_atanh atanh
#define npy_rint rint
#define npy_trunc trunc
#define npy_exp2 exp2
#define npy_frexp frexp
#define npy_ldexp ldexp
#define npy_copysign copysign
#define npy_exp exp
#define npy_sqrt sqrt
#define npy_pow pow
#define npy_modf modf

#if defined(__arm64__) && defined(__APPLE__)
/* due to a build problem with scipy, export these as functions */
NPY_INPLACE double npy_asinh(double x);
NPY_INPLACE double npy_copysign(double y, double x);
NPY_INPLACE double npy_log1p(double x);
#else
#define npy_asinh asinh
#define npy_copysign copysign
#define npy_log1p log1p
#endif
double npy_nextafter(double x, double y);
Copy link
Member

Choose a reason for hiding this comment

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

I restored this line which should not be removed

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thanks. So that is why I had to add nextafter, but it was previously no issue. I guess it could be excluded from the other file in that case, but seems it doesn't matter?

Copy link
Member

Choose a reason for hiding this comment

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

It needs to be in both files for this branch. One is compiled into libnpy_math.a, this is the one that is removed from main but on this branch it is alive and well. The other implementation is exported from the main numpy namespace because of <mumble>macos arm64 build linking problem</mumble>

Copy link
Member Author

@charris charris Feb 4, 2023 •
edited

Choose a reason for hiding this comment

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

@seberg @mattip

This seems to be causing problems with the macosx m1 wheels, the linker complains of two symbols _npy_nextafter, which I suspect is because of long double being double on the m1 platform. This doesn't seem to be a problem in main.

EDIT: This is when building wheels for 1.24.2.

Copy link
Member

Choose a reason for hiding this comment

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

Arrgg, I guess that because npy_nextafter is already here, it should not have been included in arm64_exports.c for the backport...


double npy_spacing(double x);

/*
Expand Down
8 changes: 4 additions & 4 deletions numpy/core/setup.py
View file
Open in desktop
Expand Up @@ -771,10 +771,6 @@ def get_mathlib_info(*args):
join('src', 'npymath', 'ieee754.c.src'),
join('src', 'npymath', 'npy_math_complex.c.src'),
join('src', 'npymath', 'halffloat.c'),
# Remove this once scipy macos arm64 build correctly
# links to the arm64 npymath library,
# see gh-22673
join('src', 'npymath', 'arm64_exports.c'),
]

config.add_installed_library('npymath',
Expand Down Expand Up @@ -1006,6 +1002,10 @@ def get_mathlib_info(*args):
join('src', 'multiarray', 'textreading', 'stream_pyobject.c'),
join('src', 'multiarray', 'textreading', 'str_to_int.c'),
join('src', 'multiarray', 'textreading', 'tokenize.cpp'),
# Remove this once scipy macos arm64 build correctly
# links to the arm64 npymath library,
# see gh-22673
join('src', 'npymath', 'arm64_exports.c'),
]

#######################################################################
Expand Down
9 changes: 8 additions & 1 deletion numpy/core/src/npymath/arm64_exports.c
View file
Open in desktop
@@ -1,12 +1,14 @@
#if defined(__arm64__) && defined(__APPLE__)

#include <math.h>
/*
/*
* Export these for scipy, since the SciPy build for macos arm64
* downloads the macos x86_64 NumPy, and does not error when the
* linker fails to use npymathlib.a. Importing numpy will expose
* these external functions
* See https://github.com/numpy/numpy/issues/22673#issuecomment-1327520055
*
* This file is actually compiled as part of the main module.
*/

double npy_asinh(double x) {
Expand All @@ -20,4 +22,9 @@ double npy_copysign(double y, double x) {
double npy_log1p(double x) {
return log1p(x);
}

double npy_nextafter(double x, double y) {
return nextafter(x, y);
}

#endif