Skip to content

feat(libcuvs): port libcuvs to rattler-build #751

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

Merged
merged 45 commits into from
Mar 19, 2025

Conversation

gforsyth
Copy link
Contributor

@gforsyth gforsyth commented Mar 6, 2025

This PR ports cuVS over to rattler-build for building conda packages.

Some changes to the build process:

  • cuvs-bench was recompiling the entire library separately -- caching was hopefully helping with this but it was an unnecessary double-compile. Instead, I've added the bench-ann to the pre-build stage in libcuvs and create a libcuvs-bench output there, that cuvs-bench then depends on to provide the necessary shared objects.

  • Edited build.sh so that if -n is passed, the cuvs-bench Python package is not installed.

  • libcuvs-static seems to be a leftover from a previous incarnation -- the existing libcuvs-static packages on rapidsai-nightly consist only of build metadata files and are otherwise empty. I've removed the unused --compile-static-lib option from build.sh

@gforsyth gforsyth added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Mar 6, 2025
Copy link

copy-pr-bot bot commented Mar 6, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions github-actions bot added the Python label Mar 6, 2025
@gforsyth gforsyth changed the title feat(libcudf): port libcuvs to rattler-build feat(libcuvs): port libcuvs to rattler-build Mar 6, 2025
@gforsyth
Copy link
Contributor Author

gforsyth commented Mar 6, 2025

/ok to test

@github-actions github-actions bot added the ci label Mar 6, 2025
@gforsyth
Copy link
Contributor Author

gforsyth commented Mar 6, 2025

/ok to test

@gforsyth
Copy link
Contributor Author

gforsyth commented Mar 6, 2025

/ok to test

@gforsyth
Copy link
Contributor Author

gforsyth commented Mar 6, 2025

/ok to test

@gforsyth gforsyth requested a review from AyodeAwe March 10, 2025 17:44
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

A few comments but overall this is great. I'm really glad this rattler conversion is getting easier as we progress through more RAPIDS libraries.

@gforsyth
Copy link
Contributor Author

This is ready for another look @bdice

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

All looks good to me!

then:
- libaio
- libboost-devel=1.87
- mkl-devel=2023
Copy link
Contributor

Choose a reason for hiding this comment

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

For follow-up: Why is this pinned to mkl 2023? The latest is 2025.0.1. https://anaconda.org/conda-forge/mkl/files

Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

Couple notes for future work, but LGTM.

Comment on lines +24 to +30
# Remove `-fdebug-prefix-map` line from CFLAGS and CXXFLAGS so the
# incrementing version number in the compile line doesn't break the
# cache
set -x
export CFLAGS=$(echo $CFLAGS | sed -E 's@\-fdebug\-prefix\-map[^ ]*@@g')
export CXXFLAGS=$(echo $CXXFLAGS | sed -E 's@\-fdebug\-prefix\-map[^ ]*@@g')
set +x
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Future suggestion: rather than maintaining separate cuvs-bench and cuvs-bench-cpu recipes, we should parametrize the recipe. The two should be almost identical.

@gforsyth
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 2c3df75 into rapidsai:branch-25.04 Mar 19, 2025
66 checks passed
@gforsyth gforsyth deleted the rattler-build branch March 19, 2025 17:42
@gforsyth
Copy link
Contributor Author

Followups recorded in rapidsai/build-planning#47 (comment)

divyegala pushed a commit to divyegala/cuvs that referenced this pull request Mar 21, 2025
This PR ports cuVS over to `rattler-build` for building conda packages.  

Some changes to the build process:

* `cuvs-bench` was recompiling the entire library separately -- caching was hopefully helping with this but it was an unnecessary double-compile.  Instead, I've added the `bench-ann` to the pre-build stage in `libcuvs` and create a `libcuvs-bench` output there, that `cuvs-bench` then depends on to provide the necessary shared objects.

* Edited `build.sh` so that if `-n` is passed, the `cuvs-bench` Python package is not installed.

* `libcuvs-static` seems to be a leftover from a previous incarnation -- the existing `libcuvs-static` packages on `rapidsai-nightly` consist only of build metadata files and are otherwise empty.  I've removed the unused `--compile-static-lib` option from `build.sh`

Authors:
  - Gil Forsyth (https://github.com/gforsyth)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: rapidsai#751
jiangyinzuo pushed a commit to jiangyinzuo/cuvs that referenced this pull request Mar 27, 2025
This PR ports cuVS over to `rattler-build` for building conda packages.  

Some changes to the build process:

* `cuvs-bench` was recompiling the entire library separately -- caching was hopefully helping with this but it was an unnecessary double-compile.  Instead, I've added the `bench-ann` to the pre-build stage in `libcuvs` and create a `libcuvs-bench` output there, that `cuvs-bench` then depends on to provide the necessary shared objects.

* Edited `build.sh` so that if `-n` is passed, the `cuvs-bench` Python package is not installed.

* `libcuvs-static` seems to be a leftover from a previous incarnation -- the existing `libcuvs-static` packages on `rapidsai-nightly` consist only of build metadata files and are otherwise empty.  I've removed the unused `--compile-static-lib` option from `build.sh`

Authors:
  - Gil Forsyth (https://github.com/gforsyth)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: rapidsai#751
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci improvement Improves an existing functionality non-breaking Introduces a non-breaking change Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants