Skip to content

BLD: updates to the Meson build#23740

Merged
mattip merged 2 commits intonumpy:mainfrom
rgommers:meson-updates
May 18, 2023
Merged

BLD: updates to the Meson build#23740
mattip merged 2 commits intonumpy:mainfrom
rgommers:meson-updates

Conversation

@rgommers
Copy link
Member

@rgommers rgommers commented May 9, 2023

Mostly synced from SciPy, and also addresses some warnings that show up with Meson 1.1.0

@rgommers rgommers added 03 - Maintenance Meson Items related to the introduction of Meson as the new build system for NumPy labels May 9, 2023
@github-actions github-actions bot added the 36 - Build Build related PR label May 9, 2023
@mattip
Copy link
Member

mattip commented May 9, 2023

The CI failure is in the meson build on macos_arm64.

@rgommers
Copy link
Member Author

rgommers commented May 10, 2023

Hmm, I tested that locally, but in a conda env. Shouldn't be hard to figure out. The error was:

Found CMake: /opt/homebrew/bin/cmake (3.24.1)
WARNING: CMake Toolchain: Failed to determine CMake compilers state
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)

numpy/meson.build:53:9: ERROR: Dependency "OpenBLAS" not found, tried pkgconfig, framework and cmake

also something wrong with CMake in Homebrew it looks like.

@rgommers
Copy link
Member Author

rgommers commented May 11, 2023

It's good that macos_arm64 failed, because it reminded me about dealing with lapack_lite. It turns out that in this job, OpenBLAS was installed but not found. From another passing CI log from today:

Found CMake: /opt/homebrew/bin/cmake (3.24.1)
WARNING: CMake Toolchain: Failed to determine CMake compilers state
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)

I think we want to keep this unchanged, meaning that if no BLAS/LAPACK library was found, we use the fallback library and continue. We should at least emit a clear warning though, because for many users this continues to be a performance footgun.

@mattip
Copy link
Member

mattip commented May 11, 2023

OpenBLAS was installed but not found

Do you need help debugging this? The wheel builder job

seems to be working

The numpy-1.25.0.dev0+1377.g276cc995c-cp39-cp39-macosx_11_0_arm64.whl built 4 days ago and uploaded to https://anaconda.org/scipy-wheels-nightly/numpy/files does have a 22 MB file named libopenblas64_.0.dylib in the .dylibs file.

@rgommers
Copy link
Member Author

rgommers commented May 11, 2023

Not really, this is pretty routine by now, the problem should be that the .pc file of the OpenBLAS used there is not on pkg-config's search path.

I am bandwidth-constrained though and may not touch this for several days, so if you want to fix it, I won't complain:)

@charris charris closed this May 13, 2023
@charris
Copy link
Member

charris commented May 13, 2023

close/reopen

@charris charris reopened this May 13, 2023
@rgommers rgommers removed the 36 - Build Build related PR label May 18, 2023
mattip
mattip reviewed May 18, 2023
View reviewed changes
meson.build Outdated
endif
endif
if not cy.version().version_compare('>=0.29.34')
error('SciPy requires Cython >= 0.29.34')
Copy link
Member

@mattip mattip May 18, 2023

Choose a reason for hiding this comment

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

Is there a "project_name" macro we could use here instead?

Suggested change
error('SciPy requires Cython >= 0.29.34')
error('NumPy requires Cython >= 0.29.34')

Copy link
Member Author

@rgommers rgommers May 18, 2023

Choose a reason for hiding this comment

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

Good catch, thanks. I'll see if the current CI run passes, then will push this with a few more tweaks.

Is there a "project_name" macro we could use here instead?

Not that I know of - but seems more complexity than it's worth anyway, and makes it harder to grep for the error string, so I'd prefer not to do that.

@rgommers
Copy link
Member Author

rgommers commented May 18, 2023 •
edited
Loading

It looks like we should start moving some more CI jobs over to Meson soon. Also, the automatic wheel build trigger is a massive waste of jobs.

This looks better now, I'll keep it like this and will add a comment that OpenBLAS is not used:

Library m found: YES
Found CMake: /opt/homebrew/bin/cmake (3.24.1)
WARNING: CMake Toolchain: Failed to determine CMake compilers state
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)
Run-time dependency openblas found: NO (tried pkgconfig, framework and cmake)

EDIT and the warning I wanted to see:

numpy/linalg/meson.build:17: WARNING: LAPACK was not found, NumPy is using an unoptimized, naive build from sources!

@rgommers
BLD: updates to the Meson build
941bc5b 
Mostly synced from SciPy, and also addresses some warnings that
show up with Meson 1.1.0

[skip circle] [skip azp]
@rgommers
Copy link
Member Author

rgommers commented May 18, 2023

@andyfaff FYI I make some minor tweaks to cirrus_macosx_arm64.yml. NumPy does not actually require a BLAS/LAPACK, and use of CFLAGS in CI jobs should be avoided, since it tends to mask problems that then will show up for users. With distutils there was no better (easy) way, but Meson has a compiler.get_supported_arguments method so that any needed compile flags can be added in a portable manner.

@rgommers
Copy link
Member Author

rgommers commented May 18, 2023

A fun new error with -Werror in Python 3.11.3 that I cannot reproduce locally:

FAILED: numpy/core/_multiarray_umath.cpython-311-x86_64-linux-gnu.so.p/src_umath_override.c.o 
cc -Inumpy/core/_multiarray_umath.cpython-311-x86_64-linux-gnu.so.p -Inumpy/core -I../numpy/core -Inumpy/core/include -I../numpy/core/include -I../numpy/core/src/common -I../numpy/core/src/multiarray -I../numpy/core/src/npymath -I../numpy/core/src/umath -I/usr/include/x86_64-linux-gnu/openblas-serial/ -I/opt/hostedtoolcache/Python/3.11.3/x64/include/python3.11 -fvisibility=hidden -fdiagnostics-color=always -Wall -Winvalid-pch -Werror -std=c99 -O2 -g -fno-strict-aliasing -DHAVE_CBLAS -fPIC -DNPY_INTERNAL_BUILD -DHAVE_NPY_CONFIG_H -DNPY_DISABLE_OPTIMIZATION -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE=1 -D_LARGEFILE64_SOURCE=1 -MD -MQ numpy/core/_multiarray_umath.cpython-311-x86_64-linux-gnu.so.p/src_umath_override.c.o -MF numpy/core/_multiarray_umath.cpython-311-x86_64-linux-gnu.so.p/src_umath_override.c.o.d -o numpy/core/_multiarray_umath.cpython-311-x86_64-linux-gnu.so.p/src_umath_override.c.o -c ../numpy/core/src/umath/override.c
In file included from /opt/hostedtoolcache/Python/3.11.3/x64/include/python3.11/Python.h:44,
                 from ../numpy/core/include/numpy/npy_3kcompat.h:15,
                 from ../numpy/core/src/common/npy_pycompat.h:4,
                 from ../numpy/core/src/umath/override.c:4:
../numpy/core/src/umath/override.c: In function ‘PyUFunc_CheckOverride’:
/opt/hostedtoolcache/Python/3.11.3/x64/include/python3.11/object.h:538:9: error: ‘override_array_ufunc’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  538 |         _Py_Dealloc(op);
      |         ^~~~~~~~~~~~~~~
../numpy/core/src/umath/override.c:319:19: note: ‘override_array_ufunc’ was declared here
  319 |         PyObject *override_array_ufunc;
      |                   ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

@seberg
Copy link
Member

seberg commented May 18, 2023

I think I saw it before at some point and I don't think the compiler is right, just guessing on the "this may happen" side of things. Anyway, initializing to silence the warning is OK.

@rgommers
Copy link
Member Author

rgommers commented May 18, 2023

I don't think the compiler is right, just guessing on the "this may happen" side of things

Indeed, the code wasn't broken but the compiler cannot know. I think using an uninitialized variable that is then used only within an if-statement controlled by some other not-directly-the-same-pointer-condition should always be avoided.

@rgommers
Copy link
Member Author

rgommers commented May 18, 2023

This is good to go now.

@mattip mattip merged commit bfe5f7a into numpy:main May 18, 2023
@mattip
Copy link
Member

mattip commented May 18, 2023

Thanks @rgommers

@rgommers rgommers deleted the meson-updates branch May 18, 2023 11:31
@seberg seberg mentioned this pull request Aug 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattip mattip mattip left review comments

Assignees

No one assigned

Labels

03 - Maintenance Meson Items related to the introduction of Meson as the new build system for NumPy

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rgommers @mattip @charris @seberg