refactor: use libnvcomp from libkvikio wheel to unblock Python 3.13 upgrade - #18593
Conversation
gforsyth
added
improvement
3949463 to
569295a
Compare
bdice
left a comment
bdice
left a comment
There was a problem hiding this comment.
I wonder if we will need to update any CMake, wheel library-loading, or wheel library-exclusion code to make this work?
|
|
||
| set -x | ||
| #kvikio_wheel_cpp_libkvikio_cu11_aarch64.tar.gz | ||
| LIBKVIKIO_WHL="libkvikio_${RAPIDS_PY_CUDA_SUFFIX}-25.6.0a32-py3-none-${AUDITWHEEL_PLAT}.whl" |
There was a problem hiding this comment.
I think you can follow the instructions in rapidsai/docs#593 for a more concise solution.
Maybe something like:
# ci/use_wheels_from_prs.sh
RAPIDS_PY_CUDA_SUFFIX=$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")
# download wheels, store the directories holding them in variables
LIBKVIKIO_WHEELHOUSE=$(
RAPIDS_PY_WHEEL_NAME="libkvikio_${RAPIDS_PY_CUDA_SUFFIX}" rapids-get-pr-wheel-artifact-github kvikio 702 cpp
)
KVIKIO_WHEELHOUSE=$(
RAPIDS_PY_WHEEL_NAME="kvikio_${RAPIDS_PY_CUDA_SUFFIX}" rapids-get-pr-wheel-artifact-github kvikio 702 python
)
# write a pip constraints file saying e.g. "whenever you encounter a requirement for 'librmm-cu12', use this wheel"
cat > /tmp/constraints.txt <<EOF
libkvikio-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo ${LIBKVIKIO_WHEELHOUSE}/libkvikio_*.whl)
kvikio-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo ${KVIKIO_WHEELHOUSE}/kvikio_*.whl)
EOF
export PIP_CONSTRAINT=/tmp/constraints.txtThere was a problem hiding this comment.
This doesn't seem to work unless I specify the GITHUB_RUN_ID, at which point I'm doing roughly the same amount of work
Yeah, I'm not sure. The |
1366447 to
6e75249
Compare
|
@bdice -- I think this looks good as confirmation of the upstream vendoring of |
gforsyth
changed the title
libnvcomp from libkvikio wheel to unblock Python 3.13 upgrade
jameslamb
left a comment
jameslamb
left a comment
There was a problem hiding this comment.
Looks good to me! Just left one small comment, it's not even a suggestion... I'd be comfortable merging this as-is.
|
|
||
| # TODO(nvcomp): when `nvcomp` supports Python 3.13 and we de-vendor `nvcomp` from `kvikio` this | ||
| # block should be removed | ||
| if(USE_NVCOMP_FROM_LIBKVIKIO_WHEEL) |
There was a problem hiding this comment.
Just putting this in writing to make it explicit... I think it's ok to allow USE_NVCOMP_RUNTIME_WHEEL and and USE_NVCOMP_FROM_LIBVIKIO_WHEEL to both be ON, as you've done here.
Having both RPATHs set on libcudf.so, even if accidentally, shouldn't be a problem. But just calling out that possibility, to give you an opportunity to say "oh yeah, I hadn't thought of that... ok no that's bad, let's make them mutually exclusive".
There was a problem hiding this comment.
That's a good callout -- I do think that it's ok if they're both set. It should be a temporary addition, in any case.
|
/merge |
3 participants
xref rapidsai/kvikio#702
xref rapidsai/build-planning#120
xref rapidsai/build-planning#171
This PR enables us to update
cudfto use Python 3.13. We were blocked on upgrading because there are nonvcompwheels for Python 3.13, but we've now vendorednvcompintolibkvikio, so we should be able to upgrade that way.I've added a new cmake option so that we look for
libnvcomp.so.4in the right place -- that can be reverted when we switch back to usingnvcompwheels.