Skip to content
This repository has been archived by the owner on Aug 30, 2024. It is now read-only.

build universal2 wheel #104

Merged
merged 21 commits into from
Feb 28, 2021
Merged

build universal2 wheel #104

merged 21 commits into from
Feb 28, 2021

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Feb 5, 2021

No description provided.

@isuruf
Copy link
Contributor Author

isuruf commented Feb 5, 2021

@mattip, do you know why Azure is not running?

@mattip
Copy link
Contributor

mattip commented Feb 6, 2021

Strange. The changes look fine to me, and the pipeline ran on master via the cron job last week.

@mattip
Copy link
Contributor

mattip commented Feb 6, 2021

Ahh, you are targeting the 1.20.x branch, not master. Can you retarget?

@isuruf isuruf changed the base branch from v1.20.x to master February 6, 2021 17:46
@mattip
Copy link
Contributor

mattip commented Feb 6, 2021

The new build errors with

clang: error: invalid version number in 'MACOSX_DEPLOYMENT_TARGET=11.0'

@henryiii
Copy link

henryiii commented Feb 8, 2021

Feel free to check out the work in pypa/cibuildwheel#484 - the tricks there to enable building Universal2 wheels on macOS 10.15 should be useful here.

@henryiii
Copy link

henryiii commented Feb 8, 2021

You do want the x86_64 version MACOSX_DEPLOYMENT_TARGET (10.9, or so?); the ARM part will use 11 automatically since it's the earliest. cibuildwheel/macos.py file changes are the ones you'll want to look at.

@mattip
Copy link
Contributor

mattip commented Feb 14, 2021

Something is off with the OpenBLAS linking. Did the correct OpenBLAS build get downloaded?:

lang -arch arm64 /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/tmpifekyq5s/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/tmpifekyq5s/source.o \ 
    -L/usr/local/lib -lopenblas -o /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/tmpifekyq5s/a.out
ld: warning: directory not found for option '-L/opt/arm64-builds/lib'
ld: warning: ignoring file /usr/local/lib/libopenblas.dylib, building for macOS-arm64 but \
    attempting to link with file built for macOS-x86_64
ld: warning: ignoring file /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/lib/libSystem.tbd, \
    missing required architecture arm64 in file /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/lib/libSystem.tbd
ld: dynamic main executables must link with libSystem.dylib for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@isuruf
Copy link
Contributor Author

isuruf commented Feb 14, 2021

@mattip
Copy link
Contributor

mattip commented Feb 15, 2021

Windows doesn't like this. master is passing on azure on numpy/numpy, I wonder what is different?

    with pytest.raises(np.ComplexWarning):
>       np.einsum("i,i->i", AR_LIKE_U, AR_LIKE_U, dtype=float, casting="unsafe", out=OUT_c)
E       Failed: DID NOT RAISE <class 'numpy.ComplexWarning'>

@mattip
Copy link
Contributor

mattip commented Feb 15, 2021

The weekly build job failed on windows as well.

@mattip
Copy link
Contributor

mattip commented Feb 15, 2021

These failing windows jobs report

NumPy version 1.21.0.dev0+778.gba4eab868
NumPy relaxed strides checking option: True
NumPy CPU features: SSE SSE2 SSE3 SSSE3* SSE41* POPCNT* SSE42* AVX* F16C* \
    FMA3* AVX2* AVX512F* AVX512CD* AVX512_SKX* AVX512_CLX? AVX512_CNL?

The successful windows jobs report

NumPy version 1.21.0.dev0+780.g58095d94c
NumPy relaxed strides checking option: True
NumPy CPU features: SSE SSE2 SSE3 SSSE3* SSE41* POPCNT* SSE42* AVX* F16C* \
    FMA3* AVX2* AVX512F? AVX512CD? AVX512_SKX? AVX512_CLX? AVX512_CNL?

@seberg
Copy link

seberg commented Feb 15, 2021

That build is also spitting out a lot of warnings about promoting strings, which should have caused failures on master CI.

I am a bit disappointed, because I thought warnings being swallowed was something that pytest handled fine, but this feels a bit like the old "once a warning is ignored it will never be shown again, even if warning filters change, oops!" (which python fixed in general, but...?)

@seberg
Copy link

seberg commented Feb 15, 2021

Hmmm, could it be that these builds do not have a pytest plugin that all others builds do? And that pytest plugin adds a global "ignore all warnings" or so?

EDIT: Although, I don't think that should explain the problematic einsum one, just why CI is not barfing on master due to the string-promotion warning.

@mattip
Copy link
Contributor

mattip commented Feb 15, 2021

FWIW: Failed runs have

============================= test session starts =============================
platform win32 -- Python 3.7.9, pytest-6.2.2, py-1.10.0, pluggy-0.13.1 -- D:\a\1\s\test_venv\Scripts\python.exe
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('D:\\a\\1\\s\\for_test\\.hypothesis\\examples')
rootdir: D:\a\1\s\for_test
plugins: hypothesis-6.2.0, cov-2.11.1
collecting ... collected 15531 items

Successful runs do not report their plugins since they run via python runtests.py

@seberg
Copy link

seberg commented Feb 15, 2021

I don't know... I am trying to run test_regressions.py which should report warnings (the promotion ones), but nothing happens. I tried getting the same pluggy and pytest versions. That did not help. I also tried PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 which does not make a difference.
Then, I tried adding -rw: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python3.9 runtests.py -t numpy/core/tests/test_regression.py -v -- -rw to make double sure that warnings should be reported. This also did not help (I also deleted the numpy/conftest.py for sport...).

Even if there is a pytest bug, I don't really understand why things would differ on window. I can add warning capture for the other string warnings anyway.

But, all of that might be unrelated to the complex warning. Although, complex warnings should not really have anything platform specific!

@seberg
Copy link

seberg commented Feb 15, 2021

By now I have deleted down test_regressions to the following:

import pytest
import numpy as np


def test_warns():
    with pytest.warns(FutureWarning):
        np.array([1, "a"])

def test_ooops():
    np.array([1, "a"])

and it still doesn't report a warning (testing that outside NumPy it does report the warning). The other test succeeds though.

@seberg
Copy link

seberg commented Feb 15, 2021

Hmmm, maybe one reason why the promotion warnings are weird here, is that when they are raised as an error that unfortunately "opts-in" to the future behaviour and succeeds without any error. That may be something we should work around (i.e. do not ignore all errors during promotion, but only typeerrors).

In which case: I really think that is unrelated to the complex warning problem, probably. Sorry for the noise.

@isuruf
Copy link
Contributor Author

isuruf commented Feb 25, 2021

Anybody know why numpy doesn't respect LDFLAGS when looking for openblas?

@isuruf
Copy link
Contributor Author

isuruf commented Feb 25, 2021

@mattip, @rgommers, @matthew-brett, this is ready for a review

@mattip
Copy link
Contributor

mattip commented Feb 26, 2021

The windows warnings are still there. I opened numpy/numpy#18496.

I guess we have no real way of testing the generated wheel locally, so we should put it up on https://anaconda.org/scipy-wheels-nightly/. I would like to prevent uploading to the more public release staging area at https://anaconda.org/multibuild-wheels-staging until we are a bit more confident that this works. Could we add an "if" clause to the upload to prevent that? Or am I being overly cautious, since it would still require manual intervention to get the wheels from https://anaconda.org/multibuild-wheels-staging to PyPI?

@@ -81,7 +81,7 @@ after_success:
ANACONDA_ORG="multibuild-wheels-staging";
TOKEN=${NUMPY_STAGING_UPLOAD_TOKEN};
fi
- pip install git+https://github.com/Anaconda-Server/anaconda-client;
- pip install git+https://github.com/Anaconda-Server/anaconda-client[email protected];
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@rgommers
Copy link
Contributor

The windows warnings are still there. I opened numpy/numpy#18496.

I guess we have no real way of testing the generated wheel locally, so we should put it up on https://anaconda.org/scipy-wheels-nightly/. I would like to prevent uploading to the more public release staging area at https://anaconda.org/multibuild-wheels-staging until we are a bit more confident that this works. Could we add an "if" clause to the upload to prevent that? Or am I being overly cautious, since it would still require manual intervention to get the wheels from https://anaconda.org/multibuild-wheels-staging to PyPI?

Yes, I'd say that's a little overcautious:) It shouldn't matter, and if it did it's not clear which way since we ask people to use the nightly bucket for testing NumPy master in their own CI.

I suggest to just do whatever is easiest here.

@isuruf
Copy link
Contributor Author

isuruf commented Feb 26, 2021

Merge?

Copy link
Contributor

@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.

Changes LGTM, thanks @isuruf. I haven't followed this PR in detail, so I'll leave the merge to @mattip.

@mattip mattip merged commit cfab7ac into MacPython:master Feb 28, 2021
@mattip
Copy link
Contributor

mattip commented Feb 28, 2021

Thanks @isuruf

judahrand added a commit to judahrand/scipy-wheels that referenced this pull request Jun 28, 2021
Changes ported from `numpy-wheels`  found at:
- MacPython/numpy-wheels#104
- MacPython/numpy-wheels#115
rgommers added a commit to MacPython/scipy-wheels that referenced this pull request Aug 23, 2021
* Enable universal2 wheels

Changes ported from `numpy-wheels`  found at:
- MacPython/numpy-wheels#104
- MacPython/numpy-wheels#115

* Add missing call to `wrap_wheel_builder`

* Add `install_delocate`

Ported from:
- MacPython/numpy-wheels#109
- MacPython/numpy-wheels#110

* Trigger rebuild now that the needed SciPy PR is merged

* use mingw-w64 gcc=7

* update multibuild

* cleanup config.sh

* Set F77 and F90 too

* Set MB_ML_VER

* run build_libs for osx

* gcc 7 for i686

* set _ISOC99_SOURCE

* cleanup

* Need fPIC for Linux

* fPIC for all platforms

* keep using MB_ML_VER=1 until we drop manylinux1

Co-authored-by: Judah Rand <[email protected]>
Co-authored-by: Ralf Gommers <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants