-
Notifications
You must be signed in to change notification settings - Fork 309
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
Fixes for pandas 2, latest cudf, and wheel building #4144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me. We’ll want to get this merged soon to unblock CUDA 12.2 work on branch-24.04.
It looks like the ARM builds are failing. Do we know what is going on there? |
Both ARM and x86-64 builds are failing. cuGraph is relying on functions from libcudf that were deprecated in 24.02 and removed in 24.04: rapidsai/cudf#14848 |
Yeah I started a Slack thread to get help on those fixes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for taking care of this!
@@ -11,6 +11,11 @@ python_package_name=$(echo ${package_name}|sed 's/-/_/g') | |||
mkdir -p ./dist | |||
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" | |||
|
|||
# Download wheels built during this job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this. It's unfortunate that we have to do it this way vs. something closer to what we do for conda testing (add a channel pointing to the local builds). I haven't verified one way or the other, but I'm curious if we have similar problems in our other wheel test environments, seems like we would.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I set up wheels originally I did this for every package that needs it. I know cugraph added a bunch of wheels afterwards, so it's probably worth an audit.
I don't think this is all that different from the conda case. The main difference is that there are multiple install lines. That was initially there to avoid needing to use --pre
to force installation of pre-releases, but actually with the newer versioning strategies that I put in place in wheels that probably won't be an issue any more, i.e. it's untested but we ought to be able to use a single install command here now.
/merge |
This PR contains a number of different fixes currently required to get cugraph tests passing:
DataFrame.applymap
toDataFrame.map
so creating the renumbering map with a columnmap
caused problems for attribute-based column accessrenumber_map.map
. Those columns are now renamed torenumber_map
.This code also includes a small patch to the cugraph_etl CMake so that it exports the correct package name (previously it was using cugraph).