From 9e2afdad9387da2e7f13e8771bbbe4129a906c03 Mon Sep 17 00:00:00 2001 From: van51 Date: Wed, 8 May 2013 16:17:11 +0300 Subject: [PATCH] Update on #1042. Fixed order of returned kernels --- ...lection_combined_kernel_sub_parameters.cpp | 8 +++---- src/shogun/kernel/CombinedKernel.cpp | 12 ++++++++-- src/shogun/kernel/CombinedKernel.h | 1 + tests/unit/kernel/CombinedKernel_unittest.cc | 24 ++++++++++++------- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/examples/undocumented/libshogun/modelselection_combined_kernel_sub_parameters.cpp b/examples/undocumented/libshogun/modelselection_combined_kernel_sub_parameters.cpp index 4bcfa5f588e..a91fc72fb2b 100644 --- a/examples/undocumented/libshogun/modelselection_combined_kernel_sub_parameters.cpp +++ b/examples/undocumented/libshogun/modelselection_combined_kernel_sub_parameters.cpp @@ -64,8 +64,8 @@ CModelSelectionParameters* build_combined_kernel_parameter_tree() { /* print out current kernel's subkernels */ SG_SPRINT("combined kernel %d:\n", i++); - CPolyKernel* poly=(CPolyKernel*)current->get_kernel(0); - CGaussianKernel* gaussian=(CGaussianKernel*)current->get_kernel(1); + CGaussianKernel* gaussian=(CGaussianKernel*)current->get_kernel(0); + CPolyKernel* poly=(CPolyKernel*)current->get_kernel(1); SG_SPRINT("kernel_a type: %s\n", poly->get_name()); SG_SPRINT("kernel_b type: %s\n", gaussian->get_name()); SG_SPRINT("kernel_a parameter: %d\n", poly->get_degree()); @@ -143,8 +143,8 @@ void modelselection_combined_kernel() /* print subkernel parameters, I know what the subkernel types are here */ CCombinedKernel* kernel=(CCombinedKernel*)classifier->get_kernel(); - CGaussianKernel* gaussian=(CGaussianKernel*)kernel->get_kernel(1); - CPolyKernel* poly=(CPolyKernel*)kernel->get_kernel(0); + CGaussianKernel* gaussian=(CGaussianKernel*)kernel->get_kernel(0); + CPolyKernel* poly=(CPolyKernel*)kernel->get_kernel(1); SG_SPRINT("gaussian width: %f\n", gaussian->get_width()); SG_SPRINT("poly degree: %d\n", poly->get_degree()); SG_UNREF(kernel); diff --git a/src/shogun/kernel/CombinedKernel.cpp b/src/shogun/kernel/CombinedKernel.cpp index 44aa245c1cf..d0c7db3c1d6 100644 --- a/src/shogun/kernel/CombinedKernel.cpp +++ b/src/shogun/kernel/CombinedKernel.cpp @@ -945,13 +945,17 @@ CList* CCombinedKernel::combine_kernels(CList* kernel_list) while (list) { CList* c_list= dynamic_cast(list); - if (!c_list) + if (!c_list) + { SG_SERROR("CCombinedKernel::combine_kernels() : Failed to cast list of type " "%s to type CList\n", list->get_name()); + } if (c_list->get_num_elements()==0) + { SG_SERROR("CCombinedKernel::combine_kernels() : Sub-list in position %d " "is empty.\n", list_index); + } num_combinations *= c_list->get_num_elements(); @@ -990,8 +994,10 @@ CList* CCombinedKernel::combine_kernels(CList* kernel_list) if (first_kernel) first_kernel = false; else if (c_kernel->get_kernel_type()!=prev_kernel_type) + { SG_SERROR("CCombinedKernel::combine_kernels() : Sub-list in position " "0 contains different types of kernels\n"); + } prev_kernel_type = c_kernel->get_kernel_type(); @@ -1032,8 +1038,10 @@ CList* CCombinedKernel::combine_kernels(CList* kernel_list) if (first_kernel) first_kernel = false; else if (c_kernel->get_kernel_type()!=prev_kernel_type) + { SG_SERROR("CCombinedKernel::combine_kernels() : Sub-list in position " "%d contains different types of kernels\n", list_index); + } prev_kernel_type = c_kernel->get_kernel_type(); @@ -1045,7 +1053,7 @@ CList* CCombinedKernel::combine_kernels(CList* kernel_list) { CCombinedKernel* comb_kernel = dynamic_cast(kernel_array.get_element(base+index)); - comb_kernel->insert_kernel(c_kernel); + comb_kernel->append_kernel(c_kernel); SG_UNREF(comb_kernel); } } diff --git a/src/shogun/kernel/CombinedKernel.h b/src/shogun/kernel/CombinedKernel.h index 66f7b014cf5..0fc26e9bf5f 100644 --- a/src/shogun/kernel/CombinedKernel.h +++ b/src/shogun/kernel/CombinedKernel.h @@ -403,6 +403,7 @@ class CCombinedKernel : public CKernel inline CList* get_list() {SG_REF(kernel_list); return kernel_list;} /** Returns a list of all the different CombinedKernels produced by the cross-product between the kernel lists + * The returned list performs reference counting on the contained CombinedKernels. * * @param kernel_list a list of lists of kernels. Each sub-list must contain kernels of the same type * diff --git a/tests/unit/kernel/CombinedKernel_unittest.cc b/tests/unit/kernel/CombinedKernel_unittest.cc index cf9fbaed7f6..a0070cd175d 100644 --- a/tests/unit/kernel/CombinedKernel_unittest.cc +++ b/tests/unit/kernel/CombinedKernel_unittest.cc @@ -98,7 +98,8 @@ TEST(CombinedKernelTest,combination) combined_list = CCombinedKernel::combine_kernels(kernel_list); index_t i = 0; - for (CSGObject* kernel=combined_list->get_first_element(); kernel; kernel=combined_list->get_next_element()) + for (CSGObject* kernel=combined_list->get_first_element(); kernel; + kernel=combined_list->get_next_element()) { CCombinedKernel* c_kernel = dynamic_cast(kernel); CSGObject* subkernel = c_kernel->get_first_kernel(); @@ -119,18 +120,20 @@ TEST(CombinedKernelTest,combination) sub_list_2->append_element(ck5); kernel_list->append_element(sub_list_2); - float64_t combs2[2][6] = {{ 21, 21, 21, 31, 31, 31}, - { 3, 5, 7, 3, 5, 7}}; + float64_t combs2[2][6] = {{ 3, 5, 7, 3, 5, 7}, + { 21, 21, 21, 31, 31, 31}}; combined_list = CCombinedKernel::combine_kernels(kernel_list); index_t j = 0; - for (CSGObject* kernel=combined_list->get_first_element(); kernel; kernel=combined_list->get_next_element()) + for (CSGObject* kernel=combined_list->get_first_element(); kernel; + kernel=combined_list->get_next_element()) { CCombinedKernel* c_kernel = dynamic_cast(kernel); EXPECT_EQ(c_kernel->get_num_subkernels(), 2); i = 0; - for (CSGObject* subkernel=c_kernel->get_first_kernel(); subkernel; subkernel=c_kernel->get_next_kernel()) + for (CSGObject* subkernel=c_kernel->get_first_kernel(); subkernel; + subkernel=c_kernel->get_next_kernel()) { CGaussianKernel* c_subkernel = dynamic_cast(subkernel); EXPECT_EQ(c_subkernel->get_width(), combs2[i++][j]); @@ -154,19 +157,22 @@ TEST(CombinedKernelTest,combination) kernel_list->append_element(sub_list_3); float64_t combs[3][24] = { - { 109, 109, 109, 109, 109, 109, 203, 203, 203, 203, 203, 203, 308, 308, 308, 308, 308, 308, 404, 404, 404, 404, 404, 404}, + { 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7}, { 21, 21, 21, 31, 31, 31, 21, 21, 21, 31, 31, 31, 21, 21, 21, 31, 31, 31, 21, 21, 21, 31, 31, 31}, - { 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7, 3, 5, 7}}; + { 109, 109, 109, 109, 109, 109, 203, 203, 203, 203, 203, 203, 308, 308, 308, 308, 308, 308, 404, 404, 404, 404, 404, 404} + }; combined_list = CCombinedKernel::combine_kernels(kernel_list); j = 0; - for (CSGObject* kernel=combined_list->get_first_element(); kernel; kernel=combined_list->get_next_element()) + for (CSGObject* kernel=combined_list->get_first_element(); kernel; + kernel=combined_list->get_next_element()) { CCombinedKernel* c_kernel = dynamic_cast(kernel); i = 0; EXPECT_EQ(c_kernel->get_num_subkernels(), 3); - for (CSGObject* subkernel=c_kernel->get_first_kernel(); subkernel; subkernel=c_kernel->get_next_kernel()) + for (CSGObject* subkernel=c_kernel->get_first_kernel(); subkernel; + subkernel=c_kernel->get_next_kernel()) { CGaussianKernel* c_subkernel = dynamic_cast(subkernel); EXPECT_EQ(c_subkernel->get_width(), combs[i++][j]);