Updated cross validation examples to use new API (as far as I know) #4309

FaroukY · 2018-05-26T03:23:21Z

No description provided.

iglesias

Follow-up whether the suggestions are possible. If not, we need to know clearly why.

iglesias · 2018-05-26T12:21:01Z

examples/meta/src/evaluation/cross_validation_multiple_kernel_learning_weights_storage.sg

-CSVFile f_feats("../../data/fm_train_real.dat")
-CSVFile f_labels("../../data/label_train_twoclass.dat")
+File f_feats = csv_file("../../data/fm_train_real.dat")
+File f_labels = csv_file("../../data/label_train_twoclass.dat")

 #![create_features]
 Features feats = features(f_feats)


Can you do the CombinedFeatures and BinaryLabels below with the factories?

iglesias · 2018-05-26T12:21:23Z

examples/meta/src/evaluation/cross_validation_support_vector_machine.sg

-CSVFile f_feats("../../data/fm_train_real.dat")
-CSVFile f_labels("../../data/label_train_twoclass.dat")
+File f_feats = csv_file("../../data/fm_train_real.dat")
+File f_labels = csv_file("../../data/label_train_twoclass.dat")

 Features feats = features(f_feats)
 BinaryLabels labels(f_labels)


@iglesias There is a problem with StratifiedCrossValidationSplitting class. If you update to the new API, it leads to an error. Viktor told me he needs to discuss this with Heiko first since it is a design problem, so for now any label which is used with StratifiedCrossValidationSplitting has to use the old API (BinaryLabels)

I changed this in #4313 to illustrate. You will have to rebase

iglesias · 2018-05-26T12:23:40Z

examples/meta/src/evaluation/cross_validation_multiple_kernel_learning_weights_storage.sg

@@ -52,10 +52,10 @@ real stddev = result.get_real("std_dev")
 #![get_fold_machine]
 CrossValidationStorage obs = mkl_obs.get_observation(0)
 CrossValidationFoldStorage fold = obs.get_fold(0)
-MKLClassification machine = MKLClassification:obtain_from_generic(fold.get_trained_machine())
+MKLClassification machine_mkl = MKLClassification:obtain_from_generic(fold.get_trained_machine())


Getter instead of obtain_from_generic (ofg)?

For some reason, I tried the getter but it told me that the object is private. I will try again and see if I can do it

iglesias · 2018-05-26T12:23:55Z

examples/meta/src/evaluation/cross_validation_multiple_kernel_learning_weights_storage.sg

 #![get_fold_machine]

 #![get_weights]
-CombinedKernel k = CombinedKernel:obtain_from_generic(machine.get_kernel())
+CombinedKernel k = CombinedKernel:obtain_from_generic(machine_mkl.get_kernel())


Getter in place of ofg?

Same problem as above, tried the getter, but had problems with it. Will give it another go

FaroukY · 2018-05-29T13:20:42Z

@iglesias once travis is green, this should be mergable right?

vigsterkr

the reasons of failures

vigsterkr · 2018-05-29T14:09:27Z

examples/meta/src/evaluation/cross_validation_multiple_kernel_learning_weights_storage.sg

 #![get_fold_machine]

 #![get_weights]
-CombinedKernel k = CombinedKernel:obtain_from_generic(machine.get_kernel())
+CombinedKernel k = kernel(machine_mkl.get("kernel"))


shouldnt this be simply

Kernel k = kernel(machine_mkl.get("kernel"))

vigsterkr · 2018-05-29T14:14:51Z

examples/meta/src/evaluation/cross_validation_support_vector_machine.sg

 CrossValidation cross(svm, feats, labels, splitting_strategy, evaluation_criterion)
 cross.set_num_runs(2)

-CrossValidationResult result = CrossValidationResult:obtain_from_generic(cross.evaluate())
+CrossValidationResult result = cross.evaluate()


this assumes that one could explicitly cast CEvaluationResult to CrossValidationResult, which is not the case unfortunately.

for the time being there's no solution for this other than the one you had before:

CrossValidationResult result = CrossValidationResult:obtain_from_generic(cross.evaluate())

EvalidationResult result = cross.evaluate() should work

FaroukY · 2018-05-29T20:20:38Z

@vigsterkr Travis is green :)

iglesias · 2018-05-30T11:34:12Z

@FaroukY, still waiting here for you to get back to the review.

…

On 29 May 2018 at 22:20, Elfarouk Yasser ***@***.***> wrote: @vigsterkr <https://github.com/vigsterkr> Travis is green :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4309 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABGrdvr9wu3ObylGP03ZrOdMg0qfHhAiks5t3a2fgaJpZM4UOwrB> .

karlnapf · 2018-05-30T13:48:26Z

There is a conflict, you will have to rebase

karlnapf · 2018-05-30T13:49:11Z

examples/meta/src/evaluation/cross_validation_multiple_kernel_learning_weights_storage.sg

-svm.set_kernel(kernel)
+Machine libsvm = machine("LibSVM")
+Machine svm = machine("MKLClassification", kernel=combined_kernel)
+svm.put("interleaved_optimization", False)


as said many many times before, pls put this into the kwargs
Machine svm = machine("MKLClassification", kernel=combined_kernel, interleaved_optimization=False, svm=libsvm)

vigsterkr · 2018-05-30T13:49:14Z

@karlnapf currently discussing this already on #irc thnx

karlnapf · 2018-05-30T13:51:06Z

Most changes look good. Rebase and let's merge

…s to new API

FaroukY · 2018-05-30T18:00:42Z

Changed all labels to new API now. this should address everything. Let's wait for Travis and rebase.

vigsterkr

make sure that things are tested locally prior sending in an update :)

vigsterkr · 2018-05-30T20:23:04Z

examples/meta/src/evaluation/cross_validation_support_vector_machine.sg

-CrossValidation cross(svm, feats, labs, splitting_strategy, evaluation_criterion)
+StratifiedCrossValidationSplitting splitting_strategy(labels, 5)
+Evaluation evaluation_criterion = evaluation("AccuracyMeasure")
+CrossValidation cross(svm, feats, labels, splitting_strategy, evaluation_criterion)


labs? please rename labs to labels otherwise this will fail... as it fails now

vigsterkr · 2018-05-31T20:16:15Z

this pr is mergable... its only that gcc actually times out... i've just restarted the job but this should be fine to merge!

karlnapf · 2018-06-01T09:08:02Z

examples/meta/src/evaluation/cross_validation_support_vector_machine.sg

@@ -8,11 +8,11 @@ Machine svm = machine("LibLinear")
 svm.put("liblinear_solver_type", enum LIBLINEAR_SOLVER_TYPE.L2R_L2LOSS_SVC)

 StratifiedCrossValidationSplitting splitting_strategy(labs, 5)
-AccuracyMeasure evaluation_criterion()
+Evaluation evaluation_criterion = evaluation("AccuracyMeasure")
 CrossValidation cross(svm, feats, labs, splitting_strategy, evaluation_criterion)
 cross.set_num_runs(2)

 CrossValidationResult result = CrossValidationResult:obtain_from_generic(cross.evaluate())


EvaluationResult result = cross.evaluate() didnt work?

my mistake, just testing the patch and pushing once done.

iglesias reviewed May 26, 2018

View reviewed changes

FaroukY force-pushed the evaluation_new_api branch from d4a5cb7 to 456cc0d Compare May 29, 2018 13:01

vigsterkr requested changes May 29, 2018

View reviewed changes

karlnapf reviewed May 30, 2018

View reviewed changes

FaroukY added 5 commits May 30, 2018 16:56

Updated cross validation examples to use new API (as far as I know)

81c2369

Chagned more files to new API

7d0fcff

user getter + factory instead of obtain from generic

dcf83b8

fixed the mini review

0fa5e20

added arguments in kwargs instread of using .put(), and changed label…

da01ef0

…s to new API

FaroukY force-pushed the evaluation_new_api branch from 0a2d37c to da01ef0 Compare May 30, 2018 17:59

vigsterkr requested changes May 30, 2018

View reviewed changes

changes labels to labs to get rid of the thrown error

8e1d21d

vigsterkr approved these changes May 31, 2018

View reviewed changes

vigsterkr merged commit e1f02a5 into shogun-toolbox:develop Jun 1, 2018

karlnapf reviewed Jun 1, 2018

View reviewed changes

FaroukY deleted the evaluation_new_api branch June 2, 2018 04:43

ktiefe pushed a commit to ktiefe/shogun that referenced this pull request Jul 30, 2019

Updated cross validation examples to use new API (shogun-toolbox#4309)

fb5b408

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated cross validation examples to use new API (as far as I know) #4309

Updated cross validation examples to use new API (as far as I know) #4309

FaroukY commented May 26, 2018

iglesias left a comment

iglesias May 26, 2018

iglesias May 26, 2018

FaroukY May 27, 2018

karlnapf May 30, 2018

iglesias May 26, 2018

FaroukY May 27, 2018

iglesias May 26, 2018

FaroukY May 27, 2018

FaroukY commented May 29, 2018

vigsterkr left a comment

vigsterkr May 29, 2018

vigsterkr May 29, 2018

karlnapf May 31, 2018

FaroukY commented May 29, 2018

iglesias commented May 30, 2018 via email

karlnapf commented May 30, 2018

karlnapf May 30, 2018 •

edited

vigsterkr commented May 30, 2018

karlnapf commented May 30, 2018

FaroukY commented May 30, 2018

vigsterkr left a comment

vigsterkr May 30, 2018

vigsterkr commented May 31, 2018

karlnapf Jun 1, 2018

vigsterkr Jun 1, 2018

Updated cross validation examples to use new API (as far as I know) #4309

Updated cross validation examples to use new API (as far as I know) #4309

Conversation

FaroukY commented May 26, 2018

iglesias left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FaroukY commented May 29, 2018

vigsterkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FaroukY commented May 29, 2018

iglesias commented May 30, 2018 via email

karlnapf commented May 30, 2018

karlnapf May 30, 2018 • edited

Choose a reason for hiding this comment

vigsterkr commented May 30, 2018

karlnapf commented May 30, 2018

FaroukY commented May 30, 2018

vigsterkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vigsterkr commented May 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlnapf May 30, 2018 •

edited