Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use libkvikio conda packages in libcudf, add explicit libcufile dependency. #13231

Merged
merged 6 commits into from
May 9, 2023

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Apr 26, 2023

Description

Provides a partial solution to #13230. During conda builds of libcudf, libkvikio is currently being fetched by CPM rather than supplied by a conda dependency. This means libkvikio headers are being shipped as part of libcudf's packages, which is not desirable.

I also added libcufile[-dev] as an explicit dependency of libcudf. Note that this is only available on linux64 (amd64), not aarch64 (arm64). We should always make the cuFile library available at build time for conda packages on amd64.

The impacts of this change are:

  • libcudf conda packages will no longer ship libkvikio headers (those headers will instead be supplied by libkvikio packages). This reduces the size of libcudf and prevents clobbering files from libkvikio.
  • libcudf will have a dependency on libcufile from the nvidia channel on linux64 (but not aarch64, since libcufile packages currently do not exist for aarch64).

This change does not impact builds outside of conda-build.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@bdice bdice requested a review from a team as a code owner April 26, 2023 20:49
@bdice bdice requested a review from vuule April 26, 2023 20:49
@bdice bdice self-assigned this Apr 26, 2023
@github-actions github-actions bot added the conda label Apr 26, 2023
@bdice bdice added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 26, 2023
@bdice bdice changed the title Use libkvikio conda packages in libcudf. Use libkvikio conda packages in libcudf, add explicit libcufile dependency. Apr 26, 2023
@bdice
Copy link
Contributor Author

bdice commented Apr 27, 2023

Verified that builds show the libkvikio conda package being used on amd64 and arm64.

-- CPM: using local package [email protected]

Also adding the cuFile dependency caused -DCUFILE_FOUND to be added to every compile line on amd64. This meant it had to build without sccache and took a long time! However, that's also a confirmation that cuFile support should now be working in packages built by CI. 😄 cc: @madsbk @vuule

@github-actions github-actions bot added the ci label May 1, 2023
Copy link
Contributor Author

@bdice bdice May 1, 2023

Choose a reason for hiding this comment

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

FYI @ajschmidt8 -- you requested some changes on cuspatial and ucxx for recent changes to the update-version.sh script. I started to make those changes here but decided it's too large of a scope for this PR. Also, cudf's update script is significant more complex than ucxx/cuspatial and I didn't want to set the wrong example (since so many packages look to cudf for guidance). If you want this to be updated in the near term, I would appreciate your help!

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

There are a lot of process changes I want to make around these scripts in the future.

Realistically I probably won't start that initiative until 23.08 or even 23.10.

@bdice
Copy link
Contributor Author

bdice commented May 8, 2023

For the record, pip wheels are built with cuFile support. I checked both CUDA 11.8 and CUDA 12.0 builds. AMD64 only, because cuFile isn't supported on ARM64.

  -- CPM: adding package [email protected] (branch-23.06)
  -- Found cuFile: /usr/local/cuda/lib64/libcufile.so

Copy link
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

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

FWIW, looks good to me :)
This is an important fix for cuFile integration, thank you @bdice !

@bdice
Copy link
Contributor Author

bdice commented May 8, 2023

/merge

@rapids-bot rapids-bot bot merged commit ac158da into rapidsai:branch-23.06 May 9, 2023
@bdice bdice mentioned this pull request Jun 7, 2023
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Jun 9, 2023
This PR adds a libcufile dependency specification, only for x86_64 (the package is not available on ARM). This aligns with the conda recipe specification, previously updated in #13231.

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - AJ Schmidt (https://github.com/ajschmidt8)
  - Lawrence Mitchell (https://github.com/wence-)

URL: #13523
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants