Merge pull request #1070 from van51/develop

Update on #1042. Fixed order of returned kernels
shogun-toolbox · May 8, 2013 · 2a8b6f1 · 2a8b6f1
2 parents 3cd0318 + 9e2afda
commit 2a8b6f1
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 15 deletions.
diff --git 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
@@ -945,13 +945,17 @@ CList* CCombinedKernel::combine_kernels(CList* kernel_list)
 	while (list)
 	{
 		CList* c_list= dynamic_cast<CList* >(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<CCombinedKernel* >(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
@@ -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
@@ -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<CCombinedKernel* >(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<CCombinedKernel* >(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<CGaussianKernel* >(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<CCombinedKernel* >(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<CGaussianKernel* >(subkernel);
 			EXPECT_EQ(c_subkernel->get_width(), combs[i++][j]);