Include CUDA enabled pyarrow #146

pearu · 2020-05-20T20:18:10Z

Fixes conda-forge/pyarrow-feedstock#107
Fixes #93

Checklist

Used a 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.

conda-forge-linter · 2020-05-20T20:18:17Z

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.

…da-forge-pinning 2020.05.20.09.40.28

jakirkham · 2020-05-20T21:22:04Z

Thanks for picking this up @pearu! 😄

cc @h-vetinari (who also seemed interested in this)

h-vetinari · 2020-05-20T21:31:01Z

Thanks for the ping @jakirkham; looks good @pearu!

Also closes #93 (see discussion there). Finally, merging this means the pyarrow-feedstock must be disabled/archived somehow - don't know how that works.

recipe/meta.yaml

pearu · 2020-05-20T21:49:39Z

pyarrow fails to build on aarch64:

CMake Error at cmake_modules/SetupCxxFlags.cmake:338 (message):
  Unsupported arch flag: -march=.
Call Stack (most recent call first):
  CMakeLists.txt:100 (include)

I have disabled aarch64 for pyarrow but this does not feel right.
Does anyone have an idea of how to fix the pyarrow build for aarch64?

…da-forge-pinning 2020.05.20.09.40.28

…da-forge-pinning 2020.05.20.21.21.18

h-vetinari · 2020-05-26T15:14:40Z

-  - name: pyarrow
+  - name: arrow-py

I don't think that's the right approach. We want this feedstock to serve both packages, and have the packages not interfere with each other (which is automatic if they have the same name, otherwise we need another mutex etc.).

@conda-forge/core How can we absorb one feedstock into another correctly?

pearu · 2020-05-26T16:17:44Z

I was about to propose https://github.com/conda-forge/parquet-cpp-feedstock approach where the future pyarrow packages would just point to arrow-py.
But I second the question of how to absorb feedstocks correctly.

xhochy · 2020-05-27T13:58:55Z

We should be building pyarrow here. I have the superpowers to enable that but I'll have to consult @beckermr on how to use it ;)

xhochy

Looks good, some minor suggestions.

We shouldn't rename the package though!

recipe/build-pyarrow.sh

recipe/meta.yaml

…f arrow-cpp in pyarrow. Restore pyarrow package name.

pearu · 2020-05-27T16:39:02Z

@xhochy I have addressed all your comments. Surprisingly, there are no validation failures now.
Can you land this PR?

xhochy · 2020-05-27T16:41:15Z

Requires conda-forge/feedstock-outputs#3

There should have been validation failures 🤔

I'll test this locally and merge then.

pearu · 2020-05-27T16:42:11Z

The message

Cloning into '/tmp/tmpeilv4_v4_recipe/feedstock-outputs'...
validation results:
{
  "linux-64/arrow-cpp-0.17.1-py37h1234567_3_cuda.tar.bz2": true,
  "linux-64/pyarrow-0.17.1-py37h1234567_3_cuda.tar.bz2": false,
  "linux-64/arrow-cpp-proc-1.0.0-cuda.tar.bz2": true
}
NOTE: Any outputs marked as False are not allowed for this feedstock.

persists. I guess your superpowers will be needed after all, @xhochy .

xhochy · 2020-05-27T16:47:46Z

@conda-forge-admin please restart ci

pearu · 2020-05-27T17:55:32Z

Validate seems to be ok with the feedstock:

+ validate_recipe_outputs arrow-cpp-feedstock
Cloning into '/tmp/tmp95n9c6e__recipe/feedstock-outputs'...
validation results:
{
  "linux-64/arrow-cpp-0.17.1-py37h1234567_3_cuda.tar.bz2": true,
  "linux-64/pyarrow-0.17.1-py37h1234567_3_cuda.tar.bz2": true,
  "linux-64/arrow-cpp-proc-1.0.0-cuda.tar.bz2": true
}
NOTE: Any outputs marked as False are not allowed for this feedstock.

xhochy

I'll merge tomorrow morning. If something goes wrong, I then have the time to clean up.

h-vetinari · 2020-05-27T19:09:15Z

@pearu, can you put Fixes #93 in the OP?

pearu · 2020-05-27T19:18:06Z

@pearu, can you put Fixes #93 in the OP?

Done in the description of the issue.

h-vetinari · 2020-05-27T22:24:57Z

Regarding the naming conventions, I think it might be worth renaming arrow-cpp to libarrow. This is in line with a lot of other feedstocks, but obviously a matter of taste. In this case, we'd have to have a compatibility output of arrow-cpp that depends on libarrow, which could be dropped after a few versions.

@isuruf made me aware of this in the context of faiss. Other examples I can think of off the top of my head are blas & lapack, openblas, opencv, postgresql, gdal, plus a whole bunch more (non-exhaustive).

Of course, this recipe has some counter examples to this: aws-sdk-cpp, boost-cpp, grpc-cpp, thrift-cpp, but I think they are far in the minority and the same argument could be made for renaming those (maybe worth noting that at least the last two seem to have been started by the arrow team).

Happy to open an issue for this if this needs more discussion.

isuruf · 2020-05-28T00:06:24Z

-cpp was a trend that I started with boost-cpp. That was a mistake. I'm in favour of changing it, but this is not the correct place to raise the issue.

jakirkham

This generally seems good to me. Thanks @pearu! 😄

Had a couple minor comments

jakirkham · 2020-05-28T02:56:18Z

recipe/build-pyarrow.sh

+$PYTHON setup.py \
+        build_ext $BUILD_EXT_FLAGS \
+        install --single-version-externally-managed \
+                --record=record.txt


Are we able to use pip here?

We should check that but in a separate PR.

Yeah that makes sense. Was having a little trouble seeing how this was built before. If this is no different from what we were already doing, agree we don't need to change it here.

Using pip does not work out of the box:

$PYTHON -m pip install . --no-deps -vv build_ext $BUILD_EXT_FLAGS install

fails with

pip._internal.exceptions.DistributionNotFound: No matching distribution found for build_ext

You need something like,

$PYTHON -m pip install . --no-deps -vv --global-option=build_ext --global-option=--with-cuda

Right. The following diff works locally:

diff --git a/recipe/build-pyarrow.sh b/recipe/build-pyarrow.sh index 99396d8..4b01871 100644 --- a/recipe/build-pyarrow.sh +++ b/recipe/build-pyarrow.sh @@ -22,7 +22,7 @@ BUILD_EXT_FLAGS="" if [[ ! -z "${cuda_compiler_version+x}" && "${cuda_compiler_version}" != "None" ]] then export PYARROW_WITH_CUDA=1 - BUILD_EXT_FLAGS="${BUILD_EXT_FLAGS} --with-cuda" + BUILD_EXT_FLAGS="${BUILD_EXT_FLAGS} --global-option=build_ext --global-option=--with-cuda" else export PYARROW_WITH_CUDA=0 fi @@ -35,7 +35,4 @@ fi cd python -$PYTHON setup.py \ - build_ext $BUILD_EXT_FLAGS \ - install --single-version-externally-managed \ - --record=record.txt +$PYTHON -m pip install . --no-deps -vv $BUILD_EXT_FLAGS diff --git a/recipe/meta.yaml b/recipe/meta.yaml index 8a0edf7..20523e2 100644 --- a/recipe/meta.yaml +++ b/recipe/meta.yaml @@ -181,6 +181,7 @@ outputs: - boost-cpp - cython - numpy 1.16.* + - pip - python - setuptools - setuptools_scm

Just FYI.

jakirkham · 2020-05-28T02:56:37Z

recipe/bld-pyarrow.bat

+%PYTHON%   setup.py ^
+           build_ext ^
+           install --single-version-externally-managed ^
+                   --record=record.txt


Same question about pip here

xhochy · 2020-05-28T05:47:03Z

Happy to open an issue for this if this needs more discussion.

Please do so at https://github.com/conda-forge/conda-forge.github.io

xhochy · 2020-05-28T05:49:09Z

Thanks @pearu , let's 🙏 this works!

h-vetinari · 2020-05-28T07:04:14Z

@xhochy: Please do so at https://github.com/conda-forge/conda-forge.github.io

Followed-up in conda-forge/conda-forge.github.io#1073.

Include pyarrow

2bf71bc

pearu requested review from cpcloud, jreback, kou, kszucs, leifwalsh, nealrichardson, pcmoritz, pitrou, robertnishihara, siddharthteotia, wesm and xhochy as code owners May 20, 2020 20:18

pearu marked this pull request as draft May 20, 2020 20:18

pearu changed the title ~~Include CUDA enabled arrow~~ Include CUDA enabled pyarrow May 20, 2020

pearu self-assigned this May 20, 2020

MNT: Re-rendered with conda-build 3.19.2, conda-smithy 3.7.3, and con…

877ba52

…da-forge-pinning 2020.05.20.09.40.28

Skip building pyarrow for aarch64

8fe1859

h-vetinari reviewed May 20, 2020

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

pearu added 7 commits May 21, 2020 00:52

Use azure provider for linux_aarch64

6061ee1

MNT: Re-rendered with conda-build 3.19.2, conda-smithy 3.7.3, and con…

4a0c756

…da-forge-pinning 2020.05.20.09.40.28

Re-enable aarch64 for pyarrow

7d17b71

Try fixing -march flag for aarch64

1e346ad

Switch linux_aarch64 back to default.

86d85e1

MNT: Re-rendered with conda-build 3.19.2, conda-smithy 3.7.3, and con…

0e79569

…da-forge-pinning 2020.05.20.21.21.18

Fix ARROW_ARMV8_ARCH usage.

1eb263e

xhochy requested changes May 27, 2020

View reviewed changes

recipe/build-pyarrow.sh Outdated Show resolved Hide resolved

recipe/meta.yaml Outdated Show resolved Hide resolved

recipe/meta.yaml Outdated Show resolved Hide resolved

recipe/meta.yaml Outdated Show resolved Hide resolved

Build pyarrow.gandiva unconditionally. Use pin_subpackage.. instead o…

aff54f7

…f arrow-cpp in pyarrow. Restore pyarrow package name.

pearu requested a review from xhochy May 27, 2020 16:39

conda-forge-linter closed this May 27, 2020

conda-forge-linter reopened this May 27, 2020

xhochy approved these changes May 27, 2020

View reviewed changes

jakirkham reviewed May 28, 2020

View reviewed changes

xhochy merged commit 99a1e8d into conda-forge:master May 28, 2020

h-vetinari mentioned this pull request May 28, 2020

Consistent naming for library-packages conda-forge/conda-forge.github.io#1073

Open

pearu deleted the pearu/pyarrow-107 branch May 30, 2020 22:07

jakirkham mentioned this pull request Jun 2, 2020

[FEA] Upgrade to Arrow 0.17.1 rapidsai/cudf#5323

Closed

h-vetinari mentioned this pull request Jun 27, 2020

RFC: Rename arrow-cpp to libarrow #158

Closed

h-vetinari mentioned this pull request Jan 8, 2021

Windows + Cuda #247

Merged

5 tasks

h-vetinari mentioned this pull request Oct 23, 2022

BUG: conda smithy not taking into account (all) python versions for all-in-one-aarch builds conda-forge/conda-smithy#1681

Open

h-vetinari mentioned this pull request May 3, 2024

RFC: Split off pyarrow-* builds #1381

Closed

Uh oh!

Include CUDA enabled pyarrow #146

Include CUDA enabled pyarrow #146

Uh oh!

Conversation

pearu commented May 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

conda-forge-linter commented May 20, 2020

Uh oh!

jakirkham commented May 20, 2020

Uh oh!

h-vetinari commented May 20, 2020

Uh oh!

Uh oh!

pearu commented May 20, 2020

Uh oh!

h-vetinari commented May 26, 2020

Uh oh!

pearu commented May 26, 2020

Uh oh!

xhochy commented May 27, 2020

Uh oh!

xhochy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pearu commented May 27, 2020

Uh oh!

xhochy commented May 27, 2020

Uh oh!

pearu commented May 27, 2020

Uh oh!

xhochy commented May 27, 2020

Uh oh!

pearu commented May 27, 2020

Uh oh!

xhochy left a comment

Choose a reason for hiding this comment

Uh oh!

h-vetinari commented May 27, 2020

Uh oh!

pearu commented May 27, 2020

Uh oh!

h-vetinari commented May 27, 2020

Uh oh!

isuruf commented May 28, 2020

Uh oh!

jakirkham left a comment

Choose a reason for hiding this comment

Uh oh!

jakirkham May 28, 2020

Choose a reason for hiding this comment

Uh oh!

xhochy May 28, 2020

Choose a reason for hiding this comment

Uh oh!

jakirkham May 28, 2020

Choose a reason for hiding this comment

Uh oh!

pearu May 28, 2020

Choose a reason for hiding this comment

Uh oh!

isuruf May 28, 2020

Choose a reason for hiding this comment

Uh oh!

pearu May 28, 2020

Choose a reason for hiding this comment

Uh oh!

jakirkham May 28, 2020

Choose a reason for hiding this comment

Uh oh!

xhochy commented May 28, 2020

Uh oh!

xhochy commented May 28, 2020

Uh oh!

h-vetinari commented May 28, 2020

pearu commented May 20, 2020 •

edited

Loading