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

Fix/cudf api updates #1044

Merged
merged 7 commits into from
Feb 11, 2022
Merged

Conversation

AjayThorve
Copy link
Contributor

@AjayThorve AjayThorve commented Feb 4, 2022

This PR replaces deprecated methods in cudf, related to conversion to a cupy array from cudf.DataFrame and cudf.Series. The new methods also provide performance improvements as demonstrated here: https://docs.rapids.ai/api/cudf/nightly/user_guide/10min-cudf-cupy.html#Converting-a-cuDF-Series-to-a-CuPy-Array

Finally, also updating the minimum version of cudf's last stable release (22.02)

(tested locally with the latest rapids libraries)

@jbednar
Copy link
Member

jbednar commented Feb 7, 2022

Thanks! However, it doesn't seem safe to merge this, because it requires cudf version 22.02.00 released only five days ago, and thus releasing datashader with these changes would mean people could use only the very latest cudf, which isn't always practical given the driver versions people have installed.

Since the updates are to address deprecated API, is there a minimum version of cudf supporting this API that isn't quite so new? Otherwise we'll have to park this PR and revisit it in the following rather than the upcoming Datashader release.

@AjayThorve
Copy link
Contributor Author

AjayThorve commented Feb 8, 2022

@jbednar, that makes sense.

So the minimum version the new api supports is 21.12, which was released a couple of months ago (Dec 3, 2021). How old a version of cudf do you think would be practical to merge? Another option could be using api on the current installed version of cudf, for e.g: 22.04 to be released will have removed the older version, so we can just check the user installed version and use the corresponding api:

if cudf_version >= Version("22.02"):
    return df.to_cupy()
else:
    return df.as_gpu_matrix()

Does the above work?

@AjayThorve
Copy link
Contributor Author

I have pushed some changes, and reverted the minimum cudf version update (although the recommended version would always be the latest stable release)

@AjayThorve AjayThorve force-pushed the fix/cudf-api-updates branch from 6dd5f91 to 7a77609 Compare February 9, 2022 02:29
@philippjfr
Copy link
Member

Thanks @AjayThorve! The December release is still pretty recent, what's the driver compatibility for cuDF 21.12? I guess all of the recent versions require CUDA 11.x + Driver v450.80.02+? In that case I'm okay with this. Do we need to make similar updates in HoloViews?

datashader/glyphs/points.py Outdated Show resolved Hide resolved
datashader/reductions.py Outdated Show resolved Hide resolved
@AjayThorve AjayThorve force-pushed the fix/cudf-api-updates branch from 1fcdf00 to e0b9a70 Compare February 9, 2022 20:59
@AjayThorve
Copy link
Contributor Author

what's the driver compatibility for cuDF 21.12? I guess all of the recent versions require CUDA 11.x + Driver v450.80.02+?

yes, CUDA 11.x + Driver v450.80.02+ since atleast the June,2020 release.

Do we need to make similar updates in HoloViews?

I am open to helping out replace all the deprecated cudf code across holoviews projects, I think hvplot uses it?

@philippjfr
Copy link
Member

Okay great, I'm happy to see this merged!

hvPlot indeed uses HoloViews to generate the plots, help updating it would be hugely appreciated!

@philippjfr philippjfr merged commit 7fa1b34 into holoviz:master Feb 11, 2022
AjayThorve added a commit to AjayThorve/datashader that referenced this pull request Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants