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

Support Vector Machine #912

Merged
merged 103 commits into from
Sep 27, 2019
Merged

Support Vector Machine #912

merged 103 commits into from
Sep 27, 2019

Conversation

tfeher
Copy link
Contributor

@tfeher tfeher commented Jul 31, 2019

This branch adds a Support Vector Machine Classifier (C-SVC).

We use prims from PRs #892 and #853, reviewers please focus here on the SVM files in the cpp/src/svm directory.

The Sequential Minimal Optimization method is used to fit the SVM, similarily to ThunderSVM or OHD-SVM.

Currently only binary classification is implemented.

@tfeher
Copy link
Contributor Author

tfeher commented Jul 31, 2019

@teju85 and @cjnolet: originally I had an Sklearn like stateful cpp interface #456 in svc.h, and the python bindings are using that for convenience. I have added a stateless layer afterwards. Currently everything is linked into libcuml++.so. Do we have any update on how we should separate the stateless and stateful API?

@dantegd dantegd added 3 - Ready for Review Ready for review by team New Algorithm For tracking new algorithms that will be added to our existing collection labels Aug 1, 2019
@cjnolet cjnolet added the CUDA / C++ CUDA issue label Aug 4, 2019
@tfeher
Copy link
Contributor Author

tfeher commented Sep 20, 2019

Thanks @cjnolet for the review! I will address the issues with the failing test.

Copy link
Contributor

@oyilmaz-nvidia oyilmaz-nvidia left a comment

Choose a reason for hiding this comment

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

Left couple of comments for mostly code structure. Overall, very well written. Congrats.

cpp/src/svm/kernelcache.h Outdated Show resolved Hide resolved
cpp/src/svm/kernelcache.h Outdated Show resolved Hide resolved
cpp/src/svm/results.h Show resolved Hide resolved
cpp/src/svm/results.h Show resolved Hide resolved
cpp/src/svm/smoblocksolve.h Show resolved Hide resolved
cpp/src/svm/ws_util.cu Outdated Show resolved Hide resolved
cpp/src/svm/ws_util.cu Show resolved Hide resolved
cpp/src/svm/ws_util.cu Show resolved Hide resolved
cpp/src/svm/ws_util.h Show resolved Hide resolved
cpp/src/svm/ws_util.h Show resolved Hide resolved
Copy link
Contributor Author

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks Onur for the review! I have addressed what I could now, and created new issues for those that would prefer to postpone after 0.10.

cpp/src/svm/ws_util.h Show resolved Hide resolved
cpp/src/svm/ws_util.h Show resolved Hide resolved
cpp/src/svm/ws_util.cu Show resolved Hide resolved
cpp/src/svm/ws_util.cu Show resolved Hide resolved
cpp/src/svm/results.h Show resolved Hide resolved
cpp/src/svm/kernelcache.h Outdated Show resolved Hide resolved
cpp/src/svm/smosolver.h Show resolved Hide resolved
cpp/src/svm/ws_util.cu Outdated Show resolved Hide resolved
cpp/src/svm/kernelcache.h Outdated Show resolved Hide resolved
cpp/src/svm/smoblocksolve.h Show resolved Hide resolved
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.

Just a small change to use RMM

python/cuml/utils/numba_utils.py Show resolved Hide resolved
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.

Needed an additional comment, so treat both of my reviews as a single one :P

python/cuml/utils/input_utils.py Outdated Show resolved Hide resolved
@tfeher
Copy link
Contributor Author

tfeher commented Sep 27, 2019

Hi @dantegd , thanks for the mini-review! I have addressed the issues, could you have a look?

@JohnZed JohnZed merged commit 2d36f14 into rapidsai:branch-0.10 Sep 27, 2019
jakirkham pushed a commit to jakirkham/cuml that referenced this pull request Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team CUDA / C++ CUDA issue Cython / Python Cython or Python issue New Algorithm For tracking new algorithms that will be added to our existing collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants