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

[REVIEW] Moving linalg decomp to RAFT namespaces #2906

Merged
merged 11 commits into from
Oct 19, 2020

Conversation

divyegala
Copy link
Member

@divyegala divyegala commented Oct 2, 2020

This moves the following:

  1. linalg/eig.cuh
  2. linalg/svd.cuh
  3. linalg/qr.cuh

Major changes involve API change (adding a handle param to the function signature, and removing cublasHandle_t, allocator, etc.). This change is performed in all files mentioned above

@divyegala divyegala requested a review from a team as a code owner October 2, 2020 07:24
@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@divyegala divyegala changed the base branch from branch-0.16 to branch-0.17 October 15, 2020 17:54
@codecov-io
Copy link

codecov-io commented Oct 15, 2020

Codecov Report

Merging #2906 into branch-0.17 will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.17    #2906      +/-   ##
===============================================
+ Coverage        58.86%   58.87%   +0.01%     
===============================================
  Files              143      143              
  Lines             9004     9004              
===============================================
+ Hits              5300     5301       +1     
+ Misses            3704     3703       -1     
Impacted Files Coverage Δ
...l/_thirdparty/sklearn/preprocessing/_imputation.py 62.50% <0.00%> (+0.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1295202...9dc2b36. Read the comment docs.

Copy link
Contributor

@drobison00 drobison00 left a comment

Choose a reason for hiding this comment

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

Minor updates, and a couple formatting questions; other than that LGTM.

cpp/src_prims/functions/linearReg.cuh Outdated Show resolved Hide resolved
cpp/src_prims/linalg/qr.cuh Outdated Show resolved Hide resolved
cpp/src_prims/linalg/svd.cuh Show resolved Hide resolved
cpp/test/prims/rsvd.cu Show resolved Hide resolved
@drobison00
Copy link
Contributor

Looks good!

Copy link
Member

@dantegd dantegd left a comment

Choose a reason for hiding this comment

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

approval needed from cpp-codeowners

@dantegd dantegd added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Oct 19, 2020
@dantegd dantegd merged commit afdb6c0 into rapidsai:branch-0.17 Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants