Skip to content

Commit

Permalink
Fix issue #3612 (#4127)
Browse files Browse the repository at this point in the history 
Fix issue #3612 
adds unit test functions for combined kernel subsetting behaviour. Fixes an issue in which combined kernels would not subset custom kernels if features other than combinedfeatures were passed
The latter is achieved by replacing the previous recursive call with a call to a helper function made aware of any subsets contained.
  • Loading branch information
@dhelekal @vigsterkr
dhelekal authored and vigsterkr committed Jan 29, 2018
1 parent c4e720c commit eed22ff
Show file tree
Hide file tree
Showing 3 changed files with 300 additions and 60 deletions.
207 changes: 149 additions & 58 deletions src/shogun/kernel/CombinedKernel.cpp
View file
Expand Up @@ -61,54 +61,37 @@ void CCombinedKernel::init_subkernel_weights()
eigen_wt = eigen_wt.array().exp();
set_subkernel_weights(wt);
}

bool CCombinedKernel::init(CFeatures* l, CFeatures* r)
bool CCombinedKernel::init_with_extracted_subsets(
CFeatures* l, CFeatures* r, SGVector<index_t> lhs_subset,
SGVector<index_t> rhs_subset)
{
if(enable_subkernel_weight_opt && !weight_update)
{
init_subkernel_weights();
}

/* if the specified features are not combined features, but a single other
* feature type, assume that the caller wants to use all kernels on these */
if (l && r && l->get_feature_class()==r->get_feature_class() &&
l->get_feature_type()==r->get_feature_type() &&
l->get_feature_class()!= C_COMBINED)
{
SG_DEBUG("Initialising combined kernel's combined features with the "
"same instance from parameters\n");
/* construct combined features with each element being the parameter */
CCombinedFeatures* combined_l=new CCombinedFeatures();
CCombinedFeatures* combined_r=new CCombinedFeatures();
for (index_t i=0; i<get_num_subkernels(); ++i)
{
combined_l->append_feature_obj(l);
combined_r->append_feature_obj(r);
}

/* recursive call with constructed combined kernel */
return init(combined_l, combined_r);
}

CKernel::init(l,r);
REQUIRE(l->get_feature_class()==C_COMBINED, "%s::init(): LHS features are"
" of class %s but need to be combined features!\n",
get_name(), l->get_name());
REQUIRE(r->get_feature_class()==C_COMBINED, "%s::init(): RHS features are"
" of class %s but need to be combined features!\n",
get_name(), r->get_name());
ASSERT(l->get_feature_type()==F_UNKNOWN)
ASSERT(r->get_feature_type()==F_UNKNOWN)

CFeatures* lf=NULL;
CFeatures* rf=NULL;
CKernel* k=NULL;

bool result=true;
CKernel::init(l, r);
REQUIRE(
l->get_feature_class() == C_COMBINED,
"%s::init(): LHS features are"
" of class %s but need to be combined features!\n",
get_name(), l->get_name());
REQUIRE(
r->get_feature_class() == C_COMBINED,
"%s::init(): RHS features are"
" of class %s but need to be combined features!\n",
get_name(), r->get_name());
ASSERT(l->get_feature_type() == F_UNKNOWN)
ASSERT(r->get_feature_type() == F_UNKNOWN)

CFeatures* lf = NULL;
CFeatures* rf = NULL;
CKernel* k = NULL;

auto l_combined = dynamic_cast<CCombinedFeatures*>(l);
auto r_combined = dynamic_cast<CCombinedFeatures*>(r);

bool result = true;
index_t f_idx = 0;

SG_DEBUG("Starting for loop for kernels\n")
for (index_t k_idx=0; k_idx<get_num_kernels() && result; k_idx++)
for (index_t k_idx = 0; k_idx < get_num_kernels() && result; k_idx++)
{
k = get_kernel(k_idx);

Expand All @@ -118,11 +101,11 @@ bool CCombinedKernel::init(CFeatures* l, CFeatures* r)
// skip over features - the custom kernel does not need any
if (k->get_kernel_type() != K_CUSTOM)
{
if (((CCombinedFeatures*)l)->get_num_feature_obj() > f_idx &&
((CCombinedFeatures*)r)->get_num_feature_obj() > f_idx)
if (l_combined->get_num_feature_obj() > f_idx &&
r_combined->get_num_feature_obj() > f_idx)
{
lf = ((CCombinedFeatures*)l)->get_feature_obj(f_idx);
rf = ((CCombinedFeatures*)r)->get_feature_obj(f_idx);
lf = l_combined->get_feature_obj(f_idx);
rf = r_combined->get_feature_obj(f_idx);
}

f_idx++;
Expand All @@ -131,11 +114,13 @@ bool CCombinedKernel::init(CFeatures* l, CFeatures* r)
SG_UNREF(lf);
SG_UNREF(rf);
SG_UNREF(k);
SG_ERROR("CombinedKernel: Number of features/kernels does not match - bailing out\n")
SG_ERROR(
"CombinedKernel: Number of features/kernels does not "
"match - bailing out\n")
}

SG_DEBUG("Initializing 0x%p - \"%s\"\n", this, k->get_name())
result=k->init(lf,rf);
result = k->init(lf, rf);
SG_UNREF(lf);
SG_UNREF(rf);

Expand All @@ -144,13 +129,35 @@ bool CCombinedKernel::init(CFeatures* l, CFeatures* r)
}
else
{
SG_DEBUG("Initializing 0x%p - \"%s\" (skipping init, this is a CUSTOM kernel)\n", this, k->get_name())
SG_DEBUG(
"Initializing 0x%p - \"%s\" (skipping init, this is a CUSTOM "
"kernel)\n",
this, k->get_name())
if (!k->has_features())
SG_ERROR("No kernel matrix was assigned to this Custom kernel\n")
SG_ERROR(
"No kernel matrix was assigned to this Custom kernel\n")

auto k_custom = dynamic_cast<CCustomKernel*>(k);

// clear all previous subsets
k_custom->remove_all_row_subsets();
// apply new subset
k_custom->add_row_subset(lhs_subset);

k_custom->remove_all_col_subsets();
// apply new subset
k_custom->add_col_subset(rhs_subset);

if (k->get_num_vec_lhs() != num_lhs)
SG_ERROR("Number of lhs-feature vectors (%d) not match with number of rows (%d) of custom kernel\n", num_lhs, k->get_num_vec_lhs())
SG_ERROR(
"Number of lhs-feature vectors (%d) not match with number "
"of rows (%d) of custom kernel\n",
num_lhs, k->get_num_vec_lhs())
if (k->get_num_vec_rhs() != num_rhs)
SG_ERROR("Number of rhs-feature vectors (%d) not match with number of cols (%d) of custom kernel\n", num_rhs, k->get_num_vec_rhs())
SG_ERROR(
"Number of rhs-feature vectors (%d) not match with number "
"of cols (%d) of custom kernel\n",
num_rhs, k->get_num_vec_rhs())
}

SG_UNREF(k);
Expand All @@ -169,15 +176,99 @@ bool CCombinedKernel::init(CFeatures* l, CFeatures* r)
return false;
}

if ( ((CCombinedFeatures*) l)->get_num_feature_obj()<=0 ||
((CCombinedFeatures*) l)->get_num_feature_obj() != ((CCombinedFeatures*) r)->get_num_feature_obj() )
SG_ERROR("CombinedKernel: Number of features/kernels does not match - bailing out\n")
if (l_combined->get_num_feature_obj() <= 0 ||
l_combined->get_num_feature_obj() != r_combined->get_num_feature_obj())
SG_ERROR(
"CombinedKernel: Number of features/kernels does not match - "
"bailing out\n")

init_normalizer();
initialized=true;
initialized = true;
return true;
}

bool CCombinedKernel::init(CFeatures* l, CFeatures* r)
{
if (enable_subkernel_weight_opt && !weight_update)
{
init_subkernel_weights();
}
/*
* The two subsets, we will be passing those to init_with_extracted_subsets
*/
SGVector<index_t> lhs_subset;
SGVector<index_t> rhs_subset;

/*
* We will be passing these features to init_with_extracted_subsets
*/
CCombinedFeatures* combined_l;
CCombinedFeatures* combined_r;

/*
* Extract the subsets so that we can pass them on
*/
auto l_subset_stack = l->get_subset_stack();
auto r_subset_stack = r->get_subset_stack();

if (l_subset_stack->has_subsets())
{
lhs_subset = l_subset_stack->get_last_subset()->get_subset_idx();
}
else
{
lhs_subset = SGVector<index_t>(l->get_num_vectors());
lhs_subset.range_fill();
}

if (r_subset_stack->has_subsets())
{
rhs_subset = r_subset_stack->get_last_subset()->get_subset_idx();
}
else
{
rhs_subset = SGVector<index_t>(r->get_num_vectors());
rhs_subset.range_fill();
}

SG_UNREF(l_subset_stack);
SG_UNREF(r_subset_stack);

/* if the specified features are not combined features, but a single other
* feature type, assume that the caller wants to use all kernels on these */
if (l && r && l->get_feature_class() == r->get_feature_class() &&
l->get_feature_type() == r->get_feature_type() &&
l->get_feature_class() != C_COMBINED)
{
SG_DEBUG(
"Initialising combined kernel's combined features with the "
"same instance from parameters\n");
/* construct combined features with each element being the parameter
* The we must make sure that we make any custom kernels aware of any
* subsets present!
*/
combined_l = new CCombinedFeatures();
combined_r = new CCombinedFeatures();

for (index_t i = 0; i < get_num_subkernels(); ++i)
{
combined_l->append_feature_obj(l);
combined_r->append_feature_obj(r);
}
}
else
{
/*
* Otherwise, we just pass l & r straight on
*/
combined_l = (CCombinedFeatures*)l;
combined_r = (CCombinedFeatures*)r;
}

return init_with_extracted_subsets(
combined_l, combined_r, lhs_subset, rhs_subset);
}

void CCombinedKernel::remove_lhs()
{
delete_optimization();
Expand Down Expand Up @@ -971,4 +1062,4 @@ CList* CCombinedKernel::combine_kernels(CList* kernel_list)
}

return return_list;
}
}
15 changes: 15 additions & 0 deletions src/shogun/kernel/CombinedKernel.h
View file
Expand Up @@ -458,6 +458,21 @@ class CCombinedKernel : public CKernel

private:
void init();
/**
* The purpose of this function is to make customkernels aware of any
* subsets present, regardless whether the features passed are of type
* CCombinedFeatures or not
* @param lhs combined features
* @param rhs rombined features
* @param lhs_subset subset present on lhs - pass identity subset if
* none
* @param rhs_subset subset present on rhs - pass identity subset if
* none
* @return init succesful
*/
bool init_with_extracted_subsets(
CFeatures* lhs, CFeatures* rhs, SGVector<index_t> lhs_subset,
SGVector<index_t> rhs_subset);

protected:
/** list of kernels */
Expand Down

0 comments on commit eed22ff

Please sign in to comment.