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

Remove raft/matrix/matrix.cuh includes #1498

Merged
merged 13 commits into from
May 11, 2023

Conversation

benfred
Copy link
Member

@benfred benfred commented May 8, 2023

The raft/matrix/matrix.cuh file has been marked as deprecated, and produces a compile warning when included. However it was still being referenced in a bunch of different spots within raft - making it hard to avoid these warnings.

Remove the includes, in favour of either the newer API's or in certain cases the detail API

The `raft/matrix/matrix.cuh` file has been marked as deprecated, and
produces a compile warning when included. However it was still being
referenced in a bunch of different spots within raft - making it
hard to avoid these warnings.

Remove the includes, in favour of either the newer API's or in certain
cases the detail API
@benfred benfred requested a review from a team as a code owner May 8, 2023 23:43
@benfred benfred marked this pull request as draft May 8, 2023 23:43
@github-actions github-actions bot added the cpp label May 8, 2023
@cjnolet
Copy link
Member

cjnolet commented May 9, 2023

Oh man this makes me so happy. Thanks @benfred for taking this on!

cpp/include/raft/linalg/detail/eig.cuh Outdated Show resolved Hide resolved
@benfred benfred added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 9, 2023
benfred added a commit to benfred/cuml that referenced this pull request May 9, 2023
@benfred benfred marked this pull request as ready for review May 10, 2023 16:22
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

LGTM!

* @param[out] out: output matrix
*/
template <typename m_t, typename matrix_idx_t>
void copy(raft::device_resources const& handle,
Copy link
Member

Choose a reason for hiding this comment

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

wonderful!

@@ -34,7 +34,7 @@ namespace raft::matrix {
* @param[out] matrix: matrix of size n_rows x n_cols
*/
template <typename m_t, typename idx_t, typename layout>
void set_diagonal(raft::device_resources const& handle,
void set_diagonal(raft::resources const& handle,
Copy link
Member

Choose a reason for hiding this comment

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

Yes!

rapids-bot bot pushed a commit to rapidsai/cuml that referenced this pull request May 11, 2023
Required to build after change rapidsai/raft#1498 is merged

Authors:
  - Ben Frederickson (https://github.com/benfred)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #5411
@benfred
Copy link
Member Author

benfred commented May 11, 2023

/merge

@rapids-bot rapids-bot bot merged commit 6b94e4f into rapidsai:branch-23.06 May 11, 2023
@benfred benfred deleted the remove_deprecated_matrix_cuh branch May 11, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

2 participants