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.

Sign up for GitHub

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

Jump to bottom

Fixes for CPU package #5599

Merged
merged 8 commits into from
Oct 4, 2023
Merged

Fixes for CPU package #5599

merged 8 commits into from
Oct 4, 2023

Conversation

dantegd
Copy link
Member

@dantegd dantegd commented Oct 3, 2023

This PR fixes a few issues ran into while testing the different supported algorithms:

  • Add pandas as runtime dependency (which already is of the GPU package through cuDF)
  • Use np.dtype as opposed to cp.dtype, they are the same object in the changes of the PR, and other similar changes to not trigger cudf/cupy/numba calls at import time, which cause runtime issues in some algos.
  • Other small fixes that can cause issues at runtime

@dantegd dantegd requested review from a team as code owners October 3, 2023 14:50
@github-actions github-actions bot added conda conda issue Cython / Python Cython or Python issue labels Oct 3, 2023
@dantegd dantegd added 2 - In Progress Currenty a work in progress bug Something isn't working non-breaking Non-breaking change labels Oct 3, 2023
@dantegd dantegd added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currenty a work in progress labels Oct 4, 2023
wphicks
wphicks approved these changes Oct 4, 2023
View reviewed changes
Copy link
Contributor

@wphicks wphicks left a comment

Choose a reason for hiding this comment

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

Looks great! Had one question, but it does not affect correctness, so pre-approving.

python/cuml/internals/input_utils.py
_input_type_to_str[CudfSeries] = "cudf"
_input_type_to_str[CudfDataFrame] = "cudf"
_input_type_to_str[NumbaDeviceNDArrayBase] = "numba"
except UnavailableError:
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this exception ever be thrown? I don't think we're doing anything non-trivial with these types.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, it gets thrown on at import time by just trying to create the dictionary entries with keys of GPU packages

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, of course! It needs to check hashability. Hmmm... That's something we might want to tweak in the future.

@dantegd
Copy link
Member Author

dantegd commented Oct 4, 2023

/merge

@rapids-bot rapids-bot bot merged commit cc22891 into rapidsai:branch-23.10 Oct 4, 2023
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wphicks wphicks wphicks approved these changes

@raydouglass raydouglass raydouglass approved these changes

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team bug Something isn't working conda conda issue Cython / Python Cython or Python issue non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dantegd @raydouglass @wphicks