MNT Use cibuildwheel to build the wheels by alfaro96 · Pull Request #17921 · scikit-learn/scikit-learn

alfaro96 · 2020-07-14T09:12:20Z

Reference Issues/PRs

Closes MacPython/scikit-learn-wheels#27.
Closes MacPython/scikit-learn-wheels#61.

What does this implement/fix? Explain your changes.

This PR moves the scikit-learn wheel builder to the scikit-learn organization using the cibuildwheel package.

adrinjalali

Thanks @alfaro96 ! Are we going to also upload them somewhere from this same script?

.github/workflows/wheels.yml

alfaro96 · 2020-07-14T09:58:47Z

Thanks @alfaro96 ! Are we going to also upload them somewhere from this same script?

Thank you @adrinjalali for the quickness!

The idea is to add a step to upload the wheels and source distribution to the Anaconda repositories (scipy-wheels-nightly and scikit-learn-wheels-staging).

WDYT?

rth

Thanks for looking into this @alfaro96 !

.github/workflows/wheels.yml

rth

Didn't mean to approve this PR, and can't find the 'unapprove' button now :)

Edit: there used to be a "dismiss review" button before, but I can't find it anymore..

alfaro96 · 2020-07-14T14:38:11Z

Some tests are not passing while testing the wheels. I will investigate this issue and let you know why this is happening.

Nevertheless, it is pretty straightforward to build the wheels with cibuildwheel.

alfaro96 · 2020-07-14T14:52:42Z

It seems we are running into memory issues. For instance:

test_common_check_return_X_y[fetch_covtype-fetch_covtype]: MemoryError: Unable to allocate 244. MiB for an array with shape (581012, 55) and data type float64.
test_svc_ovr_tie_breaking[SVC]: MemoryError: Unable to allocate 22.9 MiB for an array with shape (1000000, 3) and data type float64.

alfaro96 · 2020-11-05T11:09:05Z

Hey @ogrisel,

I have finally found the right way to go (and, IMHO, the best and cleanest solution).

The cibuildwheel package provides the CIBW_REPAIR_WHEEL_COMMAND_WINDOWS environment variable to repair a Windows wheel (by default, not repaired). However, I have realized that, in our case, it would be interesting to use this command to embed the vcomp140.dll. In particular, I have used the unpack + vendor_vcomp140.py + pack sequence to repair the Windows wheel.

It is important to note that the command is run on each built wheel before testing it (between the build and install + test steps).

Let us wait for green in the CD, but I think that this solution is gonna work pretty well.

Thank you @ogrisel for your valuable comments!

alfaro96 · 2020-11-05T11:16:13Z

Moreover, I have added the wheels for Python 3.9.

alfaro96 · 2020-11-05T11:56:54Z

The generated wheels vendor vcomp140.dll:

unzip -l scikit_learn-0.24.dev0-cp36-cp36m-win32.whl  | grep vcomp
   150400  09-16-2020 12:16   sklearn/.libs/vcomp140.dll

unzip -l scikit_learn-0.24.dev0-cp37-cp37m-win32.whl  | grep vcomp
   150400  09-16-2020 12:16   sklearn/.libs/vcomp140.dll

unzip -l scikit_learn-0.24.dev0-cp38-cp38-win32.whl  | grep vcomp
   150400  09-16-2020 12:16   sklearn/.libs/vcomp140.dll

unzip -l scikit_learn-0.24.dev0-cp39-cp39-win32.whl  | grep vcomp
   150400  09-16-2020 12:16   sklearn/.libs/vcomp140.dll

unzip -l scikit_learn-0.24.dev0-cp36-cp36m-win_amd64.whl  | grep vcomp
   177032  09-16-2020 12:15   sklearn/.libs/vcomp140.dll

unzip -l scikit_learn-0.24.dev0-cp37-cp37m-win_amd64.whl  | grep vcomp
   177032  09-16-2020 12:15   sklearn/.libs/vcomp140.dll

unzip -l scikit_learn-0.24.dev0-cp37-cp37m-win_amd64.whl  | grep vcomp
   177032  09-16-2020 12:15   sklearn/.libs/vcomp140.dll

unzip -l scikit_learn-0.24.dev0-cp38-cp38-win_amd64.whl  | grep vcomp
   177032  09-16-2020 12:15   sklearn/.libs/vcomp140.dll

unzip -l scikit_learn-0.24.dev0-cp39-cp39-win_amd64.whl  | grep vcomp
   177032  09-16-2020 12:15   sklearn/.libs/vcomp140.dll

thomasjpfan

This is amazing @alfaro96 !

thomasjpfan · 2020-11-05T16:12:13Z

.github/workflows/wheels.yml

+          CIBW_TEST_REQUIRES: pytest pandas threadpoolctl
+          # Test that there are no links to system libraries
+          CIBW_TEST_COMMAND: pytest --pyargs sklearn &&
+                             python -m threadpoolctl -i sklearn


Do we need code to check the output of python -m threadpoolctl -i sklearn?

We could write another helper python script that does this using the Python API of threadpoolctl instead of the command line.

I still think it's good to print it on stdout for manual checks and debugging.

By the way I manually checked the output of one of the last green build for windows below and it looks good:

[ { "filepath": "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpnijjd_bt\\lib\\site-packages\\sklearn\\.libs\\vcomp140.dll", "prefix": "vcomp", "user_api": "openmp", "internal_api": "openmp", "version": null, "num_threads": 2 }, { "filepath": "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpnijjd_bt\\lib\\site-packages\\numpy\\.libs\\libopenblas.NOIJJG62EMASZI6NYURL6JBKM4EVBGM7.gfortran-win_amd64.dll", "prefix": "libopenblas", "user_api": "blas", "internal_api": "openblas", "version": "0.3.9.dev", "num_threads": 2, "threading_layer": "pthreads" }, { "filepath": "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpnijjd_bt\\lib\\site-packages\\scipy\\.libs\\libopenblas.3HBPCJB5BPQGKWVZAVEBXNNJ2Q2G3TUP.gfortran-win_amd64.dll", "prefix": "libopenblas", "user_api": "blas", "internal_api": "openblas", "version": "0.3.9", "num_threads": 2, "threading_layer": "pthreads" } ]

.github/workflows/wheels.yml

alfaro96 · 2020-11-05T16:46:54Z

Is there anything else to do?

ogrisel · 2020-11-05T17:13:15Z

Is there anything else to do?

Addressing https://github.com/scikit-learn/scikit-learn/pull/17921/files#r518173353 would be a nice to have but I am fine with merging this PR as it is and further improve it other PRs.

ogrisel

LGTM! BTW the way, I just defined the two upload tokens as github secrets on the scikit-learn repository to hopefully have the anaconda upload work as expected.

alfaro96 · 2020-11-05T17:23:52Z

Is there anything else to do?

Addressing https://github.com/scikit-learn/scikit-learn/pull/17921/files#r518173353 would be a nice to have but I am fine with merging this PR as it is and further improve it other PRs.

Indeed, I still want to provide the wheels for aarch64 using travis, but I need to refactor the .travis.yml file and I think that it would be better to address in a separate PR.

alfaro96

Just a thought.

.github/workflows/wheels.yml

ogrisel · 2020-11-06T09:33:08Z

@thomasjpfan @rth shall we merge this to test it a bit "for real" and then open new PRs with improvements (e.g. ARM builds, additional sanity checks...)

rth · 2020-11-06T09:51:57Z

I haven't reviewed, but sound good to me. Great work @alfaro96 !

ogrisel · 2020-11-06T09:53:35Z

@rth I pinged you because you marked this PR as 1 change requested a while ago but I believe your concerns have been addressed. Let's wait for @thomasjpfan then.

ogrisel · 2020-11-06T09:54:43Z

Oh I now see that it's an artifact of the lack of cancel review button on github... Sorry for the noise.

alfaro96 · 2020-11-06T10:49:21Z

@thomasjpfan @rth shall we merge this to test it a bit "for real" and then open new PRs with improvements (e.g. ARM builds, additional sanity checks...)

That would be nice 😉.

Should we specify the [cd build] marker when merging or wait for the schedule job?

ogrisel · 2020-11-06T14:05:11Z

Let me merge it with the [cd build] to give it a full end to end run.

alfaro96 · 2020-11-06T14:12:04Z

Okay, I forgot to add a trigger for push on the master branch and only build on it if the [cd build] commit trigger is found.

A quick fix is coming.

ogrisel · 2020-11-06T14:28:57Z

yes I just noticed... cd60a35.

[WIP] MNT Use cibuildwheel to build the wheels

49ec457

alfaro96 marked this pull request as draft July 14, 2020 09:16

alfaro96 added 3 commits July 14, 2020 11:17

FIX Fix linting issues

633a13e

FIX Fix linting issues

28de443

FIX Fix linting issues

bf4376d

adrinjalali reviewed Jul 14, 2020

View reviewed changes

.github/workflows/wheels.yml Outdated Show resolved Hide resolved

.github/workflows/wheels.yml Show resolved Hide resolved

.github/workflows/wheels.yml Show resolved Hide resolved

.github/workflows/wheels.yml Outdated Show resolved Hide resolved

rth approved these changes Jul 14, 2020

View reviewed changes

.github/workflows/wheels.yml Outdated Show resolved Hide resolved

rth suggested changes Jul 14, 2020

View reviewed changes

rth mentioned this pull request Jul 14, 2020

Update to manylinux2010 MacPython/scikit-learn-wheels#62

Closed

alfaro96 added 10 commits July 14, 2020 13:03

CLN Apply suggested changes

804badf

FIX Fix linting issues

09f4752

FIX Fix linting issues

3d72b38

FIX Fix linting issues

1063808

MNT Minor changes

befe7bd

MNT Minor changes

262666b

FIX Run setup.py after installing scikit-learn from source

e86b837

FIX Run build_ext after installing scikit-learn

11272d4

TST Test if manylinux and macos builds work

4f2ae2a

MNT Minor changes

3a47d2e

alfaro96 requested review from adrinjalali and rth July 14, 2020 14:35

alfaro96 added 6 commits July 14, 2020 18:21

Merge remote-tracking branch 'upstream/master' into wheel_builder

36a0cf5

MNT Minor changes

8c9392d

Merge remote-tracking branch 'upstream/master' into wheel_builder

68f0be6

Merge remote-tracking branch 'upstream/master' into wheel_builder

210530d

[FIX] Remove macos-latest for testing.

ead8bb4

[MNT] Embed vcomp dll for Windows wheel.

d91d821

alfaro96 added 2 commits November 5, 2020 11:47

FIX Use Windows variable substitution [cd build]

0f780c0

FIX Quietly remove the directory [cd build]

07ac26d

MNT Export environment variables [cd build]

13b0118

thomasjpfan reviewed Nov 5, 2020

View reviewed changes

ogrisel reviewed Nov 5, 2020

View reviewed changes

.github/workflows/wheels.yml Show resolved Hide resolved

ogrisel reviewed Nov 5, 2020

View reviewed changes

.github/workflows/wheels.yml Show resolved Hide resolved

ogrisel approved these changes Nov 5, 2020

View reviewed changes

alfaro96 commented Nov 5, 2020

View reviewed changes

.github/workflows/wheels.yml Outdated Show resolved Hide resolved

MNT Trigger all pushes to release branches [cd build]

38249b6

ogrisel merged commit cd60a35 into scikit-learn:master Nov 6, 2020

alfaro96 deleted the wheel_builder branch November 6, 2020 14:07

This was referenced Nov 6, 2020

MNT Support and test Python 3.9 #18761

Merged

Updade the maintainers' guide to use the new Wheel Builder github actions tool #18776

Closed

alfaro96 mentioned this pull request Nov 7, 2020

CI Build wheels for the ARM64 architecture #18782

Merged

jorisvandenbossche mentioned this pull request Jul 7, 2021

Investigate cibuildwheel pygeos/pygeos#354

Closed

jorisvandenbossche mentioned this pull request Oct 14, 2021

Consider cibuildwheel for building wheels? pandas-dev/pandas#44027

Closed

Uh oh!

Conversation

alfaro96 commented Jul 14, 2020

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Uh oh!

adrinjalali left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alfaro96 commented Jul 14, 2020

Uh oh!

rth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rth left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alfaro96 commented Jul 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alfaro96 commented Jul 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alfaro96 commented Nov 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alfaro96 commented Nov 5, 2020

Uh oh!

alfaro96 commented Nov 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomasjpfan left a comment

Choose a reason for hiding this comment

Uh oh!

thomasjpfan Nov 5, 2020

Choose a reason for hiding this comment

Uh oh!

ogrisel Nov 5, 2020

Choose a reason for hiding this comment

Uh oh!

ogrisel Nov 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alfaro96 commented Nov 5, 2020

Uh oh!

ogrisel commented Nov 5, 2020

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

alfaro96 commented Nov 5, 2020

Uh oh!

alfaro96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ogrisel commented Nov 6, 2020

Uh oh!

rth commented Nov 6, 2020

Uh oh!

ogrisel commented Nov 6, 2020

Uh oh!

ogrisel commented Nov 6, 2020

Uh oh!

alfaro96 commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ogrisel commented Nov 6, 2020

Uh oh!

alfaro96 commented Nov 6, 2020

Uh oh!

rth left a comment •

edited

Loading

alfaro96 commented Jul 14, 2020 •

edited

Loading

alfaro96 commented Jul 14, 2020 •

edited

Loading

alfaro96 commented Nov 5, 2020 •

edited

Loading

alfaro96 commented Nov 5, 2020 •

edited

Loading

alfaro96 commented Nov 6, 2020 •

edited

Loading

ogrisel commented Nov 6, 2020 •

edited

Loading