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

remove LibSVM, LibLinear, LibSVR from SWIG #4494

Merged
merged 1 commit into from
Feb 1, 2019
Merged

remove LibSVM, LibLinear, LibSVR from SWIG #4494

merged 1 commit into from
Feb 1, 2019

Conversation

karlnapf
Copy link
Member

#4463
@gf712 @avramidis @vinx13 @shubham808 you get the point ... we need to start pumping those out

@karlnapf karlnapf force-pushed the feature/remove_libsvm branch from 6d2bb2b to d1289fe Compare January 29, 2019 20:06
@karlnapf
Copy link
Member Author

This error was a weird one, these kind of fixes (see relaxed tree class) we need to do when doing the transition

karlnapf
karlnapf commented Jan 29, 2019
View reviewed changes
src/shogun/multiclass/tree/RelaxedTree.cpp
@@ -141,7 +141,8 @@ bool CRelaxedTree::train_machine(CFeatures* data)
CMulticlassLabels *lab = dynamic_cast<CMulticlassLabels *>(m_labels);

RelaxedTreeUtil util;
SGMatrix<float64_t> conf_mat = util.estimate_confusion_matrix(m_machine_for_confusion_matrix,
SGMatrix<float64_t> conf_mat = util.estimate_confusion_matrix(
m_machine_for_confusion_matrix->as<CBaseMulticlassMachine>(),
Copy link
Member Author

Choose a reason for hiding this comment

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

from compile time safety to runtime safety ... :/

Copy link
Member Author

Choose a reason for hiding this comment

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

we could also add the CBaseMulticlassMachine to the shogun base classes, but I am not sure what we get API wise?
@lisitsyn you wrote that, you have any opinions?

@karlnapf karlnapf force-pushed the feature/remove_libsvm branch from d1289fe to 0ba29fe Compare January 30, 2019 10:59
karlnapf
karlnapf commented Jan 30, 2019
View reviewed changes
examples/undocumented/python/classifier_multilabeloutputliblinear.py

label_pred = classifier.apply_multilabel_output(feats_test,2)
out = label_pred.get_labels()
# TODO: figure out the new style API for the below call, disabling for now
Copy link
Member Author

Choose a reason for hiding this comment

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

@lisitsyn pls comment on API suggestions for this part, as your wrote the thing IIRC :)

karlnapf
karlnapf commented Jan 30, 2019
View reviewed changes
src/shogun/machine/KernelMulticlassMachine.cpp
@@ -32,7 +32,7 @@ void CKernelMulticlassMachine::store_model_features()
CSet<index_t> all_sv;
for (index_t i=0; i<m_machines->get_num_elements(); ++i)
{
CKernelMachine *machine=(CKernelMachine *)get_machine(i);
auto machine=get_machine(i)->as<CKernelMachine>();
Copy link
Member Author

Choose a reason for hiding this comment

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

@lisitsyn also would appreciate your input on the changes in this file

@karlnapf karlnapf force-pushed the feature/remove_libsvm branch 4 times, most recently from 89a454c to 63da846 Compare January 30, 2019 14:39
@karlnapf
Copy link
Member Author

For now postponing the removal of LibLinear from SWIG due to enum problems

@karlnapf karlnapf force-pushed the feature/remove_libsvm branch 19 times, most recently from 1b9a1e7 to 4cef4d1 Compare January 31, 2019 13:15
@karlnapf karlnapf force-pushed the feature/remove_libsvm branch 17 times, most recently from 64f7799 to 45ca9cc Compare January 31, 2019 17:06
@karlnapf karlnapf mentioned this pull request Jan 31, 2019
Open
@karlnapf
Copy link
Member Author

ok donbot now is happy for python legacy and meta...fingers crossed

@karlnapf karlnapf force-pushed the feature/remove_libsvm branch 2 times, most recently from 032307f to 07b0bf1 Compare January 31, 2019 17:16
karlnapf
karlnapf commented Jan 31, 2019
View reviewed changes
examples/meta/generator/translate.py
@@ -412,6 +412,12 @@ def getIncludePathForClass(self, type_):
uniqueCandidates[0]))
return uniqueCandidates[0]

# hack since ctags cannot find include paths for preprocessor
Copy link
Member Author

Choose a reason for hiding this comment

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

@sorig @lisitsyn any ideas how to make ctags read function names constructed using the double ## in preprocessor, see here ?

karlnapf
karlnapf commented Jan 31, 2019
View reviewed changes
src/shogun/util/factory.h
@@ -285,10 +288,5 @@ namespace shogun
SG_REF(result);
return result;
}

CMachine* machine(CPipeline* pipeline)
Copy link
Member Author

Choose a reason for hiding this comment

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

@vinx13 I removed this as it is not really needed, one can use machine (I renamed to as_machine in this PR)

@karlnapf karlnapf force-pushed the feature/remove_libsvm branch from 07b0bf1 to 4a65d73 Compare February 1, 2019 14:17
@karlnapf
Copy link
Member Author

karlnapf commented Feb 1, 2019

omg finally

@karlnapf karlnapf merged commit e42a642 into shogun-toolbox:develop Feb 1, 2019
@karlnapf karlnapf deleted the feature/remove_libsvm branch February 1, 2019 15:44
vigsterkr pushed a commit to vigsterkr/shogun that referenced this pull request Mar 9, 2019
@karlnapf @vigsterkr
remove LibSVM, LibLinear, LibSVR from SWIG (shogun-toolbox#4494)
d601184
ktiefe pushed a commit to ktiefe/shogun that referenced this pull request Jul 30, 2019
@karlnapf @ktiefe
remove LibSVM, LibLinear, LibSVR from SWIG (shogun-toolbox#4494)
b1cb4d4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@karlnapf