Disable cross compiling for POWER architecture #47

carterbox · 2021-11-17T19:02:39Z

We suspect that the emulated (as opposed to native) builds may be causing the tests to fail for these architectures. This commit tries to bump the conda-forge builds back to native hosts instead of cross-compiling.

Closes PyWavelets/pywt#607

Checklist

Used a personal fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
Ensured the license file is being packaged.

We suspect that the emulated (as opposed to native) builds may be causing the tests to fail for these architectures. This commit tries to bump the conda-forge builds back to native hosts instead of cross-compiling. PyWavelets/pywt#607

conda-forge-linter · 2021-11-17T19:02:45Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

carterbox · 2021-11-17T19:03:46Z

@conda-forge-admin, please rerender

…nda-forge-pinning 2021.11.17.15.51.01

jakirkham · 2021-11-17T19:05:43Z

ARM can't be moved ( conda-forge/conda-forge.github.io#1555 )

Edit: Though we already have it configured to emulate anyways.

conda-forge.yml

In theory, setting a provider for osx_arm64 should cause that architecture to be built, but due to the drone.io outage we need to force cross-compilation on another platform. We also need to force compilation of linux_ppc64le natively because tests fail when emulated.

…nda-forge-pinning 2021.11.17.15.51.01

carterbox · 2021-11-17T20:12:31Z

Looks like cross-compiling was the problem! Will mark ready for review once all the builds are green.

jakirkham · 2021-11-17T21:37:28Z

conda-forge.yml

-  linux_aarch64: linux_64
-  linux_ppc64le: linux_64
-  osx_arm64: osx_64
+  linux_ppc64le: linux_ppc64le  # tests fail when emulating power from x86


Think this can just be default since we build natively there

Suggested change

linux_ppc64le: linux_ppc64le # tests fail when emulating power from x86

I explicitly choose linux_ppc64le because we know that the build doesn't work when emulated. The default could be changed to linux_64 like it was for linux_aarch64, then this build would break again.

Also, It only renders with linux_ppc64le. I cannot use the default key here; it causes the following error:

File "/home/dching/miniconda3/envs/forge/lib/python3.9/site-packages/conda_smithy/configure_feedstock.py", line 947, in _get_platforms_of_provider build_platform, build_arch = build_platform_arch.split("_") ValueError: not enough values to unpack (expected 2, got 1)

Seems like the build platform is expected to be an explicit platform if defined.

conda-forge.yml

carterbox · 2021-11-17T22:47:28Z

These linux ARM builds have been running for more than 2.5 hours! 😟

jakirkham · 2021-11-17T23:37:24Z

conda-forge.yml

  osx_arm64: osx_64
 conda_forge_output_validation: true
 provider:
-  linux_aarch64: default
+  linux_aarch64: travis


There is no ARM support on Travis. We are stuck emulating them on Azure. Yes it does take a while unfortunately.

Please see PR ( conda-forge/conda-forge.github.io#1555 )

rgommers · 2021-11-18T10:50:46Z

LGTM, happy if it works!

Random question: PRs on feedstocks are always a pain to review, because the regular diff view is mostly changes from re-rendering. Is there any guidance or best practice around this, like one should be squashing history to 1-2 commits with changes, and one re-rendering commit? And then one can review the commit diff?

grlee77 · 2021-11-18T14:27:12Z

Usually I just click on the commits tab to see the non-rerendering ones individually. In most cases there are only a small number, so that works out okay.

carterbox · 2021-11-18T17:23:36Z

I believe only the files in the recipe folder and conda-forge.yml control the rendering (are managed by humans or bots), so I look at those.

isuruf · 2021-11-18T19:45:23Z

Btw, can you check if using blas=*=*netlib fix the cross compiling issue. I think it's because of openblas using some system calls not supported by qemu.

jakirkham · 2021-11-18T19:54:04Z

We hide some files with .gitattributes. Would ignore those. There's always a delicate balance around what to hide and what to include.

jakirkham · 2021-11-18T19:57:29Z

Btw, can you check if using blas=*=*netlib fix the cross compiling issue. I think it's because of openblas using some system calls not supported by qemu.

This is a really good point!

Have seen similar things with VMs (like VirtualBox) where AVX and some versions of SSE instructions don't work. Though normally in that case different kernels without those instructions get selected as OpenBLAS determines they are not supported.

What system calls did you have in mind?

isuruf · 2021-11-18T20:51:28Z

mbind

Maybe emulated builds of ppc64le are failing because the BLAS implemenation doesn't support emulation. conda-forge#47 (comment)

carterbox · 2021-11-23T23:40:15Z

Adding blas=*=*netlib to the test requirements did not work.

erykoff · 2021-11-29T05:03:00Z

FYI, I believe this PR is preventing the scikit-image python 3.10 migration.

erykoff · 2021-11-29T05:03:28Z

Or rather, merging this PR will unblock the scikit-image python 3.10 migration.

rgommers

LGTM for merging. Can this be squash-merged, or is it needed/useful to rewrite into a single commit with functional changes and one other commit to re-render?

jakirkham · 2021-11-29T11:47:44Z

It's useful to see the commits from re-rendering on the non-recipe files in case there is an issue we need to dig into later

beckermr · 2021-11-29T18:11:30Z

I agree w/ @jakirkham but there is no reason to change things now. Let's go ahead and merge.

Try to disable cross compiling for ARM and POWER

63f591a

We suspect that the emulated (as opposed to native) builds may be causing the tests to fail for these architectures. This commit tries to bump the conda-forge builds back to native hosts instead of cross-compiling. PyWavelets/pywt#607

MNT: Re-rendered with conda-build 3.21.6, conda-smithy 3.14.3, and co…

4866c05

…nda-forge-pinning 2021.11.17.15.51.01

jakirkham reviewed Nov 17, 2021

View reviewed changes

conda-forge.yml Show resolved Hide resolved

carterbox added 3 commits November 17, 2021 14:08

MNT: Re-rendered with conda-build 3.21.6, conda-smithy 3.14.3, and co…

dc0621c

…nda-forge-pinning 2021.11.17.15.51.01

Bump the build number

b66c660

carterbox force-pushed the master branch from b9fec3c to b66c660 Compare November 17, 2021 20:08

carterbox changed the title ~~Try to disable cross compiling for ARM and POWER~~ Disable cross compiling for POWER architecture Nov 17, 2021

jakirkham reviewed Nov 17, 2021

View reviewed changes

conda-forge.yml Outdated Show resolved Hide resolved

jakirkham reviewed Nov 17, 2021

View reviewed changes

conda-forge.yml Outdated Show resolved Hide resolved

DOC: Remove incorrect note from conda-forge.yml

3785d32

jakirkham reviewed Nov 17, 2021

View reviewed changes

carterbox force-pushed the master branch from 10f66e0 to 3785d32 Compare November 18, 2021 00:13

carterbox marked this pull request as ready for review November 18, 2021 00:15

carterbox requested review from grlee77, ocefpaf and rgommers as code owners November 18, 2021 00:15

jakirkham mentioned this pull request Nov 18, 2021

conda-forge.yml provider behavior is inconsistent conda-forge/conda-smithy#1546

Closed

1 task

carterbox mentioned this pull request Nov 23, 2021

DEV: Try swapping BLAS implementation when emulating #48

Closed

5 tasks

rgommers approved these changes Nov 29, 2021

View reviewed changes

jakirkham merged commit caef6b6 into conda-forge:master Nov 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable cross compiling for POWER architecture #47

Disable cross compiling for POWER architecture #47

carterbox commented Nov 17, 2021 •

edited

conda-forge-linter commented Nov 17, 2021

carterbox commented Nov 17, 2021

jakirkham commented Nov 17, 2021 •

edited

carterbox commented Nov 17, 2021

jakirkham Nov 17, 2021 •

edited

carterbox Nov 17, 2021

carterbox Nov 17, 2021 •

edited

carterbox commented Nov 17, 2021

jakirkham Nov 17, 2021

rgommers commented Nov 18, 2021

grlee77 commented Nov 18, 2021

carterbox commented Nov 18, 2021

isuruf commented Nov 18, 2021

jakirkham commented Nov 18, 2021

jakirkham commented Nov 18, 2021

isuruf commented Nov 18, 2021

carterbox commented Nov 23, 2021

erykoff commented Nov 29, 2021

erykoff commented Nov 29, 2021

rgommers left a comment

jakirkham commented Nov 29, 2021

beckermr commented Nov 29, 2021

Disable cross compiling for POWER architecture #47

Disable cross compiling for POWER architecture #47

Conversation

carterbox commented Nov 17, 2021 • edited

conda-forge-linter commented Nov 17, 2021

carterbox commented Nov 17, 2021

jakirkham commented Nov 17, 2021 • edited

carterbox commented Nov 17, 2021

jakirkham Nov 17, 2021 • edited

Choose a reason for hiding this comment

carterbox Nov 17, 2021

Choose a reason for hiding this comment

carterbox Nov 17, 2021 • edited

Choose a reason for hiding this comment

carterbox commented Nov 17, 2021

jakirkham Nov 17, 2021

Choose a reason for hiding this comment

rgommers commented Nov 18, 2021

grlee77 commented Nov 18, 2021

carterbox commented Nov 18, 2021

isuruf commented Nov 18, 2021

jakirkham commented Nov 18, 2021

jakirkham commented Nov 18, 2021

isuruf commented Nov 18, 2021

carterbox commented Nov 23, 2021

erykoff commented Nov 29, 2021

erykoff commented Nov 29, 2021

rgommers left a comment

Choose a reason for hiding this comment

jakirkham commented Nov 29, 2021

beckermr commented Nov 29, 2021

carterbox commented Nov 17, 2021 •

edited

jakirkham commented Nov 17, 2021 •

edited

jakirkham Nov 17, 2021 •

edited

carterbox Nov 17, 2021 •

edited