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

Improved CPU/GPU interoperability #5001

Merged

Conversation

viclafargue
Copy link
Contributor

No description provided.

raydouglass pushed a commit that referenced this pull request Dec 15, 2022
**Purpose of this PR**

- Provide an implementation of CumlArray that can be backed by either host or device memory
- Hook the host-compatible CumlArray into infrastructure for device and memory type selection
- Ensure that device-host and host-device transfers are minimized for both GPU and CPU execution of existing CPU/GPU interoperable models

**Non-goals of this PR**

- This PR is not intended to provide a version of CUML that can be imported or run on a CPU-only system
- This PR does not allow CUML to be compiled without nvcc
- This PR is not intended to document the CPU/GPU interoperable estimator infrastructure introduced by #4908 and improved in #5001
- This PR is intended *only* to avoid breaking existing functionality. New functionality related to CPU-only execution which is not currently being used in the codebase will be tested in a separate PR

**Tests performed on this PR**
Besides the obvious (standard test suite), benchmarks have been run against this PR for a variety of conversions to and from the new CumlArray. Additionally, the standard cuML benchmark suite was run against this PR and compared to results from current branch-22.12. Results for both are described below.

**Notable features of this PR not immediately related to its purpose**

- Implementing the new CumlArray infrastructure required us to break many of the existing circular imports in the codebase
- New [utilities](https://github.com/wphicks/cuml/blob/fea-xpu_infra/python/cuml/internals/safe_imports.py) for "safe" imports in the context of GPU-only or CPU-only installs were introduced

**Anticipated follow-on PRs**

- #5001
- #4970
- PR(s) to make all CPU/GPU-exclusive imports "safe"
- PR(s) to add additional testing for CPU-only functionality

Authors:
   - William Hicks (https://github.com/wphicks)
   - Victor Lafargue (https://github.com/viclafargue)
   - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
@github-actions github-actions bot removed the CMake label Dec 15, 2022
@viclafargue viclafargue marked this pull request as ready for review December 15, 2022 18:05
@viclafargue viclafargue requested a review from a team as a code owner December 15, 2022 18:05
@dantegd dantegd added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Dec 15, 2022
@dantegd
Copy link
Member

dantegd commented Dec 16, 2022

rerun tests

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (branch-23.02@447bded). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff               @@
##             branch-23.02    #5001   +/-   ##
===============================================
  Coverage                ?   79.24%           
===============================================
  Files                   ?      191           
  Lines                   ?    12374           
  Branches                ?        0           
===============================================
  Hits                    ?     9806           
  Misses                  ?     2568           
  Partials                ?        0           
Flag Coverage Δ
dask 45.79% <0.00%> (?)
non-dask 69.25% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dantegd
Copy link
Member

dantegd commented Dec 17, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit fa4b301 into rapidsai:branch-23.02 Dec 17, 2022
jakirkham pushed a commit to jakirkham/cuml that referenced this pull request Feb 27, 2023
**Purpose of this PR**

- Provide an implementation of CumlArray that can be backed by either host or device memory
- Hook the host-compatible CumlArray into infrastructure for device and memory type selection
- Ensure that device-host and host-device transfers are minimized for both GPU and CPU execution of existing CPU/GPU interoperable models

**Non-goals of this PR**

- This PR is not intended to provide a version of CUML that can be imported or run on a CPU-only system
- This PR does not allow CUML to be compiled without nvcc
- This PR is not intended to document the CPU/GPU interoperable estimator infrastructure introduced by rapidsai#4908 and improved in rapidsai#5001
- This PR is intended *only* to avoid breaking existing functionality. New functionality related to CPU-only execution which is not currently being used in the codebase will be tested in a separate PR

**Tests performed on this PR**
Besides the obvious (standard test suite), benchmarks have been run against this PR for a variety of conversions to and from the new CumlArray. Additionally, the standard cuML benchmark suite was run against this PR and compared to results from current branch-22.12. Results for both are described below.

**Notable features of this PR not immediately related to its purpose**

- Implementing the new CumlArray infrastructure required us to break many of the existing circular imports in the codebase
- New [utilities](https://github.com/wphicks/cuml/blob/fea-xpu_infra/python/cuml/internals/safe_imports.py) for "safe" imports in the context of GPU-only or CPU-only installs were introduced

**Anticipated follow-on PRs**

- rapidsai#5001
- rapidsai#4970
- PR(s) to make all CPU/GPU-exclusive imports "safe"
- PR(s) to add additional testing for CPU-only functionality

Authors:
   - William Hicks (https://github.com/wphicks)
   - Victor Lafargue (https://github.com/viclafargue)
   - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
jakirkham pushed a commit to jakirkham/cuml that referenced this pull request Feb 27, 2023
Authors:
  - Victor Lafargue (https://github.com/viclafargue)
  - William Hicks (https://github.com/wphicks)
  - Corey J. Nolet (https://github.com/cjnolet)
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Carl Simon Adorf (https://github.com/csadorf)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: rapidsai#5001
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants